-
Notifications
You must be signed in to change notification settings - Fork 6
Merge develop into infrahub-develop #612
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Fatih Acar <fatih@opsmill.com>
Merge stable into develop
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## infrahub-develop #612 +/- ##
====================================================
+ Coverage 75.13% 75.26% +0.12%
====================================================
Files 112 113 +1
Lines 10972 11033 +61
Branches 2368 2382 +14
====================================================
+ Hits 8244 8304 +60
Misses 2138 2138
- Partials 590 591 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
WalkthroughThis pull request introduces a new Infrahub CLI task management feature. It adds a Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/docs/infrahubctl/infrahubctl-task.mdx(1 hunks)docs/sidebars-infrahubctl.ts(1 hunks)infrahub_sdk/ctl/cli_commands.py(2 hunks)infrahub_sdk/ctl/task.py(1 hunks)infrahub_sdk/task/models.py(1 hunks)tests/unit/ctl/test_task_app.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
When implementing Infrahub checks, subclass InfrahubCheck and override validate(data); do not implement or rely on a check() method
Files:
infrahub_sdk/task/models.pyinfrahub_sdk/ctl/cli_commands.pytests/unit/ctl/test_task_app.pyinfrahub_sdk/ctl/task.py
infrahub_sdk/ctl/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
infrahub_sdk/ctl/**/*.py: Build CLI commands with Typer
Organize and keep all CLI commands within infrahub_sdk/ctl/
Files:
infrahub_sdk/ctl/cli_commands.pyinfrahub_sdk/ctl/task.py
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.{md,mdx}: Follow the Diataxis framework and classify docs as Tutorials, How-to guides, Explanation, or Reference
Structure How-to guides with required frontmatter and sections: introduction, prerequisites, step-by-step steps, validation, related resources
Structure Topics (Explanation) docs with introduction, concepts & definitions, background & context, architecture & design, connections, further reading
Use a professional, concise, informative tone with consistent structure across documents
Use proper language tags on all code blocks
Include both async and sync examples where applicable using the Tabs component
Add validation steps to guides to confirm success and expected outputs
Use callouts for warnings, tips, and important notes
Define new terms on first use and use domain-relevant terminology consistent with Infrahub’s model/UI
Conform to markdown style rules in .markdownlint.yaml and Vale styles in .vale/styles/
Files:
docs/docs/infrahubctl/infrahubctl-task.mdx
tests/unit/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Place and write unit tests under tests/unit/ (isolated component tests)
Files:
tests/unit/ctl/test_task_app.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use the custom pytest plugin (infrahub_sdk.pytest_plugin) fixtures for clients, configuration, and Infrahub-specific testing
Files:
tests/unit/ctl/test_task_app.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T13:35:12.998Z
Learning: Applies to infrahub_sdk/ctl/**/*.py : Build CLI commands with Typer
📚 Learning: 2025-08-24T13:35:12.998Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T13:35:12.998Z
Learning: Applies to infrahub_sdk/ctl/**/*.py : Organize and keep all CLI commands within infrahub_sdk/ctl/
Applied to files:
docs/sidebars-infrahubctl.tsinfrahub_sdk/ctl/cli_commands.pyinfrahub_sdk/ctl/task.py
📚 Learning: 2025-08-24T13:35:12.998Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T13:35:12.998Z
Learning: Applies to infrahub_sdk/ctl/**/*.py : Build CLI commands with Typer
Applied to files:
docs/sidebars-infrahubctl.tsinfrahub_sdk/ctl/cli_commands.pyinfrahub_sdk/ctl/task.py
📚 Learning: 2025-08-24T13:35:12.998Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T13:35:12.998Z
Learning: Applies to infrahub_sdk/config.py : Environment variables for configuration must use the INFRAHUB_ prefix
Applied to files:
docs/sidebars-infrahubctl.ts
🧬 Code graph analysis (2)
tests/unit/ctl/test_task_app.py (1)
infrahub_sdk/async_typer.py (1)
runner(17-18)
infrahub_sdk/ctl/task.py (4)
infrahub_sdk/async_typer.py (2)
AsyncTyper(11-31)command(29-31)infrahub_sdk/task/models.py (3)
TaskFilter(65-76)Task(32-62)TaskState(9-18)infrahub_sdk/ctl/client.py (1)
initialize_client(10-25)infrahub_sdk/ctl/utils.py (2)
catch_exception(78-106)init_logging(42-50)
🪛 LanguageTool
docs/docs/infrahubctl/infrahubctl-task.mdx
[style] ~33-~33: To form a complete sentence, be sure to include a subject.
Context: ...s, --state TEXT: Filter by task state. Can be provided multiple times. * --limit ...
(MISSING_IT_THERE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: unit-tests (3.10)
- GitHub Check: unit-tests (3.13)
- GitHub Check: unit-tests (3.9)
- GitHub Check: unit-tests (3.12)
- GitHub Check: unit-tests (3.11)
- GitHub Check: unit-tests (3.13)
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: Cloudflare Pages
🔇 Additional comments (8)
infrahub_sdk/task/models.py (1)
52-60: LGTM! Defensive guards improve robustness.The explicit key presence checks before processing related_nodes and logs prevent KeyError exceptions and handle missing or null values gracefully. This defensive pattern is appropriate for optional GraphQL response fields and aligns well with the CLI usage introduced in this PR.
infrahub_sdk/ctl/cli_commands.py (1)
36-36: LGTM! Task CLI module properly integrated.The new task CLI module is imported and registered following the established pattern used for other CLI commands (branch, schema, etc.). Based on learnings.
Also applies to: 69-69
docs/sidebars-infrahubctl.ts (1)
27-27: LGTM! Documentation navigation updated correctly.The task command documentation is properly added to the sidebar in alphabetical order.
docs/docs/infrahubctl/infrahubctl-task.mdx (1)
1-41: LGTM! Comprehensive CLI documentation.The documentation clearly describes the task command and its list subcommand with all available options. Follows the established format for CLI reference documentation. As per coding guidelines.
tests/unit/ctl/test_task_app.py (1)
1-100: LGTM! Comprehensive test coverage.The test suite thoroughly validates the task list CLI command including standard output, JSON formatting, state filtering, and error handling for invalid states. Tests properly use HTTPXMock for API stubbing and Typer's CliRunner for CLI testing. As per coding guidelines.
infrahub_sdk/ctl/task.py (3)
25-39: LGTM! Clean state parsing with proper error handling.The function correctly normalizes state inputs and converts them to TaskState enum values. Error handling with
typer.BadParameterprovides clear user feedback when invalid states are provided.
42-69: LGTM! Well-structured table rendering.The table rendering function properly handles both populated and empty task lists, with appropriate formatting for optional fields (progress, workflow, branch) and clear messaging when no tasks are found.
72-110: LGTM! Command implementation follows established patterns.The list command is well-structured with proper async handling, comprehensive options (filtering, pagination, output formatting), and clean separation of concerns. Implementation correctly uses AsyncTyper, initialize_client, and the catch_exception decorator per coding guidelines. Based on learnings.
| from ..task.manager import TaskFilter | ||
| from ..task.models import Task, TaskState |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
Verify TaskFilter import path.
TaskFilter is imported from ..task.manager but the relevant code snippets show it's defined in infrahub_sdk/task/models.py. Please confirm whether TaskFilter is re-exported from manager or if the import should be from ..task.models instead.
🏁 Script executed:
#!/bin/bash
# Verify where TaskFilter is defined and exported
echo "=== Checking TaskFilter definition ==="
rg -n "^class TaskFilter" --type=py
echo -e "\n=== Checking TaskFilter exports from manager ==="
rg -n "TaskFilter" infrahub_sdk/task/manager.py
echo -e "\n=== Checking TaskFilter exports from models ==="
rg -n "^from.*TaskFilter|^import.*TaskFilter|__all__.*TaskFilter" infrahub_sdk/task/models.pyLength of output: 1015
🏁 Script executed:
#!/bin/bash
# Check if manager.py explicitly defines __all__
echo "=== Checking for __all__ in manager.py ==="
rg -n "__all__" infrahub_sdk/task/manager.py
# Check import patterns across the codebase for comparison
echo -e "\n=== Checking import patterns from task module ==="
rg -n "from.*task\.(manager|models)" --type=py | head -20
# Verify the exact import context in manager.py
echo -e "\n=== First 25 lines of manager.py ==="
head -25 infrahub_sdk/task/manager.pyLength of output: 1346
Update import to use consistent path from models.
TaskFilter is defined in infrahub_sdk/task/models.py and should be imported directly from there. While manager.py imports and uses TaskFilter, it does not explicitly re-export it (no __all__ definition), and the codebase consistently imports it from models elsewhere. Change line 10 from:
from ..task.manager import TaskFilterto:
from ..task.models import TaskFilterThis aligns with the pattern used in tests and makes the import source consistent with the other items on line 11.
🤖 Prompt for AI Agents
In infrahub_sdk/ctl/task.py around lines 10 to 11, the TaskFilter import should
come from the models module to match the codebase pattern; replace the current
import from ..task.manager with an import from ..task.models so both TaskFilter,
Task, and TaskState are imported from infrahub_sdk/task/models.py, ensuring
consistent import sources and avoiding reliance on manager re-exports.
Summary by CodeRabbit
New Features
infrahubctl task listcommand for viewing and managing tasks with state filtering, pagination options, and support for both table and JSON output formats.Documentation