Add flags to CoreGeneratorDefinition to control when each generator should be executed
#578
Conversation
WalkthroughThe change introduces two boolean configuration parameters—execute_in_proposed_change and execute_after_merge—across schema, protocols, generator, and control layers. These fields are added to InfrahubGeneratorDefinitionConfig and CoreGeneratorDefinition (and sync), wired through ctl/generator.py into InfrahubGenerator’s constructor, stored on the instance, and used at runtime to select a group type dynamically: "CoreGeneratorGroup" when execute_after_merge is true, otherwise "CoreGeneratorAwareGroup". New protocol classes CoreGeneratorAwareGroup and CoreGeneratorAwareGroupSync are added. CoreGenericRepository and its sync variant gain a groups_objects relationship field. No other control flow changes. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## infrahub-develop #578 +/- ##
====================================================
- Coverage 75.68% 75.27% -0.41%
====================================================
Files 108 108
Lines 10276 9375 -901
Branches 2130 1864 -266
====================================================
- Hits 7777 7057 -720
+ Misses 1944 1823 -121
+ Partials 555 495 -60
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 8 files with indirect coverage changes 🚀 New features to boost your workflow:
|
Deploying infrahub-sdk-python with
|
| Latest commit: |
0f5a1df
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://89dab369.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://dga-20250909-generator-flags.infrahub-sdk-python.pages.dev |
![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/ctl/generator.py (1)
53-110: Consider extracting shared generator execution logic.Both code paths (lines 61-72 and 92-110) contain duplicated logic for:
- Generator class instantiation with similar parameters
- Client schema initialization (
await generator._init_client.schema.all(branch=generator.branch_name))- Generator execution (
await generator.run(identifier=generator_config.name, data=data))Consider extracting this into a helper function to reduce duplication and improve maintainability.
Example refactor:
+async def _execute_generator( + generator_class, + generator_config, + client, + branch: str | None, + params: dict, + data: dict, +) -> None: + generator = generator_class( + query=generator_config.query, + client=client, + branch=branch or "", + params=params, + convert_query_response=generator_config.convert_query_response, + execute_in_proposed_change=generator_config.execute_in_proposed_change, + execute_after_merge=generator_config.execute_after_merge, + infrahub_node=InfrahubNode, + ) + await generator._init_client.schema.all(branch=generator.branch_name) + await generator.run(identifier=generator_config.name, data=data) + async def run( generator_name: str, path: str, # noqa: ARG001 debug: bool, list_available: bool, branch: str | None = None, variables: Optional[list[str]] = None, ) -> None: # ... existing code ... if variables_dict: data = execute_graphql_query( query=generator_config.query, variables_dict=variables_dict, branch=branch, debug=False, repository_config=repository_config, ) - generator = generator_class( - query=generator_config.query, - client=client, - branch=branch or "", - params=variables_dict, - convert_query_response=generator_config.convert_query_response, - execute_in_proposed_change=generator_config.execute_in_proposed_change, - execute_after_merge=generator_config.execute_after_merge, - infrahub_node=InfrahubNode, - ) - await generator._init_client.schema.all(branch=generator.branch_name) - await generator.run(identifier=generator_config.name, data=data) + await _execute_generator( + generator_class=generator_class, + generator_config=generator_config, + client=client, + branch=branch, + params=variables_dict, + data=data, + ) else: # ... existing code for targets ... for member in targets._get_relationship_many(name="members").peers: # ... existing parameter setup ... - generator = generator_class( - query=generator_config.query, - client=client, - branch=branch or "", - params=params, - convert_query_response=generator_config.convert_query_response, - execute_in_proposed_change=generator_config.execute_in_proposed_change, - execute_after_merge=generator_config.execute_after_merge, - infrahub_node=InfrahubNode, - ) data = execute_graphql_query( query=generator_config.query, variables_dict=check_parameter, branch=branch, debug=False, repository_config=repository_config, ) - await generator._init_client.schema.all(branch=generator.branch_name) - await generator.run(identifier=generator_config.name, data=data) + await _execute_generator( + generator_class=generator_class, + generator_config=generator_config, + client=client, + branch=branch, + params=params, + data=data, + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrahub_sdk/ctl/generator.py(2 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/generator.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/generator.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: integration-tests-latest-infrahub
- 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: unit-tests (3.12)
🔇 Additional comments (2)
infrahub_sdk/ctl/generator.py (2)
67-68: LGTM! New flags correctly passed to generator.The
execute_in_proposed_changeandexecute_after_mergeflags are properly passed from the generator configuration to the generator class constructor.
98-99: LGTM! Flags consistently applied in targets path.The new execution flags are correctly passed to the generator constructor in the targets iteration path, maintaining consistency with the variables_dict code path.
dgarros
force-pushed
the
dga-20250909-generator-flags
branch
from
ce91f11 to
c3dc786
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
infrahub_sdk/generator.py (2)
19-25: Consider removing redundant class-level attribute declarations.These class-level type annotations duplicate the typing already present in
__init__(lines 27-41). As noted in the previous review comment, they provide limited value and may cause confusion if there's a mismatch between the class-level hints and the actual initialization.If you decide to keep them for documentation purposes, consider adding a comment explaining that these are instance attributes initialized in
__init__.
96-99: LGTM, but consider adding a clarifying comment.The group type selection logic is correct: generators executing after merge use
CoreGeneratorGroup, while those executing in proposed changes useCoreGeneratorAwareGroup. However, the relationship between the flag name and the group type choice is not immediately obvious.Consider adding a brief comment explaining the distinction:
+ # Use CoreGeneratorAwareGroup for generators running in proposed changes, + # CoreGeneratorGroup for post-merge execution group_type = "CoreGeneratorGroup" if self.execute_after_merge else "CoreGeneratorAwareGroup"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
infrahub_sdk/ctl/generator.py(2 hunks)infrahub_sdk/generator.py(4 hunks)infrahub_sdk/operation.py(1 hunks)infrahub_sdk/protocols.py(6 hunks)infrahub_sdk/schema/repository.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- infrahub_sdk/ctl/generator.py
- infrahub_sdk/operation.py
🧰 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/schema/repository.pyinfrahub_sdk/protocols.pyinfrahub_sdk/generator.py
🧬 Code graph analysis (2)
infrahub_sdk/protocols.py (2)
infrahub_sdk/node/relationship.py (2)
RelationshipManager(100-220)RelationshipManagerSync(223-343)infrahub_sdk/protocols_base.py (1)
BooleanOptional(147-148)
infrahub_sdk/generator.py (2)
infrahub_sdk/client.py (3)
request_context(188-189)request_context(192-193)start_tracking(195-214)infrahub_sdk/context.py (1)
RequestContext(10-13)
⏰ 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)
- GitHub Check: unit-tests (3.13)
- GitHub Check: unit-tests (3.12)
- GitHub Check: unit-tests (3.10)
- GitHub Check: unit-tests (3.11)
- GitHub Check: integration-tests-latest-infrahub
🔇 Additional comments (4)
infrahub_sdk/schema/repository.py (1)
99-106: LGTM!The new boolean flags are well-defined with sensible defaults that maintain backward compatibility (generators execute in both phases by default). The field names and descriptions clearly communicate their purpose.
infrahub_sdk/protocols.py (3)
134-134: LGTM!The new
groups_objectsrelationship follows the existing pattern for relationship definitions onCoreGenericRepository.
359-360: LGTM!The new
CoreGeneratorAwareGroupclass is correctly defined as a protocol subclass ofCoreGroup, matching the pattern used by other group variants in this file (e.g.,CoreGeneratorGroup,CoreStandardGroup).
374-375: LGTM!The execution control flags are correctly defined as
BooleanOptionalfields, allowing them to be nullable in the protocol layer while the configuration layer enforces defaults. This separation is intentional and follows the existing pattern for optional boolean fields in this codebase.
| execute_in_proposed_change: bool = True, | ||
| execute_after_merge: bool = True, |
There was a problem hiding this comment.
🧩 Analysis chain
Verify that execute_in_proposed_change is used by calling code.
The execute_in_proposed_change flag is accepted, stored, but never used within this file's logic. Only execute_after_merge influences the group_type selection (line 96). Ensure that calling code (likely in infrahub_sdk/ctl/generator.py) checks execute_in_proposed_change to determine whether to invoke run() during the proposed change phase.
Run the following script to verify usage of execute_in_proposed_change:
Also applies to: 57-58
🏁 Script executed:
#!/bin/bash
# Description: Find usages of execute_in_proposed_change to confirm it's checked by orchestration code
# Search for reads of the attribute
rg -n "\.execute_in_proposed_change" --type=pyLength of output: 335
🏁 Script executed:
#!/bin/bash
# Show context for execute_in_proposed_change in ctl/generator.py
rg -nC3 "execute_in_proposed_change" --type=py infrahub_sdk/ctl/generator.pyLength of output: 762
Remove or implement execute_in_proposed_change, it’s never used.
The flag is accepted and passed through from CLI (infrahub_sdk/ctl/generator.py:67,98) into the core generator (infrahub_sdk/generator.py:57) but never read—only execute_after_merge affects behavior. Either drop the parameter or add logic to act on it during the proposed‐change phase.
🤖 Prompt for AI Agents
In infrahub_sdk/generator.py around lines 37-38, the parameter
execute_in_proposed_change is accepted but never used; either remove this unused
parameter from the function signature and update callers
(infrahub_sdk/ctl/generator.py at the CLI call sites) to stop passing it, or
implement the missing logic: add a conditional branch in the proposed-change
execution path that checks execute_in_proposed_change and performs the intended
execution during the proposed-change phase (mirroring how execute_after_merge is
handled), and update/add unit tests and documentation accordingly.
dgarros
force-pushed
the
dga-20250909-generator-flags
branch
from
c3dc786 to
0f5a1df
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
infrahub_sdk/schema/repository.py (1)
99-106: Ensure at least one execution flag is enabled
Currently both flags default toTrue, but settingexecute_in_proposed_changeandexecute_after_mergetoFalsesilences the generator. Document this behavior or add a Pydantic root validator in infrahub_sdk/schema/repository.py to enforce that at least one of these flags isTrue.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
infrahub_sdk/ctl/generator.py(2 hunks)infrahub_sdk/generator.py(3 hunks)infrahub_sdk/protocols.py(6 hunks)infrahub_sdk/schema/repository.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- infrahub_sdk/protocols.py
- infrahub_sdk/generator.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/schema/repository.pyinfrahub_sdk/ctl/generator.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/generator.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.9)
- GitHub Check: unit-tests (3.10)
- GitHub Check: unit-tests (3.11)
- GitHub Check: unit-tests (3.13)
- GitHub Check: unit-tests (3.12)
- GitHub Check: integration-tests-latest-infrahub
🔇 Additional comments (2)
infrahub_sdk/ctl/generator.py (2)
67-68: LGTM!The new execution control parameters are correctly passed from the generator configuration to the generator class constructor, following the established pattern for other configuration parameters.
98-99: LGTM!The execution control parameters are consistently passed in both code paths (variables_dict present and targets path), ensuring uniform behavior across different execution scenarios.
2 participants
Add the flags
execute_in_proposed_changeandexecute_after_mergetoCoreGeneratorDefinitionto allow the user to control when a generator should be executed.Related to opsmill/infrahub#7183
Summary by CodeRabbit
New Features
Documentation