Skip to content

Add short options, -h/--help support, and command help text#515

Closed
rporres wants to merge 1 commit into
mainfrom
add-short-flags
Closed

Add short options, -h/--help support, and command help text#515
rporres wants to merge 1 commit into
mainfrom
add-short-flags

Conversation

@rporres

@rporres rporres commented Jun 17, 2026

Copy link
Copy Markdown
Contributor
  • Add HelpGroup (click.Group subclass) to utils.py that propagates -h/--help context settings to every registered subcommand
  • Add short options (-l, -n, -v, -N, -c, -u, -d, -t, -s, -H, -e, -r) across all CLI commands
  • Switch --dry-run from flag-pair (--dry-run/--no-dry-run) to is_flag; update configure_operator.yaml accordingly
  • Add docstrings to enclave, reconcile, operator-versions, and mgmt-cluster-version commands

- Add HelpGroup (click.Group subclass) to utils.py that propagates
  -h/--help context settings to every registered subcommand
- Add short options (-l, -n, -v, -N, -c, -u, -d, -t, -s, -H, -e, -r)
  across all CLI commands
- Switch --dry-run from flag-pair (--dry-run/--no-dry-run) to is_flag;
  update configure_operator.yaml accordingly
- Add docstrings to enclave, reconcile, operator-versions, and
  mgmt-cluster-version commands

Signed-off-by: Rafa Porres Molina <rporresm@redhat.com>
Assisted-by: Claude Code <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@rporres, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 5 minutes and 14 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1de44b5c-4242-45e1-9f20-2c966c9f8bc6

📥 Commits

Reviewing files that changed from the base of the PR and between a84e812 and 15937dd.

📒 Files selected for processing (8)
  • playbooks/tasks/configure_operator.yaml
  • pyproject.toml
  • src/enclave/cli.py
  • src/enclave/reconcile/cli.py
  • src/enclave/tools/cli.py
  • src/enclave/utils.py
  • src/tests/test_cli.py
  • src/tests/test_tools_cli.py
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch add-short-flags

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added infrastructure Infrastructure setup (VMs, networks) operators Operator installation/config labels Jun 17, 2026
@rporres

rporres commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai please review

@coderabbitai

coderabbitai Bot commented Jun 18, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Get PR details
gh pr view 515 --repo rh-ecosystem-edge/enclave --json title,body,files,additions,deletions,changedFiles

echo "---DIFF---"
gh pr diff 515 --repo rh-ecosystem-edge/enclave

Length of output: 11092


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Check full content of main cli.py
fd -e py cli.py --exec cat {}

Length of output: 17886



@rporres: Here is the security-focused review of PR #515.


PR Review: Add short options, -h/--help support, and command help text

Overall risk: LOW-MEDIUM. The changes are clean and well-structured. Two items deserve attention before merging.


🔴 Medium – --no-dry-run removal is a breaking change

Files: src/enclave/reconcile/cli.py, playbooks/tasks/configure_operator.yaml

Switching from --dry-run/--no-dry-run (flag pair) to --dry-run (is_flag) silently breaks any external caller that currently passes --no-dry-run. They will get Error: No such option: --no-dry-run at runtime — including CI pipelines, other Ansible playbooks, or runbooks outside this repository. The in-tree playbook was updated, but external consumers are invisible to this PR.

Recommended mitigation: Keep --no-dry-run as a hidden, deprecated no-op for at least one release cycle, or document this as an intentional breaking change in the commit/release notes.

`@click.option`(
    "--no-dry-run",
    is_flag=True,
    default=False,
    hidden=True,
    expose_value=False,
    is_eager=True,
    help="Deprecated no-op; --dry-run now defaults to False.",
)

🔴 Medium – Ansible playbook now relies on implicit default

File: playbooks/tasks/configure_operator.yaml

-      {% for csv in ... %}--csv-name "{{ csv }}" {% endfor %}\
-      --no-dry-run
+      {% for csv in ... %}--csv-name "{{ csv }}" {% endfor %}

The previous --no-dry-run was an explicit safety assertion ("apply changes in production"). Now production correctness relies on default=False never changing. If someone sets default=True for testing and forgets to revert, the playbook would silently do nothing.

Impact: Dry-run accidentally left on in production means cluster state diverges silently.


🟡 Low – HelpGroup.add_command() not overridden (latent gap)

File: src/enclave/utils.py

src/enclave/cli.py wires sub-CLIs via:

cli.add_command(reconcile_cli, name="reconcile")
cli.add_command(tools_cli, name="tools")

HelpGroup overrides command() and group() (decorator-style registration) but not add_command(). Currently harmless because reconcile_cli and tools_cli are already HelpGroup instances and carry their own context_settings. However, any future cli.add_command(some_plain_click_command) will silently miss -h/--help.

Suggestion: Override add_command() defensively:

def add_command(self, cmd: click.Command, name: str | None = None) -> None:
    if cmd.context_settings is None:
        cmd.context_settings = dict(_HELP_CONTEXT)
    else:
        cmd.context_settings.setdefault("help_option_names", _HELP_CONTEXT["help_option_names"])
    super().add_command(cmd, name)

🟡 Low – No tests exercise -h (the primary new feature)

Files: src/tests/test_cli.py, src/tests/test_tools_cli.py

Tests still use --help. Since -h is the headline feature of HelpGroup, add at least:

def test_cli_short_help() -> None:
    result = CliRunner().invoke(cli, ["-h"])
    assert result.exit_code == 0
    assert "Reconcile management cluster state" in result.output

def test_subcommand_short_help() -> None:
    result = CliRunner().invoke(cli, ["operator-versions", "-h"])
    assert result.exit_code == 0

🔵 Info – ANN401 suppression is file-wide

File: pyproject.toml

"src/enclave/utils.py" = [
    "ANN401",  # HelpGroup mirrors click.Group's *args/**kwargs interface
]

The suppression covers the entire file. Future functions in utils.py that incorrectly use Any won't be flagged. Consider scoping this with a # noqa: ANN401 inline comment on the two HelpGroup method signatures instead.


✅ What looks good

Item Status
HelpGroup.__init__ uses setdefault — won't clobber explicit context_settings
-H (uppercase) for --hostname correctly avoids -h collision
-v reuse across different subcommands (operator-versions, mgmt-cluster-version) — no conflict
Trailing \ removed from configure_operator.yaml for-loop line — shell continuation correct
Docstrings are meaningful and actionable
HelpGroup.group() override present for future nested groups

@rporres rporres closed this Jun 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

infrastructure Infrastructure setup (VMs, networks) operators Operator installation/config

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant