Add file_object_content_by_id and file_object_content_by_hfid filters#904
Merged
gmazoyer merged 7 commits intoinfrahub-developfrom Mar 30, 2026
Merged
Add file_object_content_by_id and file_object_content_by_hfid filters#904gmazoyer merged 7 commits intoinfrahub-developfrom
gmazoyer merged 7 commits intoinfrahub-developfrom
Conversation
Support all three file object retrieval endpoints: by storage ID (existing), by node UUID, and by HFID. Extract shared `ObjectStore` file fetching and content-type validation into helpers. Make `InfrahubFilters` accept an optional client, checking at call time instead of maintaining separate no-client fallback functions. CLIENT_FILTER_NAMES is the single source of truth for all client-dependent filter names.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds two async Jinja2 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: |
98deaf5
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b3054e11.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://gma-20260327-infp504.infrahub-sdk-python.pages.dev |
github-actions
bot
added
the
type/documentation
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## infrahub-develop #904 +/- ##
====================================================
- Coverage 81.09% 81.02% -0.08%
====================================================
Files 121 121
Lines 10592 10628 +36
Branches 1581 1584 +3
====================================================
+ Hits 8590 8611 +21
- Misses 1487 1501 +14
- Partials 515 516 +1
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:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
dev/specs/infp-504-artifact-composition/data-model.md (1)
68-69: Consider showing the completeCLIENT_FILTER_NAMEStuple for clarity.The ellipsis (
...) in the tuple obscures the actual filter names. Since this is specification documentation, showing all four names explicitly would be clearer and serve as better reference material.Based on context snippet 1 from
infrahub_sdk/template/infrahub_filters.py:19-27, the complete list is:CLIENT_FILTER_NAMES = ( "artifact_content", "file_object_content", "file_object_content_by_id", "file_object_content_by_hfid", )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/specs/infp-504-artifact-composition/data-model.md` around lines 68 - 69, Replace the truncated CLIENT_FILTER_NAMES tuple with the full list of four filter names so the spec shows exact values; locate the CLIENT_FILTER_NAMES symbol and expand its value to explicitly include "artifact_content", "file_object_content", "file_object_content_by_id", and "file_object_content_by_hfid" instead of using an ellipsis.dev/specs/infp-504-artifact-composition/spec.md (1)
121-121: Consider updating Key Entities to list all file object filters.The
InfrahubFiltersentity description mentionsartifact_contentandfile_object_contentbut doesn't explicitly list the two new filters (file_object_content_by_id,file_object_content_by_hfid). Consider updating for completeness.📝 Suggested update
-- **`InfrahubFilters`**: New class that holds an `InfrahubClient` reference and exposes `artifact_content`, `file_object_content`, and any other client-dependent filter methods. Registered into the Jinja2 filter map when a client is provided. +- **`InfrahubFilters`**: New class that holds an `InfrahubClient` reference and exposes `artifact_content`, `file_object_content`, `file_object_content_by_id`, `file_object_content_by_hfid`, and any other client-dependent filter methods. Registered into the Jinja2 filter map when a client is provided.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/specs/infp-504-artifact-composition/spec.md` at line 121, Update the InfrahubFilters Key Entities entry to explicitly list all file-object-related filters by name: include artifact_content, file_object_content, file_object_content_by_id, and file_object_content_by_hfid, and mention that these are registered into the Jinja2 filter map when an InfrahubClient is provided; locate the description for InfrahubFilters and replace the current two-filter mention with the full list to ensure completeness.infrahub_sdk/object_store.py (1)
113-117: URL-encode HFID query parameters to prevent malformed URLs with special characters.HFID components are joined directly into the query string without URL encoding. If HFID values contain special characters (spaces,
&,=, etc.), the URL will be malformed.🔧 Proposed fix using urllib.parse
+from urllib.parse import quote + async def get_file_by_hfid(self, kind: str, hfid: list[str], tracker: str | None = None) -> str: """Retrieve file object content by Human-Friendly ID.""" - params = "&".join(f"hfid={h}" for h in hfid) + params = "&".join(f"hfid={quote(h, safe='')}" for h in hfid) url = f"{self.client.address}/api/files/by-hfid/{kind}?{params}" return await self._get_file(url=url, identifier=f"{kind}:{'/'.join(hfid)}", tracker=tracker)Apply the same change to
ObjectStoreSync.get_file_by_hfidat lines 199-203.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@infrahub_sdk/object_store.py` around lines 113 - 117, The HFID query parameters in async get_file_by_hfid are concatenated raw and must be URL-encoded to avoid malformed URLs; change the params construction to use urllib.parse.urlencode (e.g. urlencode([("hfid", h) for h in hfid])) or quote each hfid before joining, then build the url as before and call _get_file; apply the identical change to ObjectStoreSync.get_file_by_hfid so both sync and async variants properly encode HFID values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@dev/specs/infp-504-artifact-composition/data-model.md`:
- Around line 68-69: Replace the truncated CLIENT_FILTER_NAMES tuple with the
full list of four filter names so the spec shows exact values; locate the
CLIENT_FILTER_NAMES symbol and expand its value to explicitly include
"artifact_content", "file_object_content", "file_object_content_by_id", and
"file_object_content_by_hfid" instead of using an ellipsis.
In `@dev/specs/infp-504-artifact-composition/spec.md`:
- Line 121: Update the InfrahubFilters Key Entities entry to explicitly list all
file-object-related filters by name: include artifact_content,
file_object_content, file_object_content_by_id, and file_object_content_by_hfid,
and mention that these are registered into the Jinja2 filter map when an
InfrahubClient is provided; locate the description for InfrahubFilters and
replace the current two-filter mention with the full list to ensure
completeness.
In `@infrahub_sdk/object_store.py`:
- Around line 113-117: The HFID query parameters in async get_file_by_hfid are
concatenated raw and must be URL-encoded to avoid malformed URLs; change the
params construction to use urllib.parse.urlencode (e.g. urlencode([("hfid", h)
for h in hfid])) or quote each hfid before joining, then build the url as before
and call _get_file; apply the identical change to
ObjectStoreSync.get_file_by_hfid so both sync and async variants properly encode
HFID values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 626eb80d-2e4a-439c-95fb-fe277f245a6f
📒 Files selected for processing (12)
dev/specs/infp-504-artifact-composition/contracts/filter-interfaces.mddev/specs/infp-504-artifact-composition/data-model.mddev/specs/infp-504-artifact-composition/quickstart.mddev/specs/infp-504-artifact-composition/spec.mddocs/_templates/sdk_template_reference.j2docs/docs/python-sdk/reference/templating.mdxinfrahub_sdk/object_store.pyinfrahub_sdk/template/__init__.pyinfrahub_sdk/template/filters.pyinfrahub_sdk/template/infrahub_filters.pytasks.pytests/unit/sdk/test_infrahub_filters.py
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
infrahub_sdk/template/infrahub_filters.py (1)
36-62: Consider consolidating validation logic with_fetch_file_object.The
artifact_contentfilter duplicates the null/empty validation and exception wrapping logic that_fetch_file_objectencapsulates. While the underlying API call differs (object_store.getvsget_file_by_*), the helper could still be reused here.Optional refactor to reduce duplication
async def artifact_content(self, storage_id: str) -> str: """Retrieve artifact content by storage_id.""" client = self._require_client(filter_name="artifact_content") - if storage_id is None: - raise JinjaFilterError( - filter_name="artifact_content", - message="storage_id is null", - hint="ensure the GraphQL query returns a valid storage_id value", - ) - if not storage_id: - raise JinjaFilterError( - filter_name="artifact_content", - message="storage_id is empty", - hint="ensure the GraphQL query returns a non-empty storage_id value", - ) - try: - return await client.object_store.get(identifier=storage_id) - except AuthenticationError as exc: - raise JinjaFilterError( - filter_name="artifact_content", message=f"permission denied for storage_id: {storage_id}" - ) from exc - except Exception as exc: - raise JinjaFilterError( - filter_name="artifact_content", - message=f"failed to retrieve content for storage_id: {storage_id}", - hint=str(exc), - ) from exc + return await self._fetch_file_object( + filter_name="artifact_content", + identifier=storage_id, + label="storage_id", + fetch=lambda: client.object_store.get(identifier=storage_id), + )🤖 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 36 - 62, The artifact_content function duplicates null/empty checks and exception wrapping already handled by _fetch_file_object; refactor by moving the shared validation/exception logic into _fetch_file_object (or extend _fetch_file_object to accept a retrieval strategy/callable) and have artifact_content call _fetch_file_object with a callable that invokes client.object_store.get(identifier=storage_id); remove the duplicated null/empty checks and exception handling from artifact_content and ensure JinjaFilterError still includes filter_name="artifact_content" when propagated.dev/specs/infp-504-artifact-composition/spec.md (1)
99-106: Add blank lines around the bullet list per markdown guidelines.The bullet list in FR-004 should have blank lines before and after it for proper markdown formatting.
Proposed fix
- **FR-004**: The system MUST provide three file object content filters, each retrieving content via a different identifier: + - `file_object_content` — accepts a `storage_id` string, uses `GET /api/files/by-storage-id/{storage_id}` - `file_object_content_by_id` — accepts a node UUID string, uses `GET /api/files/{node_id}` - `file_object_content_by_hfid` — accepts an HFID string or list and a required `kind` argument, uses `GET /api/files/by-hfid/{kind}?hfid=...` + All three share the same binary content-type rejection and error handling behavior.As per coding guidelines: "When editing
.mdor.mdxfiles, add blank lines before and after lists."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev/specs/infp-504-artifact-composition/spec.md` around lines 99 - 106, The markdown list under FR-004 needs surrounding blank lines for correct formatting; edit the FR-004 block in spec.md to insert a blank line immediately before the bullet list and another blank line after the list (so the three bullets for file_object_content, file_object_content_by_id, and file_object_content_by_hfid are separated by blank lines from the paragraphs above and below); ensure the rest of the FR-00x blocks keep the same spacing convention (FR-005, FR-006, FR-007) for consistency.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@infrahub_sdk/template/infrahub_filters.py`:
- Around line 114-130: In file_object_content_by_hfid, validate that when hfid
is a list every element is a non-empty string before calling _fetch_file_object;
currently hfid_list (created from hfid) can contain empty strings which are sent
to client.object_store.get_file_by_hfid. Update file_object_content_by_hfid to
build hfid_list = hfid if isinstance(hfid, list) else [hfid], then check
all(isinstance(x, str) and x.strip() for x in hfid_list) and raise
JinjaFilterError with filter_name="file_object_content_by_hfid" and an
appropriate message/hint if any element is empty or non-string, and pass
hfid_list as the identifier/argument to _fetch_file_object/get_file_by_hfid.
---
Nitpick comments:
In `@dev/specs/infp-504-artifact-composition/spec.md`:
- Around line 99-106: The markdown list under FR-004 needs surrounding blank
lines for correct formatting; edit the FR-004 block in spec.md to insert a blank
line immediately before the bullet list and another blank line after the list
(so the three bullets for file_object_content, file_object_content_by_id, and
file_object_content_by_hfid are separated by blank lines from the paragraphs
above and below); ensure the rest of the FR-00x blocks keep the same spacing
convention (FR-005, FR-006, FR-007) for consistency.
In `@infrahub_sdk/template/infrahub_filters.py`:
- Around line 36-62: The artifact_content function duplicates null/empty checks
and exception wrapping already handled by _fetch_file_object; refactor by moving
the shared validation/exception logic into _fetch_file_object (or extend
_fetch_file_object to accept a retrieval strategy/callable) and have
artifact_content call _fetch_file_object with a callable that invokes
client.object_store.get(identifier=storage_id); remove the duplicated null/empty
checks and exception handling from artifact_content and ensure JinjaFilterError
still includes filter_name="artifact_content" when propagated.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d2473a20-0f75-43b6-af48-d198b6be56db
📒 Files selected for processing (5)
dev/specs/infp-504-artifact-composition/data-model.mddev/specs/infp-504-artifact-composition/spec.mdinfrahub_sdk/template/__init__.pyinfrahub_sdk/template/infrahub_filters.pytests/unit/sdk/test_infrahub_filters.py
🚧 Files skipped from review as they are similar to previous changes (2)
- infrahub_sdk/template/init.py
- tests/unit/sdk/test_infrahub_filters.py
ogenstad
reviewed
Mar 30, 2026
Contributor
ogenstad
left a comment
ogenstad
left a comment
There was a problem hiding this comment.
LGTM. Looks like CodeRabbit has a point with the comment around "Edge case: empty strings within HFID list are not validated."
| | Name | CORE | WORKER | LOCAL | | ||
| | ---- | ---- | ------ | ----- | | ||
| {% for filter in infrahub %} | ||
| | `{{ filter.name }}` | {% if filter.core %}✅{% else %}❌{% endif %} | {% if filter.worker %}✅{% else %}❌{% endif %} | {% if filter.local %}✅{% else %}❌{% endif %} | |
Contributor
There was a problem hiding this comment.
Would it make sense to report on all filters using the same kind of table? I.e. for the previous table we only have the "trusted" option? Might be easier to understand if the format for the trust model is the same for all filter types?
ogenstad
approved these changes
Mar 30, 2026
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
2 participants
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 & What
This is a change following #889.
Support all three file object retrieval endpoints: by storage ID (existing), by node UUID, and by HFID. Extract shared
ObjectStorefile fetching and content-type validation into helpers.Make
InfrahubFiltersaccept an optional client, checking at call time instead of maintaining separate no-client fallback functions.Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests