-
Notifications
You must be signed in to change notification settings - Fork 39
fix: allow List attributes to use regex parameter #7718
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: stable
Are you sure you want to change the base?
Conversation
Add support for List attributes to accept regex validation via parameters.regex syntax. List attributes can now use TextAttributeParameters which includes regex support. Changes: - Added List to parameter class mapping - Created ListAttributeSchema with regex reconciliation - Updated validation to allow TextAttributeParameters for List - Added tests for List with regex parameter Fixes #7717
WalkthroughThis pull request introduces support for List attribute kind with regex parameters. The changes add a new Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
CodSpeed Performance ReportMerging #7718 will not alter performanceComparing Summary
Footnotes |
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 (3)
backend/infrahub/core/schema/attribute_parameters.py (1)
14-21: MappingListtoTextAttributeParametersis consistent with the new schema usageHooking
"List"intoget_attribute_parameters_class_for_kindso it returnsTextAttributeParameterslines up withListAttributeSchema.parametersand prevents the validation error described in the issue. It also means list attributes now exposeregex(and other text constraints) throughparameters, which is what the new tests rely on. If, longer-term, you decide that onlyregexshould be configurable for lists, you could consider a dedicatedListAttributeParameterssubclass, but for this bugfix the current change is appropriate and cohesive with the rest of the PR.backend/tests/unit/core/schema/test_attribute_parameters.py (1)
255-307: New List + regex tests exercise the key paths wellBoth tests nicely cover the intended behavior: correctly instantiating
TextAttributeParametersforkind="List", exposingregexviaget_regex(), and reconciling when onlyparameters.regexis set. To fully mirrorTextAttributeSchemasemantics, you could optionally add a third test where bothregexandparameters.regexare set with different values and assert that the chosen precedence for List matches Text/TextArea, but that’s not required for fixing the reported bug.backend/infrahub/core/schema/attribute_schema.py (1)
272-296:ListAttributeSchemaimplementation matches the intended regex behavior; minor reuse opportunitiesThe new
ListAttributeSchemareusingTextAttributeParametersand reconcilingregexbetween the attribute and its parameters is consistent withTextAttributeSchemaand with the new tests:
parameters: TextAttributeParametersensures JSON/YAML tooling sees the right parameter shape.reconcile_parameterscorrectly normalizesregex, withparameters.regexeffectively taking precedence when set.- Overriding
get_regexto read fromparametersmatches howTextAttributeSchemaexposes its final regex.If you later want lists to support additional text-style constraints (
min_length/max_length) the same way, you could factor the shared reconciliation logic into a small mixin/base class used by both Text and List schemas; conversely, if regex is the only supported constraint for lists, a brief comment or dedicated parameters type could make that intention explicit and avoid future confusion. The current implementation is still perfectly fine for this fix.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
backend/infrahub/core/schema/attribute_parameters.py(1 hunks)backend/infrahub/core/schema/attribute_schema.py(2 hunks)backend/tests/unit/core/schema/test_attribute_parameters.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
backend/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use Poetry for dependency management in backend Python projects
Files:
backend/infrahub/core/schema/attribute_parameters.pybackend/tests/unit/core/schema/test_attribute_parameters.pybackend/infrahub/core/schema/attribute_schema.py
**/*.py
📄 CodeRabbit inference engine (.github/instructions/python-docstring.instructions.md)
**/*.py: Always use triple quotes (""") for Python docstrings
Follow Google-style docstring format for Python docstrings
Include brief one-line description in Python docstrings when applicable
Include detailed description in Python docstrings when applicable
Include Args/Parameters section without typing in Python docstrings when applicable
Include Returns section in Python docstrings when applicable
Include Raises section in Python docstrings when applicable
Include Examples section in Python docstrings when applicable
**/*.py: Use type hints for all function parameters and return values in Python
Useasync deffor asynchronous functions in Python
Useawaitfor asynchronous calls in Python
Use Pydantic models for dataclasses in Python
Use ruff and mypy for type checking and validations in PythonUse ruff and mypy to validate and lint Python files
**/*.py: Use type hints for all function parameters and return values in Python code
Use async/await whenever possible; useasync deffor asynchronous functions andawaitfor asynchronous calls
Use Pydantic models for dataclasses instead of plain Python dataclasses
Always use triple quotes (""") for docstrings and follow Google-style docstring format with sections: brief description, detailed description (if needed), Args/Parameters, Returns, Raises, and Examples when applicable
Use ruff and mypy for type checking and validations; runpoetry run invoke formatto format all Python files andpoetry run invoke lintto validate formatting
Files:
backend/infrahub/core/schema/attribute_parameters.pybackend/tests/unit/core/schema/test_attribute_parameters.pybackend/infrahub/core/schema/attribute_schema.py
backend/tests/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
backend/tests/**/*.py: Run backend tests with pytest or via invoke tasks
Place backend tests in backend/tests/ directory
Files:
backend/tests/unit/core/schema/test_attribute_parameters.py
🧬 Code graph analysis (1)
backend/infrahub/core/schema/attribute_schema.py (2)
backend/infrahub/core/schema/attribute_parameters.py (1)
TextAttributeParameters(29-59)backend/infrahub/core/constants/schema.py (1)
UpdateSupport(16-21)
⏰ 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 (1)
backend/infrahub/core/schema/attribute_schema.py (1)
126-133: Parameter validation now correctly permitsTextAttributeParametersforkind="List"Allowing
TextAttributeParameterswhenself.kindis"List"bringsvalidate_parametersin line with the new parameter mapping andListAttributeSchema, preventing the previous “can’t be used as parameters for List” error. This fits the intended model without changing behavior for other kinds.
| assert config.SETTINGS.main.schema_strict_mode | ||
|
|
||
|
|
||
| def test_list_attribute_with_regex_parameter() -> None: |
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 would be good to add a test to ensure that the regex is properly applied to the values in the list
Add support for List attributes to accept regex validation via parameters.regex syntax. List attributes can now use TextAttributeParameters which includes regex support.
Changes:
Fixes #7717
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.