Merge 'stable' into 'release-1.7' with resolved conflicts #8010
Conversation
Add missing annotations to conftest
…olation Move too many statement violations to single file
Remove redundant cast
* fix warnings for deprecated attr regex/min/max * handle text param reconcile earlier in load * also handle loading legacy schema from db * add changelog
WalkthroughThis pull request refactors Text attribute schema handling by moving validation logic from TextAttributeSchema to SchemaBranch. It removes a model_validator method and introduces three new reconciliation methods that synchronize deprecated top-level fields (regex, min_length, max_length) with new TextAttributeParameters during schema loading and validation. Additionally, GraphQL schema fields are hardened to non-nullable across multiple types, DiffTreeSummary count fields are made required, type annotations are systematically added to test fixtures, and test cases are updated to reflect the new parameters-based configuration approach. 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 |
github-actions
bot
added
the
group/backend
CodSpeed Performance ReportMerging #8010 will not alter performanceComparing Summary
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
backend/infrahub/core/schema/attribute_schema.pybackend/infrahub/core/schema/schema_branch.pybackend/infrahub/graphql/app.pybackend/infrahub/graphql/queries/diff/tree.pybackend/tests/conftest.pybackend/tests/integration/schema_lifecycle/test_attribute_parameters_update.pybackend/tests/unit/core/schema_manager/test_manager_schema.pybackend/tests/unit/core/test_attribute.pybackend/tests/unit/core/test_schema.pychangelog/7778.changed.mdchangelog/7995.fixed.mdpyproject.tomlschema/schema.graphql
💤 Files with no reviewable changes (2)
- backend/infrahub/core/schema/attribute_schema.py
- backend/tests/integration/schema_lifecycle/test_attribute_parameters_update.py
🧰 Additional context used
📓 Path-based instructions (8)
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Follow Markdown coding standards defined in
dev/guidelines/markdown.md
Files:
changelog/7995.fixed.mdchangelog/7778.changed.md
{backend/infrahub/core/schema/generated/**,backend/infrahub/core/protocols.py,frontend/app/src/shared/api/graphql/generated/**,frontend/app/src/shared/api/rest/types.generated.ts,schema/schema.graphql,schema/openapi.json}
📄 CodeRabbit inference engine (AGENTS.md)
Do not edit generated files in
backend/infrahub/core/schema/generated/,backend/infrahub/core/protocols.py,frontend/app/src/shared/api/graphql/generated/,frontend/app/src/shared/api/rest/types.generated.ts,schema/schema.graphql, andschema/openapi.json
Files:
schema/schema.graphql
**/*.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
Use Async whenever possible 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 code validation in PythonUse ruff and mypy to validate and lint Python files
Files:
backend/tests/unit/core/schema_manager/test_manager_schema.pybackend/infrahub/graphql/queries/diff/tree.pybackend/infrahub/graphql/app.pybackend/tests/unit/core/test_attribute.pybackend/infrahub/core/schema/schema_branch.pybackend/tests/unit/core/test_schema.pybackend/tests/conftest.py
backend/tests/**/*.py
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Run backend tests with
pytestor viainvoketasksTest files should mirror source structure:
infrahub/core/node.pyshould have corresponding test attests/unit/core/test_node.py
Files:
backend/tests/unit/core/schema_manager/test_manager_schema.pybackend/tests/unit/core/test_attribute.pybackend/tests/unit/core/test_schema.pybackend/tests/conftest.py
backend/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
backend/**/*.py: Follow Python coding standards defined indev/guidelines/backend/python.md
Use type hints for Python in backend code
backend/**/*.py: Use async/await for all I/O operations in Python code
Type hint all function parameters and return values in Python code
Use Pydantic models for data structures in Python code
Files:
backend/tests/unit/core/schema_manager/test_manager_schema.pybackend/infrahub/graphql/queries/diff/tree.pybackend/infrahub/graphql/app.pybackend/tests/unit/core/test_attribute.pybackend/infrahub/core/schema/schema_branch.pybackend/tests/unit/core/test_schema.pybackend/tests/conftest.py
backend/infrahub/graphql/**/*.py
📄 CodeRabbit inference engine (backend/AGENTS.md)
Ask for review approval before making core schema definition changes or adding new GraphQL mutations/queries
Files:
backend/infrahub/graphql/queries/diff/tree.pybackend/infrahub/graphql/app.py
backend/infrahub/**/*.py
📄 CodeRabbit inference engine (backend/AGENTS.md)
backend/infrahub/**/*.py: Never use unparameterized Cypher queries - always use parameterized queries to prevent injection
Never block the event loop with synchronous I/O operations
Files:
backend/infrahub/graphql/queries/diff/tree.pybackend/infrahub/graphql/app.pybackend/infrahub/core/schema/schema_branch.py
{pyproject.toml,uv.lock}
📄 CodeRabbit inference engine (.github/instructions/tooling.instructions.md)
Use uv to manage the Python project and its dependencies
Files:
pyproject.toml
🧠 Learnings (7)
📚 Learning: 2025-12-26T09:03:55.145Z
Learnt from: CR
Repo: opsmill/infrahub PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-26T09:03:55.145Z
Learning: Ask before making GraphQL schema modifications
Applied to files:
schema/schema.graphql
📚 Learning: 2025-12-26T09:04:09.935Z
Learnt from: CR
Repo: opsmill/infrahub PR: 0
File: backend/AGENTS.md:0-0
Timestamp: 2025-12-26T09:04:09.935Z
Learning: Applies to backend/infrahub/graphql/**/*.py : Ask for review approval before making core schema definition changes or adding new GraphQL mutations/queries
Applied to files:
schema/schema.graphql
📚 Learning: 2025-12-26T09:03:55.145Z
Learnt from: CR
Repo: opsmill/infrahub PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-26T09:03:55.145Z
Learning: Applies to {backend/infrahub/core/schema/generated/**,backend/infrahub/core/protocols.py,frontend/app/src/shared/api/graphql/generated/**,frontend/app/src/shared/api/rest/types.generated.ts,schema/schema.graphql,schema/openapi.json} : Do not edit generated files in `backend/infrahub/core/schema/generated/`, `backend/infrahub/core/protocols.py`, `frontend/app/src/shared/api/graphql/generated/`, `frontend/app/src/shared/api/rest/types.generated.ts`, `schema/schema.graphql`, and `schema/openapi.json`
Applied to files:
pyproject.toml
📚 Learning: 2025-12-26T09:03:55.145Z
Learnt from: CR
Repo: opsmill/infrahub PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-26T09:03:55.145Z
Learning: Applies to backend/**/*.py : Follow Python coding standards defined in `dev/guidelines/backend/python.md`
Applied to files:
pyproject.toml
📚 Learning: 2025-12-26T09:03:55.145Z
Learnt from: CR
Repo: opsmill/infrahub PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-26T09:03:55.145Z
Learning: Applies to backend/**/*.py : Use type hints for Python in backend code
Applied to files:
pyproject.toml
📚 Learning: 2025-12-26T09:04:09.935Z
Learnt from: CR
Repo: opsmill/infrahub PR: 0
File: backend/AGENTS.md:0-0
Timestamp: 2025-12-26T09:04:09.935Z
Learning: Applies to backend/**/*.py : Use async/await for all I/O operations in Python code
Applied to files:
backend/infrahub/graphql/app.py
📚 Learning: 2025-12-01T22:15:40.243Z
Learnt from: CR
Repo: opsmill/infrahub PR: 0
File: .github/instructions/python.instructions.md:0-0
Timestamp: 2025-12-01T22:15:40.243Z
Learning: Applies to **/*.py : Use `await` for asynchronous calls in Python
Applied to files:
backend/infrahub/graphql/app.py
🧬 Code graph analysis (4)
backend/tests/unit/core/schema_manager/test_manager_schema.py (1)
backend/infrahub/core/schema/schema_branch.py (3)
load_schema(566-598)get(324-357)duplicate(292-301)
backend/tests/unit/core/test_attribute.py (1)
backend/infrahub/core/schema/attribute_parameters.py (1)
TextAttributeParameters(28-58)
backend/infrahub/core/schema/schema_branch.py (1)
backend/infrahub/core/schema/attribute_parameters.py (2)
NumberPoolParameters(146-173)TextAttributeParameters(28-58)
backend/tests/unit/core/test_schema.py (3)
backend/infrahub/core/schema/__init__.py (1)
SchemaRoot(63-155)backend/infrahub/core/schema/attribute_schema.py (1)
TextAttributeSchema(234-248)backend/infrahub/core/schema/attribute_parameters.py (1)
TextAttributeParameters(28-58)
⏰ 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 (16)
backend/infrahub/graphql/app.py (1)
167-177: LGTM! Clean simplification of async handling.The removal of explicit type casts in favor of direct await/return is more concise and equally type-safe. The
isawaitablecheck ensures the response is handled correctly in both async and sync cases.pyproject.toml (1)
743-748: LGTM! More specific linter configuration.Moving the PLR0915 (Too many statements) ignore from the global
backend/tests/*.pypattern to a specific file (backend/tests/db_snapshot.py) improves code quality enforcement by not suppressing warnings unnecessarily across all test files.changelog/7778.changed.md (1)
1-1: LGTM! Accurate changelog entry.The changelog entry correctly documents the breaking change to DiffTreeSummary GraphQL type, making count fields non-optional. This aligns with the code changes in
backend/infrahub/graphql/queries/diff/tree.py.changelog/7995.fixed.md (1)
1-1: LGTM! Accurate changelog entry for warning fix.The changelog entry correctly documents the fix for deprecation warnings on attribute schema fields (regex, min_length, max_length), ensuring warnings only appear when these fields are actually used. This aligns with the Text attribute schema refactoring mentioned in the PR summary.
backend/infrahub/graphql/queries/diff/tree.py (2)
149-149: No action needed. The field is always provided by the summary model.The underlying
DiffSummaryCountersmodel initializesnum_unchangedwith a default value of 0, andDiffSummaryCounters.from_graph()applies anor 0fallback when creating instances from query results. Whensummary.model_dump(exclude={"from_time", "to_time"})is called at line 547, thenum_unchangedfield is always present with an integer value. If the summary result is None, the resolver returns early without instantiatingDiffTreeSummary. The change torequired=Trueis valid.
63-66: Verified: Count fields are always initialized in source models, making the required=True change safe.The underlying data models (EnrichedDiffRoot, EnrichedDiffNode, EnrichedDiffAttribute, etc.) inherit from
BaseSummaryinbackend/infrahub/core/diff/model/path.py, where all four count fields are initialized withdefault=0:
num_added: int = field(default=0, kw_only=True)num_updated: int = field(default=0, kw_only=True)num_removed: int = field(default=0, kw_only=True)num_conflicts: int = field(default=0, kw_only=True)These fields are guaranteed to be non-None integers in the source models, so making them required in the GraphQL schema is justified and reflects the actual data guarantees.
backend/tests/unit/core/schema_manager/test_manager_schema.py (1)
390-459: Text attribute reconciliation test correctly exercises both configuration stylesThe test cleanly covers both directions of reconciliation (parameters-only and top-level-only) and asserts that regex/min_length/max_length are synchronized between deprecated fields and
TextAttributeParametersafterload_schema. This is a good, focused regression test for the new reconciliation logic.backend/tests/unit/core/test_attribute.py (1)
317-345: Updated text-parameter validation test aligns with TextAttributeParameters usageConstructing the
NodeSchemawithAttributeSchema(..., parameters=TextAttributeParameters(...))and driving validation throughNode.init/newgives realistic coverage of min_length, max_length, and regex behavior. The checks look consistent with the parameter model and the rest of this file’s validation tests.backend/tests/unit/core/test_schema.py (1)
127-164: Warning test cases correctly capture non-deprecated parameter usageThese two
SchemaWarningTestCaseDataentries precisely assert that usingTextAttributeParametersformin_length/max_lengthandregexdoes not produce deprecation warnings, which matchesSchemaRoot.gather_warnings()semantics. This is a useful guard against regressions as you phase out the legacy top-level fields.backend/infrahub/core/schema/schema_branch.py (5)
57-57: LGTM!The import is correctly updated to include
TextAttributeParametersneeded for the new reconciliation logic.
504-512: LGTM!The
_text_attr_needs_reconciliationmethod correctly detects when deprecated top-level fields differ from parameter-based fields, enabling targeted updates only when necessary.
514-538: LGTM!The reconciliation logic correctly implements bidirectional sync with parameters taking precedence. The pattern is clear and consistently applied for all three fields (regex, min_length, max_length).
566-573: LGTM!The reconciliation is correctly positioned before the merge loop, ensuring that deprecated Text attribute fields are synchronized with parameters before items are processed and cached.
606-614: LGTM!The reconciliation call in
process_pre_validationcorrectly handles schemas loaded from the database, ensuring backward compatibility with schemas that may have been stored with only deprecated top-level fields.schema/schema.graphql (1)
6340-6343: Generated file changes look consistent.Per coding guidelines,
schema/schema.graphqlis a generated file. The changes making count fields (num_added,num_conflicts,num_removed,num_updated,num_unchanged) non-nullable across the Diff-related types are consistent with the backend changes inDiffSummaryCountsandDiffTreeSummarymentioned in the summary.Also applies to: 6356-6359, 6392-6395, 6404-6407, 6421-6426, 6441-6447
backend/tests/conftest.py (1)
127-127: Excellent systematic type annotation improvements!The systematic addition of explicit type hints across all pytest fixtures significantly improves type safety and code maintainability. The changes correctly:
- Add
Nonetype annotations for fixture dependencies that perform side effects- Specify concrete return types (
SchemaBranch,SchemaRoot,Branch, etc.)- Include proper
Generatortype parameters- Add parameter type hints (
str,int,pytest.FixtureRequest)- Convert
tmpdir_factory.mktemp()result toPathfor type consistency (line 488)These changes align perfectly with the coding guidelines requiring type hints for all function parameters and return values.
Also applies to: 179-181, 219-219, 274-276, 295-295, 309-309, 331-333, 349-349, 363-363, 375-375, 393-393, 419-419, 433-433, 448-448, 453-455, 479-479, 488-488, 501-501, 544-544, 592-594, 664-664, 690-690, 747-747, 753-753, 844-844, 904-904, 913-913, 920-922, 1017-1017, 1023-1023, 1072-1072, 1279-1279, 1326-1326, 1334-1334, 1441-1441, 1497-1497, 1564-1565, 1658-1660, 1738-1738
| def _reconcile_text_attribute_parameters(self, schema: SchemaRoot | None = None) -> None: | ||
| """Reconcile regex, min_length, max_length between deprecated fields and parameters for Text attributes. | ||
| Args: | ||
| schema: If provided, reconcile incoming schema data before merging. | ||
| If None, reconcile already-loaded schemas (e.g., from database). | ||
| """ | ||
| if schema: | ||
| # Incoming schema: modify in place | ||
| for item in schema.nodes + schema.generics: | ||
| for attr in item.attributes: | ||
| self._reconcile_text_attr(attr) | ||
| return | ||
|
|
||
| # Loaded schemas: need to duplicate before modifying | ||
| for name in self.all_names: | ||
| node = self.get(name=name, duplicate=False) | ||
|
|
||
| if not any(self._text_attr_needs_reconciliation(attr) for attr in node.attributes): | ||
| continue | ||
|
|
||
| node = node.duplicate() | ||
| for attr in node.attributes: | ||
| self._reconcile_text_attr(attr) | ||
| self.set(name=name, schema=node) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if extensions can contain attributes that might need reconciliation
rg -n "class NodeExtensionSchema" -A 30Repository: opsmill/infrahub
Length of output: 2489
🏁 Script executed:
#!/bin/bash
# Check the schema root structure to understand extensions
rg -n "class SchemaRoot" -A 30Repository: opsmill/infrahub
Length of output: 2547
🏁 Script executed:
#!/bin/bash
# Check load_schema to understand extension processing
rg -n "def load_schema" -A 100 | head -150Repository: opsmill/infrahub
Length of output: 18289
🏁 Script executed:
#!/bin/bash
# Find BaseNodeExtensionSchema definition
rg -n "class BaseNodeExtensionSchema" -A 20Repository: opsmill/infrahub
Length of output: 1646
🏁 Script executed:
#!/bin/bash
# Find the load_schema method in schema_branch.py
rg -n "def load_schema" backend/infrahub/core/schema/schema_branch.py -A 60 | head -100Repository: opsmill/infrahub
Length of output: 2924
🏁 Script executed:
#!/bin/bash
# Check how extensions are used/processed after load_schema
rg -n "\.extensions\." backend/infrahub/core/schema/schema_branch.pyRepository: opsmill/infrahub
Length of output: 118
Reconcile Text attributes in schema extensions.
Extensions can contain Text attributes with deprecated fields (regex, min_length, max_length), but _reconcile_text_attribute_parameters only processes schema.nodes and schema.generics when called with a schema argument. The method should also reconcile schema.extensions.nodes to ensure consistent handling across all schema components.
Code location
Lines 547-552 in _reconcile_text_attribute_parameters:
if schema:
# Incoming schema: modify in place
for item in schema.nodes + schema.generics:
for attr in item.attributes:
self._reconcile_text_attr(attr)
returnShould also include schema.extensions.nodes.
🤖 Prompt for AI Agents
In backend/infrahub/core/schema/schema_branch.py around lines 540 to 564, the
incoming-schema branch only reconciles schema.nodes and schema.generics but
omits schema.extensions.nodes, so Text attributes in extensions won't be
updated; update the code path that handles a provided schema to also iterate
over schema.extensions.nodes (e.g., include schema.extensions.nodes in the
iteration or extend the list) and call self._reconcile_text_attr on each
attribute there before returning.
Summary by CodeRabbit
Bug Fixes
Changes
✏️ Tip: You can customize this high-level summary in your review settings.