Add config settings#8600
Add config settings#8600solababs wants to merge 5 commits intosb-13032026-delete-branch-after-merge-ifc-2336from
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (4)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 Coding Plan
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 |
…-13032026-add-config-settings-ifc-2336
| delete_git_branch_after_merge: bool = Field( | ||
| default=False, | ||
| description="When enabled, the corresponding Git branch is deleted after the Infrahub branch is deleted. " | ||
| "Requires delete_branch_after_merge to be enabled.", | ||
| ) |
There was a problem hiding this comment.
probably more of a design-level question, but it seems like we should be able to delete a branch on git regardless of whether the branch is being automatically deleted after a merge or manually deleted later
There was a problem hiding this comment.
@ajtmccarty as seen here #8431 (comment), i think this was intentionally designed this way
Why
There is no configuration surface for controlling automatic branch deletion after merge. Without these settings, the feature cannot be safely rolled out — behavior needs to default to off and be explicitly opt-in.
This PR adds the two config flags that gate the feature, along with a dependency validator between them. It is Phase 1 of the full implementation plan.
What changed
delete_branch_after_merge: bool(defaultfalse) toMainSettings— when enabled, the Infrahub branch is automatically deleted after a successful merge.delete_git_branch_after_merge: bool(defaultfalse) toMainSettings— when enabled, the corresponding Git branch is also deleted; requiresdelete_branch_after_mergeto be enabled.@model_validatorthat raises at startup ifdelete_git_branch_after_merge=truewithoutdelete_branch_after_merge=true, preventing a misconfigured state.No behavioral changes beyond the new config fields — no deletion logic is wired up yet.
Suggested review order
backend/infrahub/config.py— the two new fields and the validatorbackend/tests/unit/test_config.py— unit testsHow to review
Focus on the validator logic in
config.py— specifically whether the error message and validation mode (after) are appropriate. The rest is straightforward field additions.No schema, API, or database changes in this PR.
How to test
Expected: all three new tests pass. Attempting to instantiate
MainSettings(delete_git_branch_after_merge=True, delete_branch_after_merge=False)raises aValidationErrorwith the messagerequires 'delete_branch_after_merge' to be enabled.Impact & rollout
false, so existing deployments are unaffected.MainSettings:delete_branch_after_mergeanddelete_git_branch_after_merge.Checklist
uv run towncrier create ...)