fix: use configured default_branch in generators, transforms, checks - #1293
Open
pthmas wants to merge 3 commits into
Open
fix: use configured default_branch in generators, transforms, checks#1293pthmas wants to merge 3 commits into
pthmas wants to merge 3 commits into
Conversation
…hecks `InfrahubOperation.branch_name` and `InfrahubCheck.branch_name` resolved the branch from the local Git checkout whenever no explicit branch was passed, ignoring the client's resolved default branch. Runs against a configured `default_branch` therefore failed with `BranchNotFoundError` when the local Git branch did not exist in Infrahub. Resolve the branch from `client.default_branch` instead, which already honours the `default_branch_from_git` config flag, so the Git branch is only used when that flag is enabled.
Deploying infrahub-sdk-python with
|
| Latest commit: |
01712fb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5128c29f.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://issue-1290.infrahub-sdk-python.pages.dev |
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## stable #1293 +/- ##
==========================================
+ Coverage 84.24% 84.27% +0.03%
==========================================
Files 147 147
Lines 13066 13040 -26
Branches 1940 1928 -12
==========================================
- Hits 11007 10990 -17
+ Misses 1494 1489 -5
+ Partials 565 561 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
`ConfigBase.default_infrahub_branch` always resolved the local Git branch from the process working directory. Operations and checks are rooted at their own `root_directory`, so with `default_branch_from_git` enabled they could resolve against a different repository than the one they run in. Add `ConfigBase.get_default_infrahub_branch(directory=...)`, which the existing property now delegates to, and pass `root_directory` from `InfrahubOperation` and `InfrahubCheck`. Stub the Git lookup in the branch resolution tests. They previously compared against `get_branch()` evaluated at assert time, which depends on the state of the local checkout and fails outright when it has no active branch, as in CI.
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Re-trigger cubic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Running
infrahubctl generator,transformorcheckwithout--branchtargeted thelocal Git checkout's active branch instead of the configured
default_branch, failing withBranchNotFoundErrorwhenever that Git branch did not exist in Infrahub. The fallback thedefault_branch_from_gitflag exists to gate was applied unconditionally, overriding anexplicitly configured
INFRAHUB_DEFAULT_BRANCH.Goal: with
default_branch_from_gitunset orfalse, an operation with no explicit branchtargets the configured
default_branch— the same branch the rest of the command already uses.Non-goals:
Config.clonedrops an explicitbranchargument whendefault_branch_from_gitis enabled. Pre-existing, out of scope here, will be filed separately.
Closes #1290
What changed
Behavioral changes:
default_branch.default_branch_from_gitis enabled.infrahubctl generatoris no longer internally inconsistent: the GraphQL query and theschema fetch resolve to the same branch.
Implementation notes:
InfrahubOperation.__init__resolves the branch once asbranch or client.default_branch.client.default_branchis already the output ofConfigBase.default_infrahub_branch, whichmakes the
default_branch_from_gitdecision — so the flag stays honoured in exactly one placerather than being reimplemented. Same pattern already used in
ctl/cli_commands.py:386.branch_namebecomes a plain accessor; theGitRepoManagerlookup and theself.gitattribute are removed from
operation.py.InfrahubCheck.branch_namegets the same treatment. Its client is optional, so with no client(no config to consult) it falls back to the existing
get_branch()helper.InfrahubGeneratorandInfrahubTransformbothinherit from
InfrahubOperation, as does every library consumer.What stayed the same:
branch_nameandroot_directoryare unchanged.default_branch_from_git=truebehaviour is unchanged.How to review
Start with
infrahub_sdk/operation.py— the whole fix isself.branch = branch or client.default_branch.infrahub_sdk/checks.pyis the same change with an extra no-client arm.How to test
Regression coverage: explicit branch wins / no branch + flag off → configured
default_branch/no branch + flag on → local Git branch. Both new
default_branchassertions fail on thepre-fix code.
Impact & rollout
without setting
default_branch_from_git=truenow gets the configureddefault_branch(default
main) instead of their checkout branch. That is the documented contract; noted inthe changelog entry.
INFRAHUB_DEFAULT_BRANCHandINFRAHUB_DEFAULT_BRANCH_FROM_GITnow behave as documented.
Checklist
changelog/1290.fixed.md)Summary by cubic
Fixes generators, transforms, and checks to target the configured
default_branchwhen no explicit branch is passed, instead of falling back to the local Git checkout branch, which previously causedBranchNotFoundErrorwhen that branch didn't exist in Infrahub.Behavior changes
default_branch_from_gitis enabled, and is resolved from the operation'sroot_directoryrather than the process working directory.infrahubctl generatornow resolves the same branch for its GraphQL query and schema fetch.branch_nameandroot_directoryare unchanged.Anyone relying on the implicit Git fallback without
default_branch_from_gitnow gets the configureddefault_branch(defaultmain) instead of their checkout branch. No config or env changes required. Closes #1290.Written for commit 01712fb. Summary will update on new commits.