Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds JSON and YAML output format options to the netlab status command to support automation-friendly output. The command previously only supported human-readable text output.
Changes:
- Added
--formatargument with choicesjson,yaml, andtext(default) - Refactored status display logic to support structured data output
- Added error handling that produces JSON/YAML error messages for non-text formats
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
| netsim/cli/status.py | Added format argument parsing, restructured functions to support JSON/YAML output, added print_result and cleanup_state functions for data formatting |
| docs/netlab/status.md | Updated usage documentation to show the new --format option and added tip about automation-friendly formats |
| --format {json,yaml,text} | ||
| Specify the output format |
There was a problem hiding this comment.
Inconsistent spacing in formatted output. The usage block shows the --format option without proper indentation alignment compared to the other help text entries. The help text "Specify the output format" should align with the other help text entries for consistency.
| --format {json,yaml,text} | |
| Specify the output format | |
| --format {json,yaml,text} Specify the output format |
| ls.nodes[t_name] = { | ||
| 'device': '(tool)', | ||
| 'image': wk_state.get('image',''), | ||
| 'connection': 'docker', | ||
| 'provider': n_provider, | ||
| 'provider_name': wk_name, | ||
| 'status': wk_state.get('status','Not running') | ||
| } |
There was a problem hiding this comment.
Missing 'mgmt' field for tools. In the fetch_node_status function, tools (lines 186-193) don't have a 'mgmt' field set, but show_lab_nodes at line 200 tries to access it with 'n_data.mgmt or ''' which will raise an AttributeError when n_data doesn't have a 'mgmt' attribute. Either add 'mgmt': '' to the tools dictionary or use n_data.get('mgmt', '') instead.
|
@sdargoeuves -- this is the replacement for the first part of #3004. Would love to hear how well it works for you. Once we get this merged, I'll add the API part. |
sdargoeuves
left a comment
sdargoeuves
left a comment
There was a problem hiding this comment.
This is great! I've done quite a few tests on different labs, it works brilliantly.
I really like the addition of the yaml option, it's nicer to read for a human than the json format 😉
2 participants
No description provided.