Skip to content

Set *_content filter context to WORKER#907

Merged
gmazoyer merged 2 commits intoinfrahub-developfrom
gma-20260330-content-filter-context
Mar 30, 2026
Merged

Set *_content filter context to WORKER#907
gmazoyer merged 2 commits intoinfrahub-developfrom
gma-20260330-content-filter-context

Conversation

@gmazoyer
Copy link
Copy Markdown
Contributor

@gmazoyer gmazoyer commented Mar 30, 2026

Why & What

The idea is that these filters should never be used by the main API server. Making them LOCAL too would allow that. So rather than doing this, we prefer making them WORKER only and allow any filters to run on workers if the user turns on the proper setting in the API server.

Related to opsmill/infrahub#8705
Following #904

Checklist

  • Tests added/updated

Summary by CodeRabbit

  • Documentation

    • Updated Infrahub SDK filter support matrix.
  • Bug Fixes

    • Restricted artifact_content, file_object_content, file_object_content_by_hfid, and file_object_content_by_id filters to WORKER execution context; no longer available in LOCAL context.

The idea is that these filters should *never* be used by the main API
server. Making them `LOCAL` too would allow that. So rather than doing
this, we prefer making them `WORKER` only and allow any filters to run
on workers if the user turns on the proper setting in the API server.
@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages bot commented Mar 30, 2026

Deploying infrahub-sdk-python with  Cloudflare Pages  Cloudflare Pages

Latest commit: 64369aa
Status: ✅  Deploy successful!
Preview URL: https://f25b116a.infrahub-sdk-python.pages.dev
Branch Preview URL: https://gma-20260330-content-filter.infrahub-sdk-python.pages.dev

View logs

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 30, 2026

Walkthrough

Four Infrahub template filters—artifact_content, file_object_content, file_object_content_by_hfid, and file_object_content_by_id—have been restricted from execution in the LOCAL context. These filters now support execution only in the WORKER context. Changes were made to the filter definitions in the SDK, the corresponding documentation support matrix, and the associated unit tests.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarizes the main change: restricting *_content filters to the WORKER execution context only.
Description check ✅ Passed PR description is minimal but provides clear rationale for why filters are restricted to WORKER context, references related issues, and confirms tests were updated.

✏️ 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions bot added the type/documentation Improvements or additions to documentation label Mar 30, 2026
@codecov
Copy link
Copy Markdown

codecov bot commented Mar 30, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@                 Coverage Diff                  @@
##           infrahub-develop     #907      +/-   ##
====================================================
- 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     
Flag Coverage Δ
integration-tests 40.93% <ø> (-0.14%) ⬇️
python-3.10 52.56% <ø> (-0.10%) ⬇️
python-3.11 52.54% <ø> (-0.14%) ⬇️
python-3.12 52.54% <ø> (-0.12%) ⬇️
python-3.13 52.54% <ø> (-0.14%) ⬇️
python-3.14 54.27% <ø> (-0.09%) ⬇️
python-filler-3.12 23.87% <ø> (+0.05%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
infrahub_sdk/template/filters.py 100.00% <ø> (ø)

... and 10 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gmazoyer gmazoyer marked this pull request as ready for review March 30, 2026 13:32
@gmazoyer gmazoyer requested a review from a team as a code owner March 30, 2026 13:32
Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
tests/unit/sdk/test_infrahub_filters.py (1)

65-67: Broaden this regression to the other three worker-only filters.

The PR changes four INFRAHUB_FILTERS entries, but these assertions only exercise artifact_content. Parameterizing the trusted/LOCAL-blocked checks over file_object_content, file_object_content_by_hfid, and file_object_content_by_id would keep a typo in any one of those rows from slipping through.

Also applies to: 113-117

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/unit/sdk/test_infrahub_filters.py` around lines 65 - 67, Update the
unit tests to cover all four INFRAHUB worker-only filters rather than only
artifact_content: modify test_not_trusted_when_worker_only (and the similar
check around the other test block) to parameterize or iterate over the filter
names file_object_content, file_object_content_by_hfid,
file_object_content_by_id, and artifact_content and assert fd.trusted is False
(and that LOCAL is blocked where applicable) for each; locate the construction
using FilterDefinition and ExecutionContext.WORKER
(test_not_trusted_when_worker_only) and apply the same parameterized assertions
to the other test block that currently checks INFRAHUB_FILTERS entries to ensure
each of the four rows is validated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/docs/python-sdk/reference/templating.mdx`:
- Around line 188-191: The LOCAL description in templating.mdx is out of sync
with the filter availability table (rows for `artifact_content`,
`file_object_content`, `file_object_content_by_hfid`,
`file_object_content_by_id` show unavailable in `LOCAL` but the prose still
claims LOCAL is unrestricted); regenerate the generated docs rather than
manually editing the file by running the docs generator (uv run invoke
docs-generate) to rebuild docs/docs/python-sdk/reference/templating.mdx so the
narrative about `LOCAL` matches the matrix and ensure the `LOCAL` description
text and the table entries for `artifact_content`, `file_object_content`,
`file_object_content_by_hfid`, and `file_object_content_by_id` are consistent.

In `@infrahub_sdk/template/filters.py`:
- Around line 165-168: The listed filters are only labeled with allowed_contexts
but still get registered unconditionally, so enforce the worker-only restriction
by adding a context guard: update _register_client_filters and _set_filters to
skip registering any FilterDefinition whose allowed_contexts does not include
ExecutionContext.WORKER (or alternately wrap those filter callables in a closure
that checks a runtime context and raises if not ExecutionContext.WORKER), and/or
add a final check in Jinja2Template.render to validate the template context
before executing filters; modify the registration logic in
_register_client_filters/_set_filters and/or add the runtime guard inside the
filter wrappers so the filters (e.g., "artifact_content", "file_object_content",
"file_object_content_by_hfid", "file_object_content_by_id") cannot be invoked
unless the active context is ExecutionContext.WORKER.

---

Nitpick comments:
In `@tests/unit/sdk/test_infrahub_filters.py`:
- Around line 65-67: Update the unit tests to cover all four INFRAHUB
worker-only filters rather than only artifact_content: modify
test_not_trusted_when_worker_only (and the similar check around the other test
block) to parameterize or iterate over the filter names file_object_content,
file_object_content_by_hfid, file_object_content_by_id, and artifact_content and
assert fd.trusted is False (and that LOCAL is blocked where applicable) for
each; locate the construction using FilterDefinition and ExecutionContext.WORKER
(test_not_trusted_when_worker_only) and apply the same parameterized assertions
to the other test block that currently checks INFRAHUB_FILTERS entries to ensure
each of the four rows is validated.
🪄 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: 5f0bd051-b0cd-4e3b-84f1-56d11406ee22

📥 Commits

Reviewing files that changed from the base of the PR and between 3e93fe2 and 64369aa.

📒 Files selected for processing (3)
  • docs/docs/python-sdk/reference/templating.mdx
  • infrahub_sdk/template/filters.py
  • tests/unit/sdk/test_infrahub_filters.py

@gmazoyer gmazoyer merged commit abe1d0d into infrahub-develop Mar 30, 2026
21 checks passed
@gmazoyer gmazoyer deleted the gma-20260330-content-filter-context branch March 30, 2026 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type/documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants