Replace strategy in object file with parameters section #585
Conversation
WalkthroughThe PR replaces the strategy-based object processing API with a parameters-driven API. ObjectStrategy and legacy processor classes were removed; a new InfrahubObjectParameters model with expand_range: bool was added. InfrahubObjectFileData now exposes a parameters field and performs async processing via a new DataProcessor ABC, DataProcessorFactory, and RangeExpandDataProcessor. Documentation, YAML examples, and tests were updated to use parameters.expand_range and tests now seed the schema cache for test setup. Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ 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)
🧰 Additional context used📓 Path-based instructions (1)**/*.py📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (1)infrahub_sdk/spec/object.py (7)
⏰ 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)
🔇 Additional comments (9)
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 |
github-actions
bot
added
the
type/documentation
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## infrahub-develop #585 +/- ##
====================================================
- Coverage 75.25% 75.24% -0.01%
====================================================
Files 108 112 +4
Lines 9398 9422 +24
Branches 1868 1873 +5
====================================================
+ Hits 7072 7090 +18
- Misses 1830 1834 +4
- Partials 496 498 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 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: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/docs/python-sdk/topics/object_file.mdx(5 hunks)infrahub_sdk/spec/models.py(1 hunks)infrahub_sdk/spec/object.py(20 hunks)infrahub_sdk/spec/processors/data_processor.py(1 hunks)infrahub_sdk/spec/processors/factory.py(1 hunks)infrahub_sdk/spec/processors/range_expand_processor.py(1 hunks)tests/unit/sdk/conftest.py(1 hunks)tests/unit/sdk/spec/test_object.py(8 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.{md,mdx}: Follow the Diataxis framework and classify docs as Tutorials, How-to guides, Explanation, or Reference
Structure How-to guides with required frontmatter and sections: introduction, prerequisites, step-by-step steps, validation, related resources
Structure Topics (Explanation) docs with introduction, concepts & definitions, background & context, architecture & design, connections, further reading
Use a professional, concise, informative tone with consistent structure across documents
Use proper language tags on all code blocks
Include both async and sync examples where applicable using the Tabs component
Add validation steps to guides to confirm success and expected outputs
Use callouts for warnings, tips, and important notes
Define new terms on first use and use domain-relevant terminology consistent with Infrahub’s model/UI
Conform to markdown style rules in .markdownlint.yaml and Vale styles in .vale/styles/
Files:
docs/docs/python-sdk/topics/object_file.mdx
**/*.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/spec/processors/factory.pyinfrahub_sdk/spec/processors/range_expand_processor.pyinfrahub_sdk/spec/processors/data_processor.pytests/unit/sdk/conftest.pyinfrahub_sdk/spec/models.pyinfrahub_sdk/spec/object.pytests/unit/sdk/spec/test_object.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Place and write unit tests under tests/unit/ (isolated component tests)
Files:
tests/unit/sdk/conftest.pytests/unit/sdk/spec/test_object.py
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Use the custom pytest plugin (infrahub_sdk.pytest_plugin) fixtures for clients, configuration, and Infrahub-specific testing
Files:
tests/unit/sdk/conftest.pytests/unit/sdk/spec/test_object.py
🧠 Learnings (1)
📚 Learning: 2025-08-24T13:35:12.998Z
Learnt from: CR
PR: opsmill/infrahub-sdk-python#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-24T13:35:12.998Z
Learning: Applies to tests/**/*.py : Use the custom pytest plugin (infrahub_sdk.pytest_plugin) fixtures for clients, configuration, and Infrahub-specific testing
Applied to files:
tests/unit/sdk/conftest.py
🧬 Code graph analysis (6)
infrahub_sdk/spec/processors/factory.py (3)
infrahub_sdk/spec/models.py (1)
InfrahubObjectParameters(6-7)infrahub_sdk/spec/processors/data_processor.py (2)
DataProcessor(5-10)process_data(9-10)infrahub_sdk/spec/processors/range_expand_processor.py (2)
RangeExpandDataProcessor(12-51)process_data(47-51)
infrahub_sdk/spec/processors/range_expand_processor.py (4)
infrahub_sdk/exceptions.py (1)
ValidationError(117-129)infrahub_sdk/spec/range_expansion.py (1)
range_expansion(88-118)infrahub_sdk/spec/processors/data_processor.py (2)
DataProcessor(5-10)process_data(9-10)infrahub_sdk/spec/processors/factory.py (1)
process_data(25-34)
infrahub_sdk/spec/processors/data_processor.py (2)
infrahub_sdk/spec/processors/factory.py (1)
process_data(25-34)infrahub_sdk/spec/processors/range_expand_processor.py (1)
process_data(47-51)
tests/unit/sdk/conftest.py (3)
tests/unit/sdk/checks/conftest.py (1)
client(8-9)infrahub_sdk/client.py (1)
InfrahubClient(316-1688)infrahub_sdk/schema/__init__.py (1)
set_cache(133-148)
infrahub_sdk/spec/object.py (5)
infrahub_sdk/exceptions.py (2)
ObjectValidationError(132-139)ValidationError(117-129)infrahub_sdk/spec/models.py (1)
InfrahubObjectParameters(6-7)infrahub_sdk/spec/processors/factory.py (2)
DataProcessorFactory(11-34)process_data(25-34)infrahub_sdk/spec/processors/range_expand_processor.py (3)
RangeExpandDataProcessor(12-51)process_data(47-51)expand_data_with_ranges(16-44)infrahub_sdk/spec/processors/data_processor.py (1)
process_data(9-10)
tests/unit/sdk/spec/test_object.py (3)
infrahub_sdk/spec/object.py (6)
spec(631-637)ObjectFile(627-655)RelationshipDataFormat(37-45)get_relationship_info(105-167)validate_format(180-199)validate_format(648-652)tests/unit/sdk/conftest.py (4)
client(32-33)schema_query_01_data(1847-1849)client_with_schema_01(1882-1884)location_schema(145-177)infrahub_sdk/schema/__init__.py (1)
set_cache(133-148)
⏰ 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). (8)
- GitHub Check: documentation
- GitHub Check: unit-tests (3.9)
- GitHub Check: unit-tests (3.11)
- GitHub Check: unit-tests (3.13)
- GitHub Check: unit-tests (3.12)
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: unit-tests (3.10)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (7)
infrahub_sdk/spec/models.py (1)
1-7: LGTM! Clean model definition.The
InfrahubObjectParametersmodel provides a clean, extensible way to configure data processing behavior. The default value ofFalseforexpand_rangeensures backward compatibility.tests/unit/sdk/conftest.py (1)
1881-1884: LGTM! Helpful test fixture.This fixture simplifies test setup by pre-populating the schema cache, avoiding the need for HTTPX mocks in many test scenarios. This aligns well with the custom pytest plugin usage pattern for Infrahub testing.
infrahub_sdk/spec/processors/data_processor.py (1)
1-10: LGTM! Well-designed abstract base.The abstract base class establishes a clear contract for data processors with an appropriate async interface. The signature aligns well with the factory pattern implementation.
docs/docs/python-sdk/topics/object_file.mdx (1)
63-79: LGTM! Documentation thoroughly updated.The documentation clearly explains the new parameters-based approach with appropriate examples and default values. The structure follows the Diataxis framework for topic/explanation documentation.
infrahub_sdk/spec/processors/factory.py (1)
1-34: LGTM! Well-designed factory pattern.The factory provides clean composition of data processors based on parameters and kind. The
PROCESSOR_PER_KINDregistry enables future extensibility for kind-specific processing while keeping the code simple and maintainable.tests/unit/sdk/spec/test_object.py (1)
1-223: LGTM! Tests thoroughly updated.All tests have been consistently updated to use the new parameters-based approach. The use of
client_with_schema_01fixture simplifies test setup, and the test assertions correctly verify the newparameters.expand_rangebehavior.infrahub_sdk/spec/object.py (1)
11-13: LGTM! Clean refactoring to parameters-based approach.The refactoring successfully replaces the strategy-based system with a more flexible parameters-based approach. The async processing through
DataProcessorFactoryprovides better extensibility and cleaner separation of concerns.Also applies to: 172-178
infrahub_sdk/spec/object.py
Outdated
| context.update(rel_info.get_context(value=parent_node.id)) | ||
|
|
||
| expanded_data = expand_data_with_ranges(data=data["data"]) | ||
| expanded_data = RangeExpandDataProcessor.expand_data_with_ranges(data=data["data"]) |
There was a problem hiding this comment.
I might be missing something here, but are we always calling this method to expand_data_with_ranges()? Aren't we expected to consider the parameters for the setting first?
There was a problem hiding this comment.
DataProcessorFactory.process_data does not always process the data it will first evaluate if the user has configured a parameter before it gets applied. It only applies the range expansion if parameters.expand_range is True. If parameters.expand_range is False (the default), the data is returned unchanged—no expansion happens. So, the parameters are always considered first before any processing is done.
Doing it this way means we dont have to check each time we evaluate the data which is in multiple places we can handle the check in a single place which is in that function.
Deploying infrahub-sdk-python with
|
| Latest commit: |
5774649
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8cd9aec9.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://dga-20251026-object-paramete.infrahub-sdk-python.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
infrahub_sdk/spec/object.py (2)
224-224: Avoid mutable defaults in function signatures.Multiple method signatures use
parameters: InfrahubObjectParameters = InfrahubObjectParameters()as a default argument. This creates a single shared instance across all calls that don't provide the parameter, which can lead to unexpected mutations.Based on learnings.
Apply this pattern to lines 224, 290, 400, and 525:
async def validate_object( cls, client: InfrahubClient, schema: MainSchemaTypesAPI, data: dict, position: list[int | str], context: dict | None = None, branch: str | None = None, default_schema_kind: str | None = None, - parameters: InfrahubObjectParameters = InfrahubObjectParameters(), + parameters: InfrahubObjectParameters | None = None, ) -> list[ObjectValidationError]: + parameters = parameters or InfrahubObjectParameters() errors: list[ObjectValidationError] = []Apply the same pattern to
validate_related_nodes(line 290),create_node(line 400), andcreate_related_nodes(line 525).Also applies to: 290-290, 400-400, 525-525
171-171: Use Field with default_factory for mutable default.Line 171 uses a direct instantiation
InfrahubObjectParameters()as the default value. While Pydantic models handle this better than plain Python classes, the recommended practice is to useField(default_factory=...)to ensure each instance gets a fresh default object.Based on learnings.
Apply this diff:
- parameters: InfrahubObjectParameters = InfrahubObjectParameters() + parameters: InfrahubObjectParameters = Field(default_factory=InfrahubObjectParameters)
🧹 Nitpick comments (1)
infrahub_sdk/spec/processors/range_expand_processor.py (1)
18-60: Consider inlining expand_data_with_ranges into process_data.The
expand_data_with_rangesstatic method is only called fromprocess_data. As noted in a past review comment, this wrapper adds an unnecessary layer of indirection without providing clear benefits.Based on learnings.
Consider moving the logic directly into
process_datato simplify the implementation:- @staticmethod - def expand_data_with_ranges(data: list[dict[str, Any]]) -> list[dict[str, Any]]: - """Expand any item in data with range pattern in any value. Supports multiple fields, requires equal expansion length.""" + async def process_data( + self, + data: list[dict[str, Any]], + ) -> list[dict[str, Any]]: + """Process data with range expansion. Supports multiple fields, requires equal expansion length.""" range_pattern = re.compile(MATCH_PATTERN) expanded = [] for item in data: @@ -50,11 +47,4 @@ for key, values in expand_fields.items(): new_item[key] = values[i] expanded.append(new_item) return expanded - - @classmethod - async def process_data( - cls, - data: list[dict[str, Any]], - ) -> list[dict[str, Any]]: - return cls.expand_data_with_ranges(data)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
infrahub_sdk/spec/object.py(20 hunks)infrahub_sdk/spec/processors/range_expand_processor.py(1 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/spec/object.pyinfrahub_sdk/spec/processors/range_expand_processor.py
🧬 Code graph analysis (2)
infrahub_sdk/spec/object.py (7)
infrahub_sdk/exceptions.py (2)
ObjectValidationError(132-139)ValidationError(117-129)infrahub_sdk/schema/main.py (3)
RelationshipKind(28-36)RelationshipSchema(124-142)kind(279-280)infrahub_sdk/yaml.py (4)
InfrahubFile(143-163)InfrahubFileKind(20-22)data(147-150)kind(157-158)infrahub_sdk/spec/models.py (1)
InfrahubObjectParameters(6-7)infrahub_sdk/spec/processors/factory.py (2)
DataProcessorFactory(11-34)process_data(25-34)infrahub_sdk/spec/processors/range_expand_processor.py (1)
process_data(56-60)infrahub_sdk/spec/processors/data_processor.py (1)
process_data(9-10)
infrahub_sdk/spec/processors/range_expand_processor.py (4)
infrahub_sdk/exceptions.py (1)
ValidationError(117-129)infrahub_sdk/spec/range_expansion.py (1)
range_expansion(88-118)infrahub_sdk/spec/processors/data_processor.py (2)
DataProcessor(5-10)process_data(9-10)infrahub_sdk/spec/processors/factory.py (1)
process_data(25-34)
⏰ 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.9)
- 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: documentation
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
infrahub_sdk/spec/object.py (2)
174-177: LGTM: Consistent processor factory usage.The refactor successfully migrates to the factory pattern. Both
_get_processed_dataand the inline usage at lines 560-562 correctly delegate toDataProcessorFactory.process_data, addressing the previous inconsistency concern.Also applies to: 560-562
183-196: LGTM: Parameters propagated correctly through validation flow.The validation flow correctly passes
parametersthrough the processing pipeline, ensuring that range expansion and other processing behaviors are controlled by the configuration inInfrahubObjectParameters.Also applies to: 333-348
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
infrahub_sdk/spec/object.py (1)
205-212: Parameters dropped during processing
create_nodeis invoked without passingself.parameters, so every call resets to a freshInfrahubObjectParameters()(expand_range falls back to False). Nested relationship creation therefore ignores the configuration even when validation already expanded using the flag. Please forward the existing parameters.for idx, item in enumerate(processed_data): await self.create_node( client=client, schema=schema, data=item, position=[idx + 1], branch=branch, - default_schema_kind=self.kind, + default_schema_kind=self.kind, + parameters=self.parameters, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrahub_sdk/spec/object.py(20 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/spec/object.py
🧬 Code graph analysis (1)
infrahub_sdk/spec/object.py (4)
infrahub_sdk/spec/models.py (1)
InfrahubObjectParameters(6-7)infrahub_sdk/spec/processors/factory.py (2)
DataProcessorFactory(11-34)process_data(25-34)infrahub_sdk/spec/processors/range_expand_processor.py (1)
process_data(19-56)infrahub_sdk/spec/processors/data_processor.py (1)
process_data(9-10)
⏰ 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: integration-tests-latest-infrahub
- GitHub Check: unit-tests (3.10)
- GitHub Check: unit-tests (3.13)
- GitHub Check: unit-tests (3.11)
- GitHub Check: unit-tests (3.12)
This PR introduces a more flexible and extensible configuration system for Infrahub Object files. Instead of using a simple strategy field, Object files now support a comprehensive parameters section that allows for fine-grained control over data processing.
Object files now support a parameters section in the spec that controls how data is processed:
Future Extensibility
This new architecture makes it easy to add additional data processing parameters in the future, such as: Jinja2 processor
Summary by CodeRabbit
Documentation
New Features / Updates
Tests