Conversation
github-actions
Bot
added
infrastructure
WalkthroughThis PR introduces a Click-based command-line interface for the reconcile module, decoupling operator reconciliation logic from direct script invocation. The operator-versions command is created as a CLI entry point, the core function is renamed to match the new pattern, and all callers are updated to use the new CLI interface. ChangesCLI Infrastructure and Caller Updates
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
reconcile/cli.py (1)
1-4: ⚡ Quick winMake the CLI import work in both execution modes.
Line 4's absolute import only resolves when
cli.pyis run as a loose script from thereconcile/directory. If this is meant to be an installed/package entry point, it will fail to import its sibling module.♻️ Suggested import fallback
-import click -import sys - -from operator_versions import reconcile as operator_versions_reconcile +import click +import sys + +try: + from .operator_versions import reconcile as operator_versions_reconcile +except ImportError: + from operator_versions import reconcile as operator_versions_reconcile🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@reconcile/cli.py` around lines 1 - 4, The import of operator_versions is absolute and only works when cli.py is run as a loose script; change it to try a relative import first and fall back to the absolute import so the entry point works both as a package and as a script. Update the import block in reconcile/cli.py to attempt "from .operator_versions import reconcile as operator_versions_reconcile" and on ImportError fall back to "from operator_versions import reconcile as operator_versions_reconcile", keeping the symbol operator_versions_reconcile unchanged so the rest of the file (e.g., any calls to operator_versions_reconcile) continues to work.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@reconcile/cli.py`:
- Around line 13-19: The CLI default for --operators is a dict literal ("{}")
which causes reconcile.operator_versions.init_ns_op_version_map() to crash when
it expects a list; change the click option default in operator_versions
(function operator_versions) from "{}" to "[]" so sys.argv =
["operator_versions.py", operators, dry_run] provides a list literal string that
operator_versions_reconcile() and init_ns_op_version_map() can ast.literal_eval
into a list; ensure any tests or callers expecting "{}" are updated accordingly.
- Around line 22-24: The mgmt_cluster_version command currently raises
NotImplementedError and will print a traceback; either remove or hide the
command until implemented or change the failure to a clean Click error: replace
the `@cli.command`() decorator usage or add hidden=True to it (e.g.,
`@cli.command`(hidden=True)) to hide the command, or keep the command but replace
raise NotImplementedError("mgmt-cluster-version") with raising a Click exception
(e.g., raise click.ClickException("mgmt-cluster-version is not implemented")) so
users receive a clean CLI error; update the mgmt_cluster_version function
accordingly and ensure click is imported if using ClickException.
---
Nitpick comments:
In `@reconcile/cli.py`:
- Around line 1-4: The import of operator_versions is absolute and only works
when cli.py is run as a loose script; change it to try a relative import first
and fall back to the absolute import so the entry point works both as a package
and as a script. Update the import block in reconcile/cli.py to attempt "from
.operator_versions import reconcile as operator_versions_reconcile" and on
ImportError fall back to "from operator_versions import reconcile as
operator_versions_reconcile", keeping the symbol operator_versions_reconcile
unchanged so the rest of the file (e.g., any calls to
operator_versions_reconcile) continues to work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b193ccde-9145-44f3-9728-f0e0d126c356
📒 Files selected for processing (6)
.gitignoreansible_pip_requirements.txtoperators/openshift-pipelines-operator-rh/operator_update_pipeline.yamlplaybooks/tasks/configure_operator.yamlreconcile/cli.pyreconcile/operator_versions.py
rporres
reviewed
May 4, 2026
rporres
reviewed
May 4, 2026
|
|
||
| @cli.command() | ||
| def mgmt_cluster_version(): | ||
| raise click.ClickException("mgmt-cluster-version is not implemented yet") |
Contributor
There was a problem hiding this comment.
Happy to implement this once this is merged.
Contributor
|
nice! ❤️ |
|
Tarball created: |
rporres
approved these changes
May 4, 2026
Contributor
rporres
left a comment
rporres
left a comment
There was a problem hiding this comment.
At some point we will need to add dependency scaffolding to our python stack (thinking about pyproject bits and such), but we can plan that further on the road
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
related to #294
this PR introduces (enclave-)reconcile, a python cli to reconcile components of enclave (and/or provisioned by encalve) from their current state to a desired state, as part of an upgrade flow.
Summary by CodeRabbit
New Features
Refactor
Chores