feat(cli): add --output json/yaml to sandbox get, status, and sandbox create#1989
Conversation
|
All contributors have signed the DCO ✍️ ✅ |
|
I have read the DCO document and I hereby sign the DCO. |
|
recheck |
1816278 to
9aa0ec4
Compare
|
@johntmyers thanks for the review! As I'm on my way to dive into the codebase I will try to understand the codebase a bit better (not a Rust coder yet). But I think your comment ironically also hint to a problmen that this PR tries to partially solve: We need to be careful currently around the wording of log/error messages as they are parsed and used for result evaluation (i know at least one tool that wraps that parses the output of Therefor having the option for a structured output make much sense for these use cases. I wonder though, whether one should keep stderr free for informal messages or whether not (if |
|
I'll create a follow-up issue for emitting structured JSON errors when |
f067c05 to
d287542
Compare
|
All comments resolved, what do I need to trigger the CI ? 🙏 |
--> #2040 |
d287542 to
a0a287f
Compare
elezar
left a comment
There was a problem hiding this comment.
Requesting changes because structured output does not currently guarantee clean stdout.
When --output json or --output yaml is selected, display is set to None. However, handle_platform_progress_event() interprets None as non-interactive/plain output and prints completed steps and details with println!(). If provisioning emits a progress event, stdout will contain human-readable lines before the final JSON/YAML document, making it invalid for automation.
Please:
- Introduce an explicit structured progress mode that is silent or writes only to stderr.
- Reserve stdout exclusively for the final JSON/YAML document.
- Add integration coverage that emits provisioning progress, captures the complete stdout from
sandbox create --output json/yaml, and successfully parses it.
I recommend modeling interactive, plain, and structured progress as explicit variants rather than using Option<ProvisioningDisplay>. The full presenter refactor is not required to unblock this PR; the blocking requirement is that structured stdout remains parseable under all provisioning event paths.
baee9b4 to
22b16cb
Compare
|
Thanks for the review. All four points are addressed: 1. Explicit structured progress mode (commit 2. Stdout reserved for final JSON/YAML (commit 3. Integration coverage (commit 4. Replace |
|
@elezar Gentle ping - the feedback from your review has been addressed in commit |
|
@rhuss I think this looks better now. Do you mind rebasing so that we can complete the review? (I have a added a test-only commit on top that you can include in your changes as you see fit). |
5c4f020 to
7c16a74
Compare
|
@elezar rebased on upstream/main and squashed to a single commit to clean up a bit (otherwise a lot of back and forth between the various commits, mostly because of several rebasing rounds). Let me know if something is still missing. |
… create Add structured output support (JSON and YAML) to sandbox lifecycle commands: sandbox get, status, and sandbox create. Introduce ProgressOutput enum to cleanly separate interactive, plain, and silent display modes during sandbox provisioning. Signed-off-by: Roland Huß <rhuss@redhat.com>
7c16a74 to
212217a
Compare
Signed-off-by: Evan Lezar <elezar@nvidia.com>
|
/ok-to-test 212217a |
Signed-off-by: Evan Lezar <elezar@nvidia.com>
|
Pushed two small follow-up fixes:
Possible follow-ups:
|
|
/ok-to-test 0a91547 |
|
Label |
|
@rhuss please resolve the comments that have been addressed. This should merge the PR. |
Summary
Add
--output json/yamltosandbox get,status, andsandbox createso automation and MCP tool wrappers can consume structured data instead of parsing ANSI-colored human output.Related Issue
Closes #1964
Changes
sandbox get --output json/yaml: Newsandbox_detail_to_jsonconverter enriches the existingsandbox_to_jsonwith policy source, revision, and active policy content fromGetSandboxConfigResponse. Usessandbox_policy_to_json_valuefor direct proto-to-JSON conversion.status --output json/yaml: Newstatus_to_jsonconverter emits gateway, server, status, and conditional fields (auth, version, error, http_status). Refactoredgateway_statusto build data before output routing.sandbox create --output json/yaml: Emits sandbox metadata after Ready phase. Human chrome (header, spinner) suppressed from stdout; progress message on stderr. Uploads and port forwarding still execute before output. Clap rejects--outputcombined with--editoror trailing commands.status --output jsonwith no configured gateway emits{"status": "not_configured"}instead of human text.OutputFormatenum andprint_output_singlehelper.--outputflag) is byte-identical to current behavior (verified by diffing raw ANSI output).sandbox get,sandbox create, andstatusinmanage-sandboxes.mdxandmanage-gateways.mdx.Testing
cargo clippyclean,cargo fmtcleancargo test --lib)sandbox_detail_to_jsonandstatus_to_jsonconverterssandbox get --output json,status --output json/yaml, default output byte-identical regression checkChecklist