-
Notifications
You must be signed in to change notification settings - Fork 6
Add value_to_enum_name filter
#727
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: infrahub-develop
Are you sure you want to change the base?
Conversation
This Jinja2 filter aims to convert a value into a name when it is part of an enum. This is useful when the name of an enum member has more meaning, e.g. in a string which is exposed to users. An optional `enum_path` parameter can be specified. When using a valid import string for an enum class, that class will be imported to look for the value within the enum members. This mean that this filter can be used with any enum classes, including the ones found in the Infrahub server code.
WalkthroughA new filter function Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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: |
5ebde43
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://156b19cc.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://gma-20251230-jinja2-enum-fil.infrahub-sdk-python.pages.dev |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## infrahub-develop #727 +/- ##
====================================================
+ Coverage 76.29% 76.35% +0.05%
====================================================
Files 114 114
Lines 9831 9858 +27
Branches 1509 1513 +4
====================================================
+ Hits 7501 7527 +26
Misses 1834 1834
- Partials 496 497 +1
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:
|
| INFRAHUB_FILTER_DEFINITIONS = [ | ||
| FilterDefinition(name=name, trusted=True, source="infrahub-sdk-python") for name in sorted(INFRAHUB_FILTERS.keys()) | ||
| ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wrote it like that so that we can easily add filters in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
infrahub_sdk/template/filters.py (1)
40-45: Minor: Redundant None check for enum member name.The
nameattribute of an enum member is always a non-None string, so theif enum_member.name is not None:check on line 42 is unnecessary and can be simplified.🔎 Suggested simplification
try: enum_member = enum_type(value) - if enum_member.name is not None: - return enum_member.name + return enum_member.name except (ValueError, TypeError): pass
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
infrahub_sdk/template/__init__.pyinfrahub_sdk/template/filters.pytests/unit/sdk/test_template.py
🧰 Additional context used
📓 Path-based instructions (4)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use type hints on all function signatures
Never mix async/sync inappropriately
Never bypass type checking without justification
Files:
infrahub_sdk/template/filters.pytests/unit/sdk/test_template.pyinfrahub_sdk/template/__init__.py
infrahub_sdk/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Follow async/sync dual pattern for new features in the Python SDK
Files:
infrahub_sdk/template/filters.pyinfrahub_sdk/template/__init__.py
tests/**/*.py
📄 CodeRabbit inference engine (tests/AGENTS.md)
tests/**/*.py: Usehttpx_mockfixture for HTTP mocking in tests instead of making real HTTP requests
Do not add@pytest.mark.asynciodecorator to async test functions (async auto-mode is globally enabled)
Files:
tests/unit/sdk/test_template.py
tests/unit/**/*.py
📄 CodeRabbit inference engine (tests/AGENTS.md)
Unit tests must be fast, mocked, and have no external dependencies
Files:
tests/unit/sdk/test_template.py
🧬 Code graph analysis (2)
infrahub_sdk/template/filters.py (1)
infrahub_sdk/_importer.py (1)
import_module(16-63)
tests/unit/sdk/test_template.py (2)
infrahub_sdk/template/filters.py (2)
FilterDefinition(8-11)value_to_enum_name(14-47)infrahub_sdk/template/__init__.py (2)
Jinja2Template(28-182)get_variables(78-87)
⏰ 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). (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
infrahub_sdk/template/filters.py (2)
50-53: Verify thattrusted=Trueis appropriate for this filter.The filter is marked as
trusted=True, meaning it can be used in restricted template contexts. Since it dynamically imports modules based on user-provided paths, please confirm this trust level is intentional. The dynamic import allows access to any importable module, though the filter only extracts enum member names.If this is the intended security posture (given the stated use case for Infrahub server enums), the implementation looks correct.
14-47: LGTM - Well-structured filter implementation.The function handles multiple edge cases gracefully:
- Direct enum input returns name immediately
- Missing enum_path falls back to string conversion
- Import/attribute errors are caught and handled
- Non-enum classes are rejected
- Invalid values for the enum fall back safely
Good defensive programming with comprehensive error handling.
infrahub_sdk/template/__init__.py (1)
158-161: LGTM - Clean integration following established patterns.The INFRAHUB_FILTERS integration mirrors the existing netutils_filters pattern exactly, with proper gating via
_available_filterscheck.tests/unit/sdk/test_template.py (3)
318-325: LGTM - Good test enum and path setup.The
SampleIntEnumprovides a clean test fixture with representative values, andTEST_ENUM_PATHcorrectly references the test module path for dynamic import testing.
334-370: LGTM - Comprehensive unit test coverage.The tests cover all critical paths:
- Valid enum resolution with full import path
- Direct enum instance input
- Missing enum_path fallback
- Invalid module, class, and value handling
- Edge cases (zero value, malformed path)
Good defensive testing for the filter's error handling behavior.
373-421: LGTM - Thorough template integration tests.The test cases validate real-world template usage patterns including:
- Simple enum value conversion
- Chained filters (
| lower)- Complex template strings with multiple variables
- Invalid path fallback behavior
The async test correctly omits
@pytest.mark.asyncioas per the coding guidelines (async auto-mode is globally enabled).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
infrahub_sdk/template/filters.py (1)
44-45: Simplify the conditional check.The check
if enum_member.name is not None:is unnecessary since enum members always have anameattribute that is a string, neverNone.🔎 Proposed simplification
try: enum_member = enum_type(raw_value) - if enum_member.name is not None: - return enum_member.name + return enum_member.name except (ValueError, TypeError): pass
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrahub_sdk/template/filters.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use type hints on all function signatures
Never mix async/sync inappropriately
Never bypass type checking without justification
Files:
infrahub_sdk/template/filters.py
infrahub_sdk/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
Follow async/sync dual pattern for new features in the Python SDK
Files:
infrahub_sdk/template/filters.py
🧬 Code graph analysis (1)
infrahub_sdk/template/filters.py (2)
infrahub_sdk/protocols_base.py (1)
Enum(110-111)infrahub_sdk/_importer.py (1)
import_module(16-63)
⏰ 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.12)
- GitHub Check: unit-tests (3.11)
- GitHub Check: unit-tests (3.14)
- GitHub Check: unit-tests (3.10)
- GitHub Check: unit-tests (3.13)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (4)
infrahub_sdk/template/filters.py (4)
2-4: LGTM!The imports are appropriate for the new filter functionality. Using the standard library
import_moduleis correct here since it handles string-based module paths (e.g., "infrahub.core.constants"), whereas the customimport_modulein_importer.pyis designed for file-based imports.
14-49: Well-designed filter with robust error handling.The implementation correctly handles multiple scenarios:
- Values already as Enum instances
- Dynamic enum class import and validation
- Extracting raw values from different enum types (per commit message)
- Graceful fallback to string representation
Type hints follow coding guidelines properly.
52-55: LGTM!The filter registration structure is well-designed for future extensibility. Using
sorted()ensures consistent ordering of filter definitions, and marking the filter astrusted=Trueis appropriate for SDK-provided functionality.
198-198: LGTM!The integration of INFRAHUB_FILTER_DEFINITIONS into AVAILABLE_FILTERS is correct and maintains the existing pattern.
| try: | ||
| module_path, class_name = enum_path.rsplit(".", 1) | ||
| module = import_module(module_path) | ||
| enum_type = getattr(module, class_name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is allowing the user to import any Python module inside of the SDK a security concern?
I guess the user can already do this b/c they are using the SDK in their own code, but we also use the SDK within Infrahub and I wonder if there is a way for the user to force the Infrahub server to import something malicious
This Jinja2 filter aims to convert a value into a name when it is part of an enum. This is useful when the name of an enum member has more meaning, e.g. in a string which is exposed to users.
An optional
enum_pathparameter can be specified. When using a valid import string for an enum class, that class will be imported to look for the value within the enum members. This mean that this filter can be used with any enum classes, including the ones found in the Infrahub server code.Note: I'd like to use this filter to move our current
display_labelsfor permission nodes into Jinja2 baseddisplay_label.Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.