Use rich for rendering providers command#12698
Use rich for rendering providers command#12698turbaszek wants to merge 1 commit intoapache:masterfrom
Conversation
| return tabulate(tabulate_data, tablefmt=tablefmt, headers='keys') | ||
|
|
||
|
|
||
| def _remove_rst_syntax(value: str) -> str: |
| def providers_list(args): | ||
| """Lists all providers at the command line""" | ||
| print(_tabulate_providers(ProvidersManager().providers.values(), args.output)) | ||
| console = Console() |
There was a problem hiding this comment.
The previous version supported different kinds of format output (for example --output grid . We've lost that feature. Can we get it back? Or at the very least we shoud remove the "--output" arg from the command . I am using grid in one of the PRs to check for presence of providers, so it would be nice to keep it.
There was a problem hiding this comment.
We should be able but we would need to use rich tables formats. Some of tabulate formats are not supported (latex, html). Which one do you use?
I'm wondering in general how crucial this feature is. In my opinion the output should be predefined and opinionated, and easy to pipe through bash commands.
There was a problem hiding this comment.
Since one line can contain more than one element, the output is not easily parsed by other scripts.
There was a problem hiding this comment.
Since one line can contain more than one element
I agree and that has not changed. The only change here is removing rst syntax from description and adding | between columns which helps when using cut -d "|"
| for _, provider in ProvidersManager().providers.values(): | ||
| table.add_row( | ||
| provider['package-name'], | ||
| _remove_rst_syntax(provider['description']), |
There was a problem hiding this comment.
| _remove_rst_syntax(provider['description']), | |
| provider['name'], |
Do we need to display the full description in the table? Maybe the name is enough?
There was a problem hiding this comment.
The description has a url which can be helpful. However, I'm leaning towards showing only provider + version. On the other hand that information is also available via airflow info
| print(f"Provider: {args.provider_name}") | ||
| print(f"Version: {provider_version}") | ||
| if args.full: | ||
| provider_info["description"] = _remove_rst_syntax(provider_info["description"]) |
There was a problem hiding this comment.
I would say that if args.full is passed we should not print the first two lines - just output the yaml / json, this will allow users to use it in jq for example
3 participants
Before:
After:
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.