Add graph_version and status to branch
#600
Conversation
WalkthroughAdds a Pre-merge checks and finishing touches✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (5)
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 |
Deploying infrahub-sdk-python with
|
| Latest commit: |
5e8e304
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f84a9991.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://gma-20251105-branch-graph-ve.infrahub-sdk-python.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests.
@@ Coverage Diff @@
## infrahub-develop #600 +/- ##
====================================================
- Coverage 76.44% 69.67% -6.78%
====================================================
Files 112 112
Lines 10856 9453 -1403
Branches 2335 1879 -456
====================================================
- Hits 8299 6586 -1713
- Misses 1982 2366 +384
+ Partials 575 501 -74
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 34 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: 0
🧹 Nitpick comments (1)
infrahub_sdk/branch.py (1)
38-38: LGTM! Optional: Consider field ordering for consistency.The field is correctly added to the GraphQL query mapping. Optionally, you could reorder it to match the
BranchDatamodel definition (placing it betweenhas_schema_changesandorigin_branch) for better maintainability, though the current positioning is functionally equivalent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
changelog/+branch-graph-version.added.md(1 hunks)infrahub_sdk/branch.py(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.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/branch.py
⏰ 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). (7)
- GitHub Check: unit-tests (3.12)
- GitHub Check: unit-tests (3.10)
- GitHub Check: unit-tests (3.13)
- GitHub Check: unit-tests (3.9)
- GitHub Check: unit-tests (3.11)
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
changelog/+branch-graph-version.added.md (1)
1-1: LGTM!The changelog entry clearly documents the addition of the
graph_versionproperty to the Branch entity.infrahub_sdk/branch.py (1)
24-24: LGTM!The
graph_versionfield is properly typed as optional and will be backward compatible with older Infrahub versions that don't return this property.
| sync_with_git: bool | ||
| is_default: bool | ||
| has_schema_changes: bool | ||
| graph_version: int | None = None |
There was a problem hiding this comment.
We should probably also add status.
There was a problem hiding this comment.
Ah ok, we get that regardless with the query as seen in the integration tests..
gmazoyer
changed the title
graph_version to branchgraph_version and status to branch
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
infrahub_sdk/ctl/branch.py (1)
78-79: Same None handling issue applies here.The same concern about converting
Noneto"None"applies here for non-default branches.Apply the same fix as suggested for lines 62-63:
- str(branch.graph_version), + str(branch.graph_version) if branch.graph_version is not None else "-",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrahub_sdk/ctl/branch.py(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/ctl/branch.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/branch.py
⏰ 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). (7)
- GitHub Check: unit-tests (3.12)
- GitHub Check: unit-tests (3.13)
- GitHub Check: unit-tests (3.11)
- GitHub Check: unit-tests (3.10)
- GitHub Check: unit-tests (3.9)
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: documentation
🔇 Additional comments (1)
infrahub_sdk/ctl/branch.py (1)
49-50: LGTM! New columns added correctly.The new "Graph Version" and "Status" columns are properly added to the table.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
infrahub_sdk/ctl/branch.py (1)
76-76: Pre-existing bug: Wrong variable used for schema changes.This line uses
default_branch.has_schema_changesinstead ofbranch.has_schema_changes, causing all non-default branches to incorrectly display the default branch's schema changes status. This issue was already flagged in previous reviews.Apply this diff to fix the bug:
- "[green]True" if default_branch.has_schema_changes else "[#FF7F50]False", + "[green]True" if branch.has_schema_changes else "[#FF7F50]False",
🧹 Nitpick comments (1)
infrahub_sdk/ctl/branch.py (1)
62-63: Consider improving user-friendly display of graph_version and status.The current implementation has two display concerns:
Using a truthiness check on
graph_versionworks but is less explicit than checkingis not None. Agraph_versionof0(though unlikely) would incorrectly display as an empty string.The
statusenum will display as the raw enum name (e.g.,NEED_REBASE), which is not user-friendly.Apply this diff to improve both displays:
- str(default_branch.graph_version) if default_branch.graph_version else "", - default_branch.status, + str(default_branch.graph_version) if default_branch.graph_version is not None else "-", + default_branch.status.value.replace("_", " ").title() if default_branch.status else "-",Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
infrahub_sdk/branch.py(3 hunks)infrahub_sdk/ctl/branch.py(3 hunks)tests/unit/ctl/conftest.py(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- tests/unit/ctl/conftest.py
🚧 Files skipped from review as they are similar to previous changes (1)
- infrahub_sdk/branch.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.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/ctl/branch.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/branch.py
⏰ 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). (6)
- 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.10)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (1)
infrahub_sdk/ctl/branch.py (1)
49-50: LGTM! New columns added correctly.The two new columns "Graph Version" and "Status" are properly defined following the existing table column pattern.
This PR simply adds the
graph_versionandstatusproperties onBranchas they will be available in Infrahub 1.5.Summary by CodeRabbit