add caib show#93
Conversation
Signed-off-by: Benny Zlotnik <bzlotnik@redhat.com>
📝 WalkthroughWalkthroughThis pull request introduces a new Changes
Sequence DiagramsequenceDiagram
actor User
participant CLI as CLI (show command)
participant Client as API Client
participant Server as API Server
User->>CLI: caib show build-name
CLI->>Client: GetBuild(ctx, "build-name")
Client->>Server: GET /v1/builds/build-name
Server-->>Client: BuildResponse
alt Parameters missing
CLI->>Client: GetBuildTemplate(ctx, "build-name")
Client->>Server: GET /v1/builds/build-name/template
Server-->>Client: BuildTemplateResponse
CLI->>CLI: Populate Parameters from template
end
CLI->>CLI: Format output (JSON/YAML/table)
CLI-->>User: Display formatted build details
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@cmd/caib/main.go`:
- Around line 1949-1959: The current fallback call to executeWithReauth ignores
errors and swallows failures when st.Parameters is nil; update the block so you
capture the returned error from executeWithReauth(...) and, if non-nil, write a
clear warning to stderr that includes context (e.g., "failed to fetch build
template for showBuildName" or similar) and the actual error value rather than
discarding it; keep the existing behavior of setting st.Parameters =
buildParametersFromTemplate(tpl) on success and only log (not fatal) on failure
so users see why parameters are missing.
🧹 Nitpick comments (3)
cmd/caib/README.md (1)
255-262: Note:-oflag semantics differ betweenshowanddownload.In
show,-ocontrols output format (table/json/yaml), while indownload,-ospecifies the destination file path. This is documented correctly but worth calling out for users who switch between the two commands. Consider whether--formatmight be clearer forshowto avoid ambiguity.internal/buildapi/types.go (1)
224-240:omitemptyonboolfields suppressesfalsein JSON/YAML output.Fields like
BuildDiskImage,FlashEnabled, andUseServiceAccountAuthwill be omitted from JSON/YAML whenfalse. This meanscaib show -o jsonwon't distinguish "explicitly false" from "field absent." If that distinction matters for consumers, removeomitemptyfrom these bool fields.If the intent is purely supplementary info where absence implies false, this is fine as-is.
cmd/caib/main.go (1)
2003-2018: Consider simplifying the all-fields-empty check withreflect.DeepEqual.The exhaustive field-by-field zero check is correct but fragile — adding a new field to
BuildParametersrequires updating this check too. A simpler alternative:if *params == (buildapitypes.BuildParameters{}) { return nil }This compares against the zero value of the struct in one expression and stays in sync as fields are added.
| // Backward-compatible fallback for older API servers that do not yet include response parameters. | ||
| if st.Parameters == nil { | ||
| _ = executeWithReauth(serverURL, &authToken, func(api *buildapiclient.Client) error { | ||
| tpl, err := api.GetBuildTemplate(ctx, showBuildName) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| st.Parameters = buildParametersFromTemplate(tpl) | ||
| return nil | ||
| }) | ||
| } |
There was a problem hiding this comment.
Silently swallowing template fallback errors hides legitimate failures.
When Parameters is nil (older server), the template fetch error is discarded with _ = executeWithReauth(...). If the failure is due to a network error, auth issue, or server bug (not just a missing endpoint), the user sees no parameters with no explanation.
Consider logging a warning to stderr on failure:
Suggested improvement
if st.Parameters == nil {
- _ = executeWithReauth(serverURL, &authToken, func(api *buildapiclient.Client) error {
+ err := executeWithReauth(serverURL, &authToken, func(api *buildapiclient.Client) error {
tpl, err := api.GetBuildTemplate(ctx, showBuildName)
if err != nil {
return err
}
st.Parameters = buildParametersFromTemplate(tpl)
return nil
})
+ if err != nil {
+ fmt.Fprintf(os.Stderr, "Warning: could not fetch build parameters: %v\n", err)
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Backward-compatible fallback for older API servers that do not yet include response parameters. | |
| if st.Parameters == nil { | |
| _ = executeWithReauth(serverURL, &authToken, func(api *buildapiclient.Client) error { | |
| tpl, err := api.GetBuildTemplate(ctx, showBuildName) | |
| if err != nil { | |
| return err | |
| } | |
| st.Parameters = buildParametersFromTemplate(tpl) | |
| return nil | |
| }) | |
| } | |
| // Backward-compatible fallback for older API servers that do not yet include response parameters. | |
| if st.Parameters == nil { | |
| err := executeWithReauth(serverURL, &authToken, func(api *buildapiclient.Client) error { | |
| tpl, err := api.GetBuildTemplate(ctx, showBuildName) | |
| if err != nil { | |
| return err | |
| } | |
| st.Parameters = buildParametersFromTemplate(tpl) | |
| return nil | |
| }) | |
| if err != nil { | |
| fmt.Fprintf(os.Stderr, "Warning: could not fetch build parameters: %v\n", err) | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@cmd/caib/main.go` around lines 1949 - 1959, The current fallback call to
executeWithReauth ignores errors and swallows failures when st.Parameters is
nil; update the block so you capture the returned error from
executeWithReauth(...) and, if non-nil, write a clear warning to stderr that
includes context (e.g., "failed to fetch build template for showBuildName" or
similar) and the actual error value rather than discarding it; keep the existing
behavior of setting st.Parameters = buildParametersFromTemplate(tpl) on success
and only log (not fatal) on failure so users see why parameters are missing.
2 participants
Summary by CodeRabbit
New Features
showcommand to display detailed build information in JSON, YAML, or table formats.Changed
downloadcommand to use positional arguments instead of named flags.-o/--outputformat.