Move CRUD commands under object subgroup#931
Conversation
The get, create, update, and delete commands were registered at the root of infrahubctl, which felt out of place next to unrelated top-level commands like check, render, and transform. They naturally belong under `infrahubctl object` alongside load and validate, since they all operate on Infrahub objects. Tests updated accordingly.
Deploying infrahub-sdk-python with
|
| Latest commit: |
d50bdc5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c5b38e37.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://gma-20260409-crud-object.infrahub-sdk-python.pages.dev |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
WalkthroughThis pull request reorganizes the infrahubctl CLI command structure by moving 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
github-actions
bot
added
the
type/documentation
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## develop #931 +/- ##
===========================================
- Coverage 82.56% 81.09% -1.47%
===========================================
Files 134 132 -2
Lines 13194 10920 -2274
Branches 2287 1653 -634
===========================================
- Hits 10893 8856 -2037
+ Misses 1652 1514 -138
+ Partials 649 550 -99
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 17 files with indirect coverage changes 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
dev/knowledge/cli-architecture.md (1)
16-24: Consider varying sentence structure.The content is accurate and helpful, but three successive sentences begin with "For" (lines 18, 24). Consider rewording for better flow.
📝 Suggested rewording
-For a **root command**, define the function in the appropriate module and register it in `cli_commands.py`: +To add a **root command**, define the function in the appropriate module and register it in `cli_commands.py`: ```python app.command(name="mycommand")(my_function)-For a subcommand, add it to the relevant group's module. For example, object subcommands go in
infrahub_sdk/ctl/object.pyor in dedicated files underinfrahub_sdk/ctl/commands/and are registered on the object app.
+When adding a subcommand, place it in the relevant group's module. Object subcommands, for instance, go ininfrahub_sdk/ctl/object.pyor in dedicated files underinfrahub_sdk/ctl/commands/and are registered on the object app.</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@dev/knowledge/cli-architecture.mdaround lines 16 - 24, Update the wording
to avoid repetitive sentence starts: change the second "For a subcommand"
sentence to something like "When adding a subcommand, place it in the
relevant group's module," and keep the examples referencing
infrahub_sdk/ctl/object.py and infrahub_sdk/ctl/commands/ as-is; also ensure the
root command example using app.command(name="mycommand")(my_function) remains
unchanged while the surrounding sentences flow variedly.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@docs/docs/infrahubctl/infrahubctl-object.mdx:
- Around line 33-37: The example CLI invocations in the generated docs still use
root-level commands (e.g., "infrahubctl create InfraDevice ..."); update the
source CLI help/docstrings to use the object subgroup (e.g., "infrahubctl object
create InfraDevice --set ...") so examples become "infrahubctl object
create/get/update/delete ...", then re-run the docs generator (uv run invoke
docs-generate) to regenerate docs/docs/infrahubctl/*.mdx; specifically update
the CLI command definitions that produce the examples for the
create/get/update/delete flows so the examples emitted for InfraDevice include
the "object" subgroup.
Nitpick comments:
In@dev/knowledge/cli-architecture.md:
- Around line 16-24: Update the wording to avoid repetitive sentence starts:
change the second "For a subcommand" sentence to something like "When adding
a subcommand, place it in the relevant group's module," and keep the
examples referencing infrahub_sdk/ctl/object.py and infrahub_sdk/ctl/commands/
as-is; also ensure the root command example using
app.command(name="mycommand")(my_function) remains unchanged while the
surrounding sentences flow variedly.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `4cd27e79-6294-4ce8-8b57-4a3e124fcdbe` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 95ceea824f05ab23d1b0aa33e1724c7305c6f269 and 0043ac0a1443c244842b8f74234689c99216d3ab. </details> <details> <summary>📒 Files selected for processing (14)</summary> * `AGENTS.md` * `dev/knowledge/cli-architecture.md` * `dev/knowledge/doc-generation.md` * `docs/docs/infrahubctl/infrahubctl-create.mdx` * `docs/docs/infrahubctl/infrahubctl-delete.mdx` * `docs/docs/infrahubctl/infrahubctl-get.mdx` * `docs/docs/infrahubctl/infrahubctl-object.mdx` * `docs/docs/infrahubctl/infrahubctl-update.mdx` * `infrahub_sdk/ctl/cli_commands.py` * `infrahub_sdk/ctl/object.py` * `tests/unit/ctl/commands/test_create.py` * `tests/unit/ctl/commands/test_delete.py` * `tests/unit/ctl/commands/test_get.py` * `tests/unit/ctl/commands/test_update.py` </details> <details> <summary>💤 Files with no reviewable changes (5)</summary> * docs/docs/infrahubctl/infrahubctl-update.mdx * docs/docs/infrahubctl/infrahubctl-delete.mdx * docs/docs/infrahubctl/infrahubctl-create.mdx * docs/docs/infrahubctl/infrahubctl-get.mdx * infrahub_sdk/ctl/cli_commands.py </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/integration/test_enduser_cli.py (1)
195-205:⚠️ Potential issue | 🟠 MajorAdd teardown/isolation for created and mutated test objects.
test_create_inlinepersists"Integration Test Person"andtest_update_inlinemutates shared fixture data without restoration. This keeps state between tests and can make reruns or order-randomized execution flaky.♻️ Suggested direction (fixture with cleanup)
+ `@pytest.fixture` + async def created_person_name( + self, client: InfrahubClient, base_dataset: None + ) -> Generator[str, None, None]: + name = "Integration Test Person" + try: + yield name + finally: + node = await client.get(kind="TestingPerson", id=name, raise_when_missing=False) + if node is not None: + await node.delete() - def test_create_inline(self, base_dataset: None) -> None: + def test_create_inline(self, created_person_name: str, base_dataset: None) -> None: result = runner.invoke( app, - ["object", "create", "TestingPerson", "--set", "name=Integration Test Person", "--set", "height=190"], + ["object", "create", "TestingPerson", "--set", f"name={created_person_name}", "--set", "height=190"], )Use the same pattern for update (either update a throwaway record or restore original value in teardown).
As per coding guidelines:
tests/integration/**/*.py: Clean up resources in integration tests.Also applies to: 215-224
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/test_enduser_cli.py` around lines 195 - 205, The tests persist and mutate shared data: ensure isolation by cleaning up or using throwaway records—modify test_create_inline (and test_update_inline) to create a uniquely-named record (avoid "Integration Test Person") or register a fixture teardown that deletes or restores the created/updated object via the InfrahubClient; locate the tests test_create_inline_verify, test_create_inline, and test_update_inline and use client.get/client.delete or an explicit fixture (e.g., a tmp_object fixture using base_dataset and InfrahubClient) to remove the created object in teardown so each test leaves no persistent state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@tests/integration/test_enduser_cli.py`:
- Around line 195-205: The tests persist and mutate shared data: ensure
isolation by cleaning up or using throwaway records—modify test_create_inline
(and test_update_inline) to create a uniquely-named record (avoid "Integration
Test Person") or register a fixture teardown that deletes or restores the
created/updated object via the InfrahubClient; locate the tests
test_create_inline_verify, test_create_inline, and test_update_inline and use
client.get/client.delete or an explicit fixture (e.g., a tmp_object fixture
using base_dataset and InfrahubClient) to remove the created object in teardown
so each test leaves no persistent state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: faf0c781-d277-4cde-b934-7713dad6c7ec
📒 Files selected for processing (1)
tests/integration/test_enduser_cli.py
ogenstad
left a comment
ogenstad
left a comment
There was a problem hiding this comment.
Thanks! LGTM, as a future refactoring I think it could make sense to also move the commands into some other module so that they are grouped together.
2 participants
Why & What
The get, create, update, and delete commands were registered at the root of infrahubctl, which felt out of place next to unrelated top-level commands like check, render, and transform. They naturally belong under
infrahubctl objectalongside load and validate, since they all operate on Infrahub objects. Tests updated accordingly.Checklist
uv run towncrier create ...)Summary by CodeRabbit
Documentation
get,create,update, anddeletecommands now accessed via theobjectsubcommand (e.g.,infrahubctl object get).Breaking Changes
get,create,update,delete) moved toinfrahubctl objectsubcommand group.