Add option for tabular output by wfchandler · Pull Request #1167 · oxidecomputer/oxide.rs

wfchandler · 2025-07-22T21:46:50Z

Currently all commands will display their results as pretty-printed JSON. Tabular output is convenient for easy comparison of a single field when multiple objects are returned, and more amenable to traditional shell processing.

Add a new global --format flag to control output format, defaulting to the existing JSON output. As larger output items, such as instances, can easily overflow a typical terminal width, we allow users to specify which fields to print with --format=table:field1,field2,....

Non-scalar fields within a returned object will be printed in compact JSON format, e.g. {"cpus":0,"memory":0,"storage":0} for the allocated field on oxide silo utilization list.

To determine the field names to be shown in the table header, we parse the schema for the return type as part of OxideOverride. This logic makes some assumptions about the shape of the data returned, and we need to ensure that it remains valid. Add a new return_types xtask job, which writes out all return types from the Oxide API to a file, against which we test the parsing logic.

cli/src/main.rs

Currently all commands will display their results as pretty-printed JSON. Tabular output is convenient for easy comparison of a single field when multiple objects are returned, and more amenable to traditional shell processing. Add a new global `--format` flag to control output format, defaulting to the existing JSON output. As larger output items, such as instances, can easily overflow a typical terminal width, we allow users to specify which fields to print with `--format=table:field1,field2,...`. Non-scalar fields within a returned object will be printed in compact JSON format, e.g. `{"cpus":0,"memory":0,"storage":0}` for the `allocated` field on `oxide silo utilization list`. To determine the field names to be shown in the table header, we parse the schema for the return type as part of `OxideOverride`. This logic makes some assumptions about the shape of the data returned, and we need to ensure that it remains valid. Add a new return_types `xtask` job, which writes out all return types from the Oxide API to a file, against which we test the parsing logic.

xtask/src/main.rs

wfchandler · 2025-07-25T13:05:54Z

I manually verified this works against all list and view subcommands, other than alert receiver, which don't have any data in our environments. Also spot checked a couple create commands, though we won't show a table for these as they have a string return type.

ahl

I think it would be helpful to have an overview of how this works.

Obviously I had designed the overrides to enable this work, but this isn't how I expected it to be implemented.

xtask/src/main.rs

ahl · 2025-08-04T21:43:59Z

xtask/src/main.rs

    }

+    if return_types {
+        print!("generating return types ... ");


this may require additional explanation

No longer printed now that return types are generated in sdk.

ahl · 2025-08-04T21:46:05Z

xtask/src/main.rs

+        std::io::stdout().flush().unwrap();
+        let mut ret_types = BTreeSet::new();
+
+        for path in spec.paths.paths.values().cloned() {


this approach seems brittle. Did you consider using syn to parse the generated sdk:

for types in mod builder { look for an `fn send` take its response type }

You could make this part of the "generate SDK" potentially if you're worried about input and output getting out of sync.

It's a little more verbose to use syn, but I agree that it's less likely to blow up in the future. I've also moved this into the sdk step as suggested.

ahl · 2025-08-04T21:53:12Z

cli/src/oxide_override.rs

+                let printable_fields = set_header_fields(&fields, available_fields, &mut table);
+
+                let serde_json::Value::Object(obj) =
+                    serde_json::to_value(std::ops::Deref::deref(value))


We need a JSON object to access the fields expected from the schema.

Does that answer your question?

Sorry: try value.as_ref()

Ah ok, makes sense.

ahl · 2025-08-04T22:06:45Z

cli/src/oxide_override.rs

+                }
+
+                // Store our list of fields to print.
+                std::mem::swap(&mut printable_fields, &mut fields);


I don't follow why you're doing this

I've refactored the table logic into a new TableFormatter struct. I think this easier to follow.

wfchandler · 2025-08-13T18:38:23Z

I think it would be helpful to have an overview of how this works.

Added a comment on this in 0dd9137

Obviously I had designed the overrides to enable this work, but this isn't how I expected it to be implemented.

What was your plan?

ahl

$ cargo run -- --format table:t project list
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.13s
     Running `target/debug/oxide --format 'table:t' project list`
WARNING: 644 permissions on "/Users/ahl/.config/oxide/credentials.toml" may allow other users to access your login credentials.
WARNING: 't' is not a valid field
table formatting is not supported for this command

One of the things I'm struggling with here is how it this option will be used outside of a demo context. For example, --format table doesn't seem very usable on its own, so I need to choose specific columns. But how do I know what those are? What's a typical workflow? Would we imagine people embedding these lists .. in scripts? What happens when a new and important field is added to the output.

ahl · 2025-08-25T21:09:49Z

xtask/src/main.rs

+
+    let schemas = return_types.into_iter().map(|ty| {
+        quote! {
+            (stringify!(oxide::#ty), schemars::schema_for!(oxide::#ty))


why stringify! rather than making it a string here? I guess they're equivalent, but curious about the decision.

ahl · 2025-08-25T21:15:17Z

xtask/src/main.rs

+    let schemas = return_types.into_iter().map(|ty| {
+        quote! {
+            (stringify!(oxide::#ty), schemars::schema_for!(oxide::#ty))
+        }
+    });
+
+    quote! {
+        pub fn schemas() -> Vec<(&'static str, schemars::schema::RootSchema)> {
+            vec![
+                #(#schemas),*
+            ]
+        }
+    }


Also, I think you could inline this if you wanted:

quote! { pub fn schemas() -> ... { vec![ #( stringify!(oxide:: #return_types), schemars::schema_for!(oxide:: #return_types)),* ] } }

ahl · 2025-08-25T21:17:25Z

xtask/src/main.rs

+            _ => None,
+        }) {
+            if let ReturnType::Type(_, ty) = &method.sig.output {
+                if let Some(return_type) = extract_response_value_inner_type(ty) {


if this fails we ignore it? Is that intended?

ahl · 2025-08-25T21:37:55Z

xtask/src/main.rs

+/// Extract the Oxide success type returned by `send`.
+/// For example, `types::Vpc` in `Result<ResponseValue<types::Vpc>, Error>`.


I think more words here would be helpful

ahl · 2025-08-26T17:22:33Z

cli/src/oxide_override.rs

+                if available_fields.is_empty() {
+                    println_nopipe!("{TABLE_NOT_SUPPORTED}");
+                    return;
+                }


in what situation will this occur?

ahl · 2025-08-26T17:31:02Z

cli/src/oxide_override.rs

+        }
+    };
+
+    // Check if we're receiving an array.


this seems to do the opposite i.e. it checks if we're not receiving an array.

ahl · 2025-08-26T17:41:59Z

cli/src/oxide_override.rs

+}
+
+/// Find the fields present on a the object returned by `success_item`.
+fn success_item_fields(root_schema: &RootSchema) -> (Vec<String>, ReturnType) {


I'm concerned that this function seems fairly brittle. Is the idea that it will be mitigated by the tests?

ahl · 2025-08-26T17:42:26Z

cli/src/oxide_override.rs

+            include!("../tests/data/api_return_types.rs");
+        }
+        for (type_name, schema) in schemas::schemas() {
+            let fields = if type_name.ends_with("Page") {


can we articulate this heuristic?

ahl · 2025-08-26T17:43:47Z

cli/src/oxide_override.rs

+    use super::*;
+
+    #[test]
+    fn test_table_schema_parsing() {


My fear is that this test is going to break frequently as a result of unexpected schema data. Is that a rational fear or do you expect that this will be fairly robust?

ahl · 2025-08-26T17:47:07Z

cli/src/oxide_override.rs

@@ -0,0 +1,600 @@
+// This Source Code Form is subject to the terms of the Mozilla Public


it would be easier to review this if we first move override to its own file.

wfchandler · 2025-09-29T20:23:00Z

Closing in favor of #1221

wfchandler marked this pull request as draft July 22, 2025 21:47

david-crespo reviewed Jul 22, 2025

View reviewed changes

cli/src/main.rs Outdated Show resolved Hide resolved

wfchandler force-pushed the wc/tablular-output branch from 9ef7b5d to d005c78 Compare July 23, 2025 15:17

wfchandler force-pushed the wc/tablular-output branch from d005c78 to bc5cd9b Compare July 23, 2025 15:34

wfchandler marked this pull request as ready for review July 23, 2025 16:06

wfchandler requested a review from ahl July 23, 2025 16:06

wfchandler commented Jul 23, 2025

View reviewed changes

xtask/src/main.rs Outdated Show resolved Hide resolved

ahl requested changes Aug 4, 2025

View reviewed changes

wfchandler added 4 commits August 13, 2025 13:04

Get return types with syn

d1817ff

Refactor table printing to struct

f7eb719

Comment on how table formatting works

0dd9137

Merge branch 'main' into wc/tablular-output

4cfc84a

wfchandler requested a review from ahl August 13, 2025 18:39

wfchandler added 3 commits August 14, 2025 14:14

Use as_ref instead of deref

085cc7c

Merge branch 'main' into wc/tablular-output

4d139b6

Update docs and codegen

26ca4df

ahl reviewed Aug 26, 2025

View reviewed changes

wfchandler closed this Sep 29, 2025

		/// Extract the Oxide success type returned by `send`.
		/// For example, `types::Vpc` in `Result<ResponseValue<types::Vpc>, Error>`.

		@@ -0,0 +1,600 @@
		// This Source Code Form is subject to the terms of the Mozilla Public

Conversation

wfchandler commented Jul 22, 2025

Uh oh!

Uh oh!

Uh oh!

wfchandler commented Jul 25, 2025

Uh oh!

ahl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wfchandler commented Aug 13, 2025

Uh oh!

ahl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wfchandler commented Sep 29, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants