-
Notifications
You must be signed in to change notification settings - Fork 548
Feature/Improved CLI lists with enhanced tables and new formats #3866
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Conversation
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like where this is going. It's been a long time that we needed some uniformity in how we list items in the CLI as well as give the user more options around the formatting.
However, not all list commands were created equal and I feel some of them have lost some of their glamour after this change. Perhaps allowing things like emojis and multi-line output in the table format can allow them to shine under new management as they currently do ? Or something like supporting multiple verbosity levels, if you feel the default amount of information is too verbose to be useful for the user.
🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Add missing parameter documentation and return values for CLI display functions to resolve CI docstring validation errors. This includes fixes for service connectors, table utilities, tags, pipelines, stack components, models, artifacts, authorized devices, secrets, and service accounts. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
Complete the docstring fixes for artifact versions and ensure all critical CLI table functions have proper parameter and return documentation. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Add missing Args and Returns sections to functions in model.py - Fix docstring issues in stack_components.py flavor functions - Add parameter documentation to code_repository.py - Fix utils.py print_page_info docstring - Complete user_management.py docstring documentation - Add missing documentation to project.py functions All darglint docstring validation errors are now resolved for CLI files. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Fix Page[T] vs List[Any] type mismatches in prepare_list_data calls - Add type ignore comments for model_dump no-any-return issues - Fix missing type annotations in user_management.py - Handle object vs List[Any] issues with proper type ignore comments - Ensure proper handling of paginated vs non-paginated responses Resolves all remaining mypy errors that were causing CI failures. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
- Fix line formatting in project.py, service_accounts.py, tag.py, user_management.py - Ensure consistent formatting across all modified CLI files 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
@schustmi This commit should solve our output rerouting issues. Feel free to take a look. |
Documentation Link Check Results❌ Absolute links check failed |
@schustmi I also removed the method that suppresses the server-client mismatch warning. |
✅ No broken links found! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't have the mental capacity to even start looking at the _render_table(...)
function and all the helper functions it calls, I'll do that some other day.
src/zenml/cli/utils.py
Outdated
default=False, | ||
help="Disable truncation of long values.", | ||
), | ||
click.option( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way we can tell users what the available columns are?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, this is a bit more difficult. Maybe we can think about how we can generalize it. My first approach was about using the corresponding Response
model. We can use the ID, name (if present), and the attributes in the ResponseBody
. However, most entities have specific columns that are not present in the body, like:
active
column is not present for the StackResponseBody or the StackResponseMetadatastack
column is not present for the RunResponseBody, but can be extracted from the RunResponseMetadataflavor
columns is namedflavor_name
in the ComponentResponseBody.user
columns come from the name column within the UserScopedResources
So, it is a bit hard to generate this before loading anything. But I will try to think of a way.
ENV_ZENML_CODE_REPOSITORY_IGNORE_UNTRACKED_FILES = ( | ||
"ZENML_CODE_REPOSITORY_IGNORE_UNTRACKED_FILES" | ||
) | ||
ENV_ZENML_DEFAULT_OUTPUT = "ZENML_DEFAULT_OUTPUT" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ZENML_CLI_DEFAULT_OUTPUT_FORMAT
maybe?
"ZENML_CODE_REPOSITORY_IGNORE_UNTRACKED_FILES" | ||
) | ||
ENV_ZENML_DEFAULT_OUTPUT = "ZENML_DEFAULT_OUTPUT" | ||
ENV_ZENML_CLI_COLUMN_WIDTH = "ZENML_CLI_COLUMN_WIDTH" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ZENML_CLI_TABLE_COLUMN_WIDTH
src/zenml/cli/utils.py
Outdated
ZenML responses have a structured format: | ||
- Top-level fields: id, name (extracted) | ||
- Body: Core entity data (all fields extracted) | ||
- Metadata: Domain-specific data (IGNORED as requested) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(IGNORED as requested)
: I will not approve this PR as long as there is any vibe-coded stuff like this anywhere.
src/zenml/cli/utils.py
Outdated
Args: | ||
items: List of BaseResponse instances to format | ||
enrichment_func: Optional function to add/modify fields |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not the way we format docstrings.
src/zenml/utils/table_utils.py
Outdated
|
||
# Add pagination metadata if provided | ||
if pagination: | ||
output_data = {"items": prepared_data, "pagination": pagination} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we put the pagination info at the root level, it would mirror our Page
class which is nicer IMO
src/zenml/utils/table_utils.py
Outdated
|
||
# Add pagination metadata if provided | ||
if pagination: | ||
output_data = {"items": prepared_data, "pagination": pagination} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as for json
src/zenml/cli/utils.py
Outdated
click.option( | ||
"--output", | ||
"-o", | ||
type=click.Choice(["table", "json", "yaml", "tsv", "none"]), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing csv
src/zenml/utils/table_utils.py
Outdated
return yaml.dump(prepared_data, default_flow_style=False) | ||
|
||
|
||
def _render_tsv( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is essentially a CSV with a different separator. There are classes in the python stdlib to convert items to a CSV that even allow configuring separators, I think we should use them here.
src/zenml/utils/table_utils.py
Outdated
if not data: | ||
return [] | ||
|
||
# Clean internal fields if requested (for JSON/YAML output) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would any format want to display internal fields?
ZenML CLI Performance Comparison (Threshold: 1.0s, Timeout: 60s, Slow: 5s)❌ Failed Commands on Current Branch (feat/improved-cli-tables)
🚨 New Failures IntroducedThe following commands fail on your branch but worked on the target branch:
Performance Comparison
Summary
Environment Info
|
Describe changes
I implemented enhanced CLI table formatting and multiple output formats to achieve consistent, flexible, and user-friendly list outputs across all ZenML CLI commands.
This PR introduces:
table
,json
,yaml
,tsv
, andnone
--columns
,--sort
,--reverse
,--no-truncate
,--no-color
,--output
zenml_table()
functionenhanced_list_options
) that adds consistent table functionality to all list commandsThe changes affect all major CLI list commands, including
stacks
,models
,pipelines
,artifacts
,users
, and more, providing a unified and improved user experience.Pre-requisites
Please ensure you have done the following:
develop
and the open PR is targetingdevelop
. If your branch wasn't based on develop read Contribution guide on rebasing branch to develop.Types of changes