IFC-2275 Add Jinja2 filters for artifact composition#889
IFC-2275 Add Jinja2 filters for artifact composition#889gmazoyer merged 12 commits intoinfrahub-developfrom
Conversation
WalkthroughThis pull request introduces Jinja2 template filters for artifact composition within the Infrahub SDK. It adds four new filters: 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
Deploying infrahub-sdk-python with
|
| Latest commit: |
06fda33
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5dcbf956.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://gma-infp-504-artifact-compos.infrahub-sdk-python.pages.dev |
ce7a42e to
d7514e6
Compare
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## infrahub-develop #889 +/- ##
====================================================
+ Coverage 80.98% 81.04% +0.05%
====================================================
Files 120 121 +1
Lines 10449 10591 +142
Branches 1562 1580 +18
====================================================
+ Hits 8462 8583 +121
- Misses 1475 1489 +14
- Partials 512 519 +7
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:
|
56476d3 to
4e2de5e
Compare
28ec56e to
75800b8
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
dev/specs/infp-504-artifact-composition/contracts/filter-interfaces.md (1)
80-87: Add blank lines before and after the bullet list.As per coding guidelines for markdown files, add blank lines before and after lists.
📝 Suggested fix
**Accepted content-types** (text-based): + - `text/*` - `application/json` - `application/yaml` - `application/x-yaml` + **Rejected**: All other content-types → `JinjaFilterError` with binary content message.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/specs/infp-504-artifact-composition/contracts/filter-interfaces.md` around lines 80 - 87, Add blank lines immediately before the "Accepted content-types (text-based):" heading and immediately after the bullet list (before the "Rejected:" line) so the list is separated by empty lines per markdown guidelines; locate the block containing "Accepted content-types (text-based):" and the following bullets (`text/*`, `application/json`, `application/yaml`, `application/x-yaml`) and insert one blank line above and one blank line below that list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dev/specs/infp-504-artifact-composition/contracts/filter-interfaces.md`:
- Line 22: Update the validation context text to include LOCAL for both
artifact_content and file_object_content: change "Allowed in WORKER context" to
"Allowed in WORKER and LOCAL contexts" (or equivalent) to match the code where
artifact_content and file_object_content have
allowed_contexts=ExecutionContext.WORKER | ExecutionContext.LOCAL in
infrahub_sdk/template/filters.py; ensure the Validation line for both entries
mentions LOCAL alongside WORKER and that CORE remains as Blocked.
In `@dev/specs/infp-504-artifact-composition/data-model.md`:
- Around line 156-157: Update the FilterDefinition registrations for
"artifact_content" and "file_object_content" to reflect the actual allowed
contexts by changing allowed_contexts from ExecutionContext.WORKER to
ExecutionContext.WORKER | ExecutionContext.LOCAL (matching the implementation in
infrahub_sdk/template/filters.py), and update the accompanying comment from
"worker context only" to "worker and local contexts"; ensure you modify the
FilterDefinition(...) entries for those two names and any nearby comment text in
the same file.
In `@dev/specs/infp-504-artifact-composition/spec.md`:
- Line 102: The spec FR-007 is inconsistent with the code: the filters
artifact_content and file_object_content are registered in FilterDefinition with
allowed_contexts=ExecutionContext.WORKER | ExecutionContext.LOCAL in
infrahub_sdk/template/filters.py; update the spec text for FR-007 to list both
WORKER and LOCAL (or explicitly state that LOCAL is allowed alongside WORKER) so
the documented allowed_contexts matches the implementation for
FilterDefinition/validate().
---
Nitpick comments:
In `@dev/specs/infp-504-artifact-composition/contracts/filter-interfaces.md`:
- Around line 80-87: Add blank lines immediately before the "Accepted
content-types (text-based):" heading and immediately after the bullet list
(before the "Rejected:" line) so the list is separated by empty lines per
markdown guidelines; locate the block containing "Accepted content-types
(text-based):" and the following bullets (`text/*`, `application/json`,
`application/yaml`, `application/x-yaml`) and insert one blank line above and
one blank line below that list.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 52ef8c23-d67e-4725-952d-62d9bb6240bd
📒 Files selected for processing (15)
changelog/+artifact-composition.added.mdchangelog/+artifact-composition.changed.mddev/specs/infp-504-artifact-composition/contracts/filter-interfaces.mddev/specs/infp-504-artifact-composition/data-model.mddev/specs/infp-504-artifact-composition/plan.mddev/specs/infp-504-artifact-composition/quickstart.mddev/specs/infp-504-artifact-composition/research.mddev/specs/infp-504-artifact-composition/spec.mddev/specs/infp-504-artifact-composition/tasks.mdinfrahub_sdk/object_store.pyinfrahub_sdk/template/__init__.pyinfrahub_sdk/template/exceptions.pyinfrahub_sdk/template/filters.pyinfrahub_sdk/template/infrahub_filters.pytests/unit/sdk/test_infrahub_filters.py
dev/specs/infp-504-artifact-composition/contracts/filter-interfaces.md
Outdated
Show resolved
Hide resolved
75800b8 to
06fda33
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
infrahub_sdk/template/infrahub_filters.py (1)
102-113: Type annotation doesn't cover scalar YAML values.The return type
dict | listdoesn't account for valid YAML scalar values (e.g.,yaml.safe_load("42")returns42). The spec documentation atfilter-interfaces.mdline 64 states the output is "Parsed dict, list, or scalar". Consider updating the type hint for accuracy.📝 Suggested fix
-def from_yaml(value: str) -> dict | list: - """Parse a YAML string into a Python dict or list.""" +def from_yaml(value: str) -> dict | list | str | int | float | bool | None: + """Parse a YAML string into a Python object (dict, list, or scalar)."""Alternatively, use
Anyif the broader type is acceptable:def from_yaml(value: str) -> Any:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sdk/template/infrahub_filters.py` around lines 102 - 113, The from_yaml function's return type (dict | list) omits scalar YAML values; update the type annotation on from_yaml to include scalars (e.g., dict | list | str | int | float | bool | None) or use a broad Any to match yaml.safe_load and the spec in filter-interfaces.md; keep the function body as-is and only change the signature's return type to accurately reflect possible parsed outputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@infrahub_sdk/template/infrahub_filters.py`:
- Around line 102-113: The from_yaml function's return type (dict | list) omits
scalar YAML values; update the type annotation on from_yaml to include scalars
(e.g., dict | list | str | int | float | bool | None) or use a broad Any to
match yaml.safe_load and the spec in filter-interfaces.md; keep the function
body as-is and only change the signature's return type to accurately reflect
possible parsed outputs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fdf43fec-51e1-4d3a-b048-ec9800eb6b30
📒 Files selected for processing (12)
changelog/+artifact-composition.added.mdchangelog/+artifact-composition.changed.mddev/specs/infp-504-artifact-composition/contracts/filter-interfaces.mddev/specs/infp-504-artifact-composition/data-model.mddev/specs/infp-504-artifact-composition/spec.mddev/specs/infp-504-artifact-composition/tasks.mdinfrahub_sdk/object_store.pyinfrahub_sdk/template/__init__.pyinfrahub_sdk/template/exceptions.pyinfrahub_sdk/template/filters.pyinfrahub_sdk/template/infrahub_filters.pytests/unit/sdk/test_infrahub_filters.py
✅ Files skipped from review due to trivial changes (3)
- changelog/+artifact-composition.added.md
- changelog/+artifact-composition.changed.md
- infrahub_sdk/template/exceptions.py
🚧 Files skipped from review as they are similar to previous changes (4)
- infrahub_sdk/object_store.py
- dev/specs/infp-504-artifact-composition/data-model.md
- dev/specs/infp-504-artifact-composition/tasks.md
- dev/specs/infp-504-artifact-composition/spec.md
Why
Customers building modular configuration pipelines need to compose larger artifacts from smaller sub-artifacts inside Jinja2 transforms, without duplicating template logic or GraphQL query fields. Today there is no way for a template to reference the rendered content of another artifact.
This PR implements the SDK side of the "Artifact of Artifacts" initiative, adding new Jinja2 filters and the supporting infrastructure to enable artifact content composition.
What changed
artifact_contentthat accepts astorage_idstring, fetches artifact content from the object store viaObjectStore.get()file_object_contentwith the same pattern but uses the/api/files/by-storage-id/{storage_id}endpoint, with binary content-type rejectionfrom_json/from_yamlto parse strings into Python dicts/lists, enabling chaining like{{ sid | artifact_content | from_json }}FilterDefinition.trusted: boolwithallowed_contexts: ExecutionContextusing Python'sFlagenumCORE(API server computed attributes, most restrictive),WORKER(Prefect workers),LOCAL(CLI/unrestricted)client: InfrahubClientconstructor parameterset_client()method for deferred client injection (per PR Add plan to implement INFP-504 #885 feedback)JinjaFilterErrorwith an actionable messageJinjaFilterError(filter_name, message, hint)subclassingJinjaTemplateErrorget_file_by_storage_id()method (async + sync) using/api/files/by-storage-id/{storage_id}application/octet-stream, etc.)Suggested review order
infrahub_sdk/template/filters.pyforExecutionContextenum andFilterDefinitionmigrationinfrahub_sdk/template/infrahub_filters.pyforInfrahubFiltersclass,from_json,from_yaml3
infrahub_sdk/template/__init__.pyforJinja2Templatewiring,set_client(),validate()changesinfrahub_sdk/object_store.pyforget_file_by_storage_id()How to review
validate()context logic iit is the core of the trust model change_register_client_filters()andset_client()ensure client-dependent filters are correctly bound or fallbackHow to test
Impact & rollout
validate(restricted=True/False)continues to work as before.FilterDefinition.trustedproperty preserved.Jinja2Templateconstructor is backward compatible (newclientparam is optional with defaultNone).Checklist
uv run towncrier create ...)Summary by CodeRabbit
Release Notes
New Features
from_jsonandfrom_yamlfilters for template data transformationDocumentation
Tests