fix(logs): match server LogEvent body field, docs fixes (VOL-648 sub-tickets) - #149
Merged
Merged
Conversation
volcano-hosting renamed LogEvent's content field from message (string) to body (oneOf string/object/array/number/boolean), but the vendored OpenAPI spec and generated client were never updated. Build/publish log streaming rendered blank lines for every event since event.Message no longer existed in the API response. Regenerate the client from the corrected spec and render the union body type for both function log commands. VOL-636
internal/api's own test fixtures and assertions still referenced the old LogSearchEvent.Message field/JSON key, missed in the initial client.gen.go regeneration for the message->body rename.
|
|
--region's usage string gave no hint of the aws-<aws-region> format the API expects, so a --help user could still hit the same 400 the docs fix addresses. Also add array/number/boolean/null coverage for LogEvent body rendering alongside the existing string/object cases.
The null-body case in TestLogEventsRendersEveryBodyVariant only proved no panic; assert the rendered line actually stays empty so a regression that prints the literal "null" would be caught.
internal/cmd/frontends, internal/cmd/functions, and internal/logfollow still built test log events with the old "message" field, so their fixtures silently decoded to an empty body under the regenerated client and left TestFrontendsLogs/TestFunctionsLogs asserting on blank output. internal/logfollow's stream tests additionally hung: the follow loop never observed a non-empty body to detect stream progress, so it blocked forever on the next read instead of failing fast.
These test helpers build the "body" wire field but still named the parameter "message" from before the server-side rename, same as client_test.go's logsResponse already does.
There was a problem hiding this comment.
Pull request overview
Updates log handling to match the server’s polymorphic body field and corrects database/project documentation.
Changes:
- Regenerates API log models and renders all supported body types.
- Updates log fixtures and regression tests.
- Corrects database-region guidance and documents project anon keys.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
openapi/openapi.yaml |
Defines the new log body schema. |
internal/apiclient/client.gen.go |
Regenerates log models and unions. |
internal/output/functions.go |
Renders polymorphic log bodies. |
internal/output/functions_test.go |
Tests body variants and null handling. |
internal/logfollow/follow_test.go |
Updates streaming fixtures. |
internal/cmd/functions/logs_test.go |
Updates function-log fixtures. |
internal/cmd/frontends/logs_test.go |
Updates frontend-log fixtures. |
internal/api/log_stream_test.go |
Verifies streamed body decoding. |
internal/api/frontends_test.go |
Updates frontend API assertions. |
internal/api/client_test.go |
Adds body extraction test support. |
internal/cmd/databases/create.go |
Clarifies region flag help. |
docs/README.md |
Documents project anon-key retrieval. |
docs/databases.md |
Corrects database region examples. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }, | ||
| } | ||
| cmd.Flags().StringVar(®ion, "region", "", "Database region") | ||
| cmd.Flags().StringVar(®ion, "region", "", "Database region (aws-<aws-region>, e.g. aws-us-east-1)") |
| # Create a database (defaults to type volcano-db-xs) | ||
| volcano databases create app | ||
| volcano databases create app --type volcano-db-s --region us-east-1 | ||
| volcano databases create app --type volcano-db-s --region aws-us-east-1 |
Comment on lines
+9050
to
+9051
| nullable: true | ||
| oneOf: |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes for VOL-648 sub-tickets scoped to volcano-cli:
LogEvent's content field frommessage(string) tobody(oneOf string/object/array/number/boolean) in a prior release, but the vendored OpenAPI spec and generated client here were never updated —volcano cloud frontends logs/volcano functions logsstreamed blank lines for every event sinceevent.Messageno longer existed in the API response. Regenerated the client from a corrected spec and updated both log-rendering call sites and the affected tests.docs/databases.md's--regionexample used a plain AWS region (us-east-1); the API expects Neon-styleaws-<region>identifiers and 400s otherwise. Fixed the example.volcano projects keys, PR feat(cli): add 'volcano projects keys' command to retrieve a project's anon key #86) but undocumented in the CLI overview, so a user searching undercloudwouldn't find it. Added it todocs/README.md.VOL-640 (
setup --harness/--yesmutual exclusivity) is scoped to a different in-flight ticket and not touched here — also independently already resolved on main (--agentrename, PR #146).Ticket
VOL-648 (umbrella, cancelled as duplicate) — sub-tickets VOL-636, VOL-645, VOL-646
Todo (gates — each requires evidence)
projects keysdocs (VOL-645/VOL-646)self-review-workclean — cycles: 3, final pass no blockers057dbaccPrior Art
internal/output/functions.go's existing pattern for other oapi-codegen oneOf union types — reused theMarshalJSON/string-unmarshal accessor pattern rather than inventing a new one.Testing
make openapi-generated-checkmake test(now passes cleanly,go build ./...+go vet ./...clean)make lint— fails on a pre-existing, unrelated issue ininternal/auth/auth.go(stale//nolint:contextcheckdirective now flagged as unused); reproduced on a cleanorigin/maincheckout with none of this branch's changes, so it predates this PR. Flagged separately for its own fix.null-body case in the new table test only proved no panic — asserted nothing about the rendered output. Fixed by asserting the render doesn't contain the literal"null".LogEvent/LogSearchEventrender helpers' test-fixture builders still named their string parammessagepost-rename (fixed for consistency withclient_test.go's already-renamedlogsResponse); extraLogSearchEventnon-string coverage suggested but skipped as low-value (shares the samebodyTexthelperLogEventalready covers for all variants).go test ./...— broader than either review pass's own scope of "packages touched by the diff" — surfaced thatinternal/cmd/frontends,internal/cmd/functions, andinternal/logfollowstill built their test log fixtures with the oldmessagefield, soTestFrontendsLogs/TestFunctionsLogssilently asserted on blank rendered output, andinternal/logfollow's stream tests hung indefinitely (the follow loop never observed a non-empty body to detect progress). Fixed all three files' fixtures to usebody.