refactor(cli): migrate from Click to Cyclopts + Rich - #1894
Conversation
84923bc to
6b3c080
Compare
Replace Click with Cyclopts for argument parsing/help and Rich for all output. - pyinfra.api.output gains a shared Rich Console; format_text/echo are Rich-backed adapters preserving the legacy click.style signature - pyinfra_cli.console holds the stderr (human) and stdout (JSON) consoles - cli.py uses a single Cyclopts @app.default preserving the exact CLI syntax (inventory + variadic operations, faked subcommand dispatch, exec -- passthrough, -v counting, --user/--port aliases, PYINFRA_YES) - exceptions.py drops click.ClickException for Rich-rendered CliException - CLI test harness swaps click.testing.CliRunner for a Cyclopts-based run_cli - Shell completion installed via `pyinfra --install-completion` (Cyclopts); the old Click completion scripts are removed and docs/CHANGELOG updated stdout stays byte-for-byte JSON; all human output remains on stderr.
- debug-inventory renders a Rich table with syntax-highlighted, pretty-printed host data; --json output is highlighted on a terminal and plain when piped - de-duplicate host groups and tidy the table layout - make INVENTORY positional-only and organise options into ordered help groups - syntax-highlight the CLI help examples
…ling Replace the flat `--> ` phase logs with a Rich rendering of deploys. - Drop the `--> ` log prefix; render proposed changes and results as Rich trees/tables (file -> operation -> hosts) with per-op success/error/no-change counts and failed hosts listed under errored operations - Hierarchical live tree (TTY, non-JSON, all verbosity levels): phases (Loading, Connecting, Preparing, each operation) render as spinner -> check/cross rows with a parent progress bar (n/total); verbose detail lines (facts, command input/output) nest under the host nodes - New pyinfra_cli.routing attributes log/echo lines to hosts and records per-host errors so nothing interleaves with the live region - Deferred, aggregated failure prompt: during Prepare, hosts evaluate in parallel, so failures are collected and a single prompt (preceded by a red "Failed hosts" block with each host's error) is shown after the phase - Consistent status colours; dim the `@connector/` host-name prefix; restyle the bracket-free host log prefix - New api operation_host_skipped state callback; fire operation_end before fail_hosts so the tree finalises before prompting In piped/JSON modes the live tree is disabled and output stays clean/pure.
6b3c080 to
c6eb31b
Compare
wowi42
left a comment
There was a problem hiding this comment.
LGTM
This migrates the CLI from Click to Cyclopts for argument parsing and Rich for human-facing output, while keeping the existing command syntax and all CLI flags intact. The new output layer in pyinfra.api.output stays decoupled from Rich/Click so the core remains testable, and the live progress tree during deploys is a nice improvement. All CI passes, including the full end-to-end matrix.
The shell injection surface is clean: user input still flows through the same operation/fact code paths, and Cyclopts handles argument parsing without introducing any new quoting concerns. The _lenient_bool converter for PYINFRA_YES preserves the legacy Click behavior including on/off and empty-string-as-false. The host.print_prefix change from [hostname] to hostname (with color and dimmed connector prefix) is an internal API change, not a public contract, and the e2e tests were updated to match. The operation_end callback reordering (before fail_hosts instead of after) is deliberate so the progress tree can finalize its display before error handling prompts or raises.
One small thing: the _support_command registered as @app.command(name="--support") and the support parameter in cli() both handle --support. The parameter body (if support: print_support_info(); return) looks unreachable since Cyclopts routes --support to the subcommand. It is harmless dead code but worth cleaning up in a follow-up.
Fizzadar
left a comment
There was a problem hiding this comment.
Hi @noirbizarre - I'm SUPER excited by this PR! But I think there's a few things to address:
--serial/--no-waitloose all output under a TTY--diffalso drops output unless-vis set with a TTY- all host attributed logs from deploy code also dropped with TTY
This is also a significant change, I believe it would be better split into: click -> cyclopts, then Rich output / print and finally the live progress. This would make reviewing much easier as well if you don't mind 🙏
|
Hi @Fizzadar Thanks for the feedback; it was very useful. Especially the So there it is; I split this PR into 3 separate stacked PRs (I'll keep this one as reference):
Those PRs include fixes for the case you both highlighted. I also provided some recordings, including more of those cases. Important I tried to have as stable and consistent as possible intermediate state for the PRs. Note Also not that they are all targeting the |
|
@noirbizarre awesome, thank you! I think we can certainly merge Cyclopts migration on it's own first, then possibly the others together as you say. Going to focus on that PR first so we can start landing these changes ASAP! |
|
Closing this PR: the proposal and its details have been moved to the discussion at #1885 (comment) for further design discussion. The branch remains available if we decide to pick this up later. Thanks @noirbizarre for the thorough work and the detailed write-up. |
Migrates the pyinfra CLI from Click to Cyclopts for argument parsing and adopts Rich for all human-facing output, while keeping the existing command syntax and behaviour.
Highlights:
--help, colourised usage, shell completion, and a machine-readable--jsonmode (stdout stays pure JSON; all human output goes to stderr).debug-inventory: hosts rendered in a zebra-striped table with data shown askey: valuelines (nested values as JSON), type-coloured to match the JSON highlighter.Output is routed through a small pluggable layer in
pyinfra.api.output, decoupled from Rich/Click so the core stays testable. Existing behaviour, exit codes and command syntax are preserved; a few intentional UX changes (usage-error exit code,--versionoutput, confirm-prompt re-prompting) are documented in the commits.See #1885 for context (includes recordings and screenshots).