Add delete branch after merge#8613
Add delete branch after merge#8613solababs wants to merge 5 commits intosb-13032026-add-config-settings-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 |
Merging this PR will improve performance by 38.81%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_nodemanager_querypeers |
1.5 ms | 1.2 ms | +21.8% |
| ⚡ | test_schemabranch_process |
1,040.4 ms | 795.5 ms | +30.78% |
| ⚡ | test_get_schema |
327.3 ms | 251.7 ms | +30.03% |
| ⚡ | test_graphql_generate_schema |
516.1 ms | 371.8 ms | +38.81% |
| ⚡ | test_query_rel_many |
693.2 ms | 523.6 ms | +32.38% |
| ⚡ | test_query_rel_one |
662 ms | 501.1 ms | +32.1% |
| ⚡ | test_get_menu |
246 ms | 188.3 ms | +30.62% |
| ⚡ | test_query_one_model |
464.1 ms | 353.5 ms | +31.29% |
| ⚡ | test_base_schema_duplicate_CoreProposedChange |
2.1 ms | 1.7 ms | +23.66% |
| ⚡ | test_load_node_to_db_node_schema |
67.1 ms | 52.2 ms | +28.57% |
| ⚡ | test_schemabranch_duplicate |
7.2 ms | 5.5 ms | +31.28% |
| ⚡ | test_relationshipmanager_getpeer |
157.4 µs | 131 µs | +20.2% |
Comparing sb-13032026-delete-infrahub-branch-after-merge-ifc-2336 (3462605) with sb-13032026-add-config-settings-ifc-2336 (b07039d)
…026-delete-infrahub-branch-after-merge-ifc-2336
…026-delete-infrahub-branch-after-merge-ifc-2336
…026-delete-infrahub-branch-after-merge-ifc-2336
| parameters={"branch_name": obj.name}, | ||
| ) | ||
|
|
||
| if config.SETTINGS.main.delete_branch_after_merge and not obj.is_default and proposed_change_id is None: |
There was a problem hiding this comment.
why does the proposed_change_id need to be None here?
| await get_workflow().submit_workflow( | ||
| workflow=BRANCH_DELETE, | ||
| context=context, | ||
| parameters={"branch": source_branch.name}, |
There was a problem hiding this comment.
I'm guessing that the logic here is to prevent breaking open proposed changes, but any open PCs will be unmergeable once this PC finishes merging
maybe the right thing to do in this case is to set any open PCs on this branch to closed and then allow deleting it
Why
After a branch is merged, it typically has no further use but remains in the branch list until manually deleted. There is no way to automatically clean it up today.
This PR wires up the
delete_branch_after_mergeconfig setting (added in Phase 1) to the actual merge paths — both directBranchMergeand proposed change merge — so that the Infrahub branch is automatically deleted after a successful merge when the setting is enabled.What changed
backend/infrahub/core/branch/tasks.py— after a successfulBranchMerge, ifdelete_branch_after_merge=Trueand the branch is not the default branch and there is no associated proposed change, submits theBRANCH_DELETEworkflow.backend/infrahub/proposed_change/tasks.py— after a proposed change is merged, ifdelete_branch_after_merge=True, checks that no other open proposed changes exist for the source branch before submittingBRANCH_DELETE.backend/tests/functional/branch/test_branch_delete_after_merge.py— 4 functional tests covering: config-enabled standard merge triggers delete, config-disabled standard merge does not, proposed change merge triggers delete, and a skipped test for the multi-PC guard (multiple PCs on the same branch are not currently allowed).The default value of
delete_branch_after_mergeremainsFalse, so existing deployments are unaffected.Suggested review order
merge_branchchangemerge_proposed_changechangeHow to review
proposed_change/tasks.py: confirm the open-PC guard logic is correct — we query for open PCs on the source branch before deciding to delete. Worth checking whether the PC being merged is already in a non-open state at this point and would be excluded from the query.proposed_change_id is Noneguard inbranch/tasks.pyprevents a double-delete when a proposed change merge internally callsmerge_branch.How to test
Manual flow:
delete_branch_after_merge = trueininfrahub.toml, restart service.BranchMergeGraphQL mutation — branch should no longer appear in the branch list.delete_branch_after_merge = false(default) — branch is retained after merge.Impact & rollout
delete_branch_after_mergedefaults tofalse; existing behaviour is unchanged.delete_branch_after_mergeinMainSettings(introduced in Phase 1).Checklist
uv run towncrier create ...)