Merge 'develop' into 'stable' to prepare for SDK 1.17.0 #687
Conversation
- Added _hierarchy_support flag to detect hierarchical nodes - Initialized _hierarchical_data dict for ancestors/descendants - Created RelationshipManager instances for ancestors/descendants - Updated __getattr__ to return hierarchical data - Added query generation for ancestors/descendants in both async and sync versions Co-authored-by: BeArchiTek <[email protected]>
- Created 16 tests covering initialization, data handling, and query generation - Tests verify ancestors and descendants are properly initialized - Tests confirm query generation includes hierarchical fields - Tests validate both async and sync node implementations - All tests pass successfully Co-authored-by: BeArchiTek <[email protected]>
- Removed redundant 'if should_fetch' checks in both async and sync implementations - The checks were unnecessary since we already continue if should_fetch is False - All tests continue to pass Co-authored-by: BeArchiTek <[email protected]>
* IHS-176: Remove `is_visible` property from attribute metadata This PR removes `is_visible` property from attribute metadata. * add news fragment
Causes visual linting issues in the editor if the vale extension is installed
Set vale to ignore yaml files
Validate uv.lock in CI
* docs: add and streamline AGENTS.md files for AI coding assistants 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <[email protected]> Co-authored-by: Pete Crocker <[email protected]>
…upport Add ancestors and descendants support for hierarchical nodes
Merge 'stable' into 'develop' with resolved conflicts
Fix linting violations after merge
Merge stable into develop
Signed-off-by: Fatih Acar <[email protected]>
Merge stable into develop
Python 3.14
WalkthroughAdds hierarchical node support to InfrahubNode (async and sync) including new internal flags, storage, and processing methods; introduces unit tests for hierarchical behavior. Bumps Python support to include 3.14 and updates pyproject (requires-python <3.15), adds pydantic-settings and updates whenever. CI: adds 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
group/ci
![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
🧹 Nitpick comments (1)
AGENTS.md (1)
41-49: Markdown style rules are practical but could be more comprehensive.The key rules (language tags, blank lines, fenced code blocks) are good and address Vale integration. However, they do not explicitly cover the tone and consistency requirements mentioned in prior learnings. Consider supplementing with guidance on professional but approachable tone and consistent document structure patterns (or verify these are in docs/AGENTS.md).
Consider adding explicit tone/language guidance to the Markdown Style section, such as:
- Maintain a professional but approachable tone
- Use concise and direct language
- Follow consistent document structure patterns (e.g., Diataxis categories)
Or, if this guidance exists in docs/AGENTS.md, add a cross-reference here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
.github/workflows/ci.yml(2 hunks).vale.ini(1 hunks).vale/styles/spelling-exceptions.txt(3 hunks)AGENTS.md(1 hunks)CLAUDE.md(1 hunks)changelog/+08aa1a85.removed.md(1 hunks)changelog/+b4648cfa.added.md(1 hunks)docs/AGENTS.md(1 hunks)infrahub_sdk/ctl/AGENTS.md(1 hunks)infrahub_sdk/node/constants.py(1 hunks)infrahub_sdk/node/node.py(14 hunks)infrahub_sdk/pytest_plugin/AGENTS.md(1 hunks)infrahub_sdk/timestamp.py(2 hunks)pyproject.toml(2 hunks)tests/AGENTS.md(1 hunks)tests/unit/sdk/test_hierarchical_nodes.py(1 hunks)tests/unit/sdk/test_timestamp.py(1 hunks)tests/unit/sdk/test_utils.py(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
docs/**/*.{md,mdx}
📄 CodeRabbit inference engine (CLAUDE.md)
docs/**/*.{md,mdx}: Use professional but approachable tone, concise and direct language, and consistent document structure patterns following Diataxis categories in documentation
In documentation code examples: use proper language tags, include both async and sync examples using Tabs component where applicable, provide realistic examples, add validation steps, and use callouts for warnings and tips
Use domain-relevant Infrahub terminology (playbooks, branches, schemas, commits) consistently with Infrahub's data model and UI naming conventions in documentation
Validate documentation accuracy against the latest Infrahub version and follow markdown style as defined in.markdownlint.yamland Vale styles
Files:
docs/AGENTS.md
tests/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
tests/**/*.py: Use the custom pytest plugin (infrahub_sdk.pytest_plugin) providing fixtures for Infrahub clients, configuration, and support for testing checks, transforms, and queries
Organize unit tests intests/unit/for isolated component testing and integration tests intests/integration/for testing against real Infrahub instances
Files:
tests/unit/sdk/test_utils.pytests/unit/sdk/test_hierarchical_nodes.pytests/unit/sdk/test_timestamp.py
infrahub_sdk/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
infrahub_sdk/**/*.py: UseInfrahubClient(async) andInfrahubClientSync(sync) providing identical interfaces following the dual client pattern
Implement all operations following the dual async/sync pattern whereInfrahubClientprovides async methods andInfrahubClientSyncprovides sync equivalents
Files:
infrahub_sdk/timestamp.pyinfrahub_sdk/node/constants.pyinfrahub_sdk/node/node.py
infrahub_sdk/node/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Implement the Node system with lazy loading of attributes and relationships, supporting batch operations and automatic relationship handling
Files:
infrahub_sdk/node/constants.pyinfrahub_sdk/node/node.py
🧠 Learnings (16)
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to docs/**/*.{md,mdx} : Use domain-relevant Infrahub terminology (playbooks, branches, schemas, commits) consistently with Infrahub's data model and UI naming conventions in documentation
Applied to files:
CLAUDE.mddocs/AGENTS.mdAGENTS.md
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to docs/**/*.{md,mdx} : In documentation code examples: use proper language tags, include both async and sync examples using Tabs component where applicable, provide realistic examples, add validation steps, and use callouts for warnings and tips
Applied to files:
CLAUDE.mddocs/AGENTS.md
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to docs/**/*how-to*.mdx : For documentation How-to guides: include prerequisites section, step-by-step instructions with code snippets, validation steps, and related resources links
Applied to files:
CLAUDE.mddocs/AGENTS.md
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to docs/**/*.{md,mdx} : Use professional but approachable tone, concise and direct language, and consistent document structure patterns following Diataxis categories in documentation
Applied to files:
CLAUDE.mddocs/AGENTS.md
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to tests/**/*.py : Use the custom pytest plugin (`infrahub_sdk.pytest_plugin`) providing fixtures for Infrahub clients, configuration, and support for testing checks, transforms, and queries
Applied to files:
infrahub_sdk/pytest_plugin/AGENTS.mdtests/AGENTS.md
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to tests/**/*.py : Organize unit tests in `tests/unit/` for isolated component testing and integration tests in `tests/integration/` for testing against real Infrahub instances
Applied to files:
infrahub_sdk/pytest_plugin/AGENTS.mdtests/AGENTS.mdtests/unit/sdk/test_hierarchical_nodes.py
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to docs/**/*.mdx : Follow the Diataxis framework for documentation with four types: Tutorials (learning-oriented), How-to guides (task-oriented), Explanation (understanding-oriented), and Reference (information-oriented)
Applied to files:
docs/AGENTS.md
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to docs/**/*.{md,mdx} : Validate documentation accuracy against the latest Infrahub version and follow markdown style as defined in `.markdownlint.yaml` and Vale styles
Applied to files:
docs/AGENTS.md
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to docs/**/*topic*.mdx : For documentation Topics (understanding-oriented): include concepts & definitions, background & context, architecture & design with diagrams, connections to other concepts, and further reading links
Applied to files:
docs/AGENTS.md
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Run unit tests using `uv run pytest --cov infrahub_sdk tests/unit/`
Applied to files:
tests/AGENTS.md
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to infrahub_sdk/**/*.py : Implement all operations following the dual async/sync pattern where `InfrahubClient` provides async methods and `InfrahubClientSync` provides sync equivalents
Applied to files:
tests/AGENTS.mdAGENTS.mdinfrahub_sdk/ctl/AGENTS.md
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Run linting using `uv run invoke lint` which includes ruff, mypy, yamllint, and markdownlint
Applied to files:
.github/workflows/ci.yml
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to infrahub_sdk/**/*.py : Use `InfrahubClient` (async) and `InfrahubClientSync` (sync) providing identical interfaces following the dual client pattern
Applied to files:
AGENTS.mdinfrahub_sdk/ctl/AGENTS.mdinfrahub_sdk/node/node.py
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to infrahub_sdk/ctl/**/*.py : Implement CLI commands using Typer for schema management, transformations, checks, branch operations, and object management in `infrahub_sdk/ctl/`
Applied to files:
infrahub_sdk/ctl/AGENTS.md
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to infrahub_sdk/node/**/*.py : Implement the Node system with lazy loading of attributes and relationships, supporting batch operations and automatic relationship handling
Applied to files:
infrahub_sdk/node/node.py
📚 Learning: 2025-11-25T13:23:15.190Z
Learnt from: wvandeun
Repo: opsmill/infrahub-sdk-python PR: 637
File: infrahub_sdk/operation.py:74-76
Timestamp: 2025-11-25T13:23:15.190Z
Learning: In infrahub_sdk/operation.py, the recursive=True parameter in _process_relationships is a temporary workaround to access Proposed Changes data. This will be replaced with proper object-level metadata implementation in version 1.7.
Applied to files:
infrahub_sdk/node/node.py
🧬 Code graph analysis (2)
tests/unit/sdk/test_utils.py (2)
infrahub_sdk/timestamp.py (1)
subtract(184-212)infrahub_sdk/utils.py (1)
calculate_time_diff(355-378)
infrahub_sdk/node/node.py (3)
infrahub_sdk/schema/main.py (6)
GenericSchemaAPI(288-292)ProfileSchemaAPI(317-318)RelationshipCardinality(17-19)RelationshipKind(28-36)RelationshipSchemaAPI(145-151)TemplateSchemaAPI(321-322)infrahub_sdk/node/related_node.py (8)
kind(123-126)RelatedNode(181-225)peer(212-213)peer(259-260)get(215-225)get(262-272)_generate_query_data(154-178)RelatedNodeSync(228-272)infrahub_sdk/node/relationship.py (5)
RelationshipManager(98-218)extend(194-197)extend(317-320)_generate_query_data(67-95)RelationshipManagerSync(221-340)
⏰ 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 (40)
changelog/+08aa1a85.removed.md (1)
1-1: Changelog entry format is correct.The entry follows the towncrier convention and clearly documents the removal of the
is_visibleproperty. The content is appropriately concise for a changelog entry.Please verify that the
is_visibleproperty removal is actually implemented elsewhere in this PR. Since this is a breaking change for SDK 1.17.0, you may also want to consider adding migration guidance in release notes if this property was widely used.CLAUDE.md (1)
1-2: Verify documentation consolidation captures prior guidance.CLAUDE.md is being replaced entirely with a reference to @AGENTS.md. The prior content included four guidance blocks (domain terminology, code examples, how-to structure, and tone). Ensure the new AGENTS.md structure (root + subdirectories) comprehensively documents all prior guidance so that AI assistants and developers have equivalent or better guidance.
Review the new AGENTS.md files and verify that all four prior guidance themes are preserved or improved across the hierarchy. Specifically:
- Root AGENTS.md should reference domain terminology standards
- docs/AGENTS.md should cover code example style and how-to structure
- Root AGENTS.md should document tone and consistency requirements
changelog/+b4648cfa.added.md (1)
1-1: LGTM!The changelog entry accurately reflects the Python 3.14 support being added and follows the towncrier format correctly.
pyproject.toml (3)
10-10: Python 3.14 support additions are consistent.The requires-python constraint and Python 3.14 classifier are correctly updated and align with the CI workflow matrix changes. The changes appear sound for supporting Python 3.14 released in October 2025.
Also applies to: 18-18
22-22: pydantic-settings>=2.0 is actively used in the codebase.This dependency is imported in multiple modules (infrahub_sdk/config.py, infrahub_sdk/ctl/config.py, infrahub_sdk/recorder.py, infrahub_sdk/playback.py, and tasks.py) for BaseSettings, SettingsConfigDict, and EnvSettingsSource. The dependency addition is justified and necessary.
Likely an incorrect or invalid review comment.
27-27: whenever 0.9.3 upgrade is compatible with current codebase.The upgrade from
>=0.7.2,<0.8.0to>=0.9.3,<0.10.0introduces only a bug fix for timezone offsets before 1950 (Issue #296) with no breaking changes. The codebase uses standard whenever APIs (parse_iso(),to_tz(),assume_utc(),assume_tz(),at(),subtract(),add(),timestamp(),py_datetime(),difference(),in_hrs_mins_secs_nanos()) that remain stable across the versions. No code changes are required..vale/styles/spelling-exceptions.txt (1)
23-23: LGTM!The four new spelling exceptions (callouts, Diataxis, frontmatter, Typer) are all appropriate for the Infrahub SDK project and align with the documentation restructuring and CLI tooling present in this PR. Entries maintain alphabetical ordering correctly.
Also applies to: 27-27, 47-47, 120-120
.vale.ini (1)
19-22: LGTM!The new [.yml] section appropriately mirrors the existing [.toml] behavior to disable Vale formatting rules for YAML files. This prevents false positives in GitHub Actions workflows and other CI/CD YAML configurations.
.github/workflows/ci.yml (2)
130-151: New uv-lock-check job is well-structured.The uv-lock-check job correctly validates pyproject.toml consistency with uv.lock in offline mode. Job dependencies, environment setup, and logic appear sound. However, note that the job uses
actions/checkout@v5while other jobs in the workflow use@v6.Ensure that the checkout action version is consistent across the CI workflow. If other jobs use @v6, consider updating line 141 to use the same version for consistency.
254-254: Python 3.14 matrix addition is correct.The addition of Python 3.14 to the unit-tests matrix aligns with pyproject.toml changes and ensures the SDK is tested against the newly supported version. Positioning in the list is correct.
tests/AGENTS.md (1)
1-60: LGTM!The new tests/AGENTS.md provides clear, comprehensive guidance on test organization, commands, patterns, and boundaries. The content aligns well with the learnings about async auto-mode, the custom pytest plugin, separation of unit and integration tests, and the requirement to use httpx_mock. Code examples are realistic and demonstrate proper async/sync test patterns.
AGENTS.md (3)
1-77: AGENTS.md provides good project guidance, but verify complete coverage of prior CLAUDE.md themes.The new root AGENTS.md is well-structured and covers commands, tech stack, code patterns, structure, markdown style, boundaries, and subdirectory references. However, compared to the learnings from prior CLAUDE.md guidance, some themes appear deferred or incomplete:
Domain terminology consistency (learnings: "Use domain-relevant Infrahub terminology...") is not explicitly documented in this file. Verify it's covered in docs/AGENTS.md.
How-to guide structure (learnings: "prerequisites section, step-by-step instructions with code snippets, validation steps, and related resources links") is referenced to docs/AGENTS.md but not verified here.
Tone and language (learnings: "professional but approachable tone, concise and direct language, consistent document structure patterns") is only partially addressed via "Markdown Style" rules and is not explicitly stated.
Verify that the subdirectory AGENTS.md files (particularly docs/AGENTS.md) comprehensively document the domain terminology consistency, how-to structure requirements, and tone guidelines that were in prior CLAUDE.md. If these are not covered, add explicit guidance to either this file or the relevant subdirectory guide.
19-28: Code pattern example is good.The dual async/sync example clearly demonstrates the pattern and includes realistic method calls (get, save). This aligns well with the learnings about implementing operations in the dual async/sync pattern.
30-39: Project structure is accurate and helpful.The structure clearly maps main modules to their purposes (client.py, config.py, node/, ctl/, pytest_plugin/). This aligns with the PR's hierarchical node support and CLI tooling mentioned in the summary.
infrahub_sdk/pytest_plugin/AGENTS.md (1)
1-67: LGTM! Clear and well-structured documentation.The documentation effectively covers the pytest plugin's structure, usage patterns, and boundaries. The examples are clear and the table format for test kinds is easy to reference.
docs/AGENTS.md (1)
1-60: LGTM! Comprehensive documentation guidance.The documentation provides clear commands, structure explanations, and MDX patterns. The boundaries section effectively enforces the requirement for both async and sync examples using Tabs, which aligns with the coding guidelines.
infrahub_sdk/ctl/AGENTS.md (1)
1-67: LGTM! Clear CLI documentation with practical examples.The documentation effectively covers the CLI command patterns, file organization, and boundaries. The command pattern example demonstrates the required decorators and client initialization approach, which aligns with the coding guidelines for the CLI layer.
tests/unit/sdk/test_utils.py (1)
219-233: LGTM! Consistent with timestamp API updates.The changes replace
format_common_iso()withformat_iso(), aligning with the broader timestamp API refactoring seen ininfrahub_sdk/timestamp.py. Test expectations remain unchanged, confirming behavioral consistency.infrahub_sdk/node/constants.py (1)
20-21: LGTM! Follows established patterns.The new constant follows the existing naming convention and provides a clear error message for hierarchical field operations. It's consistent with other feature-specific error messages in the file.
tests/unit/sdk/test_timestamp.py (1)
30-43: LGTM! Updated to reflect new parsing API.The test correctly updates from
parse_common_isotoparse_isoand frominstant()toto_instant(), aligning with the API changes ininfrahub_sdk/timestamp.py. Test logic and validation remain sound.infrahub_sdk/timestamp.py (2)
9-9: LGTM! Import updated for PlainDateTime.The import correctly replaces
LocalDateTimewithPlainDateTime, aligning with the parsing method updates throughout the file.
52-97: LGTM! Consistent parsing API refactoring.The
_parse_stringmethod has been systematically updated to useparse_isofor all datetime types (ZonedDateTime, Instant, PlainDateTime, OffsetDateTime, Date). The control flow and UTC normalization logic remain unchanged, ensuring behavioral consistency. The refactoring is complete and internally consistent.tests/unit/sdk/test_hierarchical_nodes.py (6)
18-51: LGTM! Well-structured test fixture.The fixture creates a comprehensive hierarchical schema with parent/children relationships. The manual setting of the
hierarchyfield with an explanatory comment is appropriate for testing purposes and clearly documents the expected backend behavior.
53-109: LGTM! Comprehensive hierarchy support tests.The tests thoroughly validate hierarchy support detection and field initialization. The parametrization for both standard and sync clients ensures consistent behavior across both implementations. The assertions verify proper field types (RelatedNode for parent, RelationshipManager for children/ancestors/descendants) and schema properties.
111-228: LGTM! Thorough data initialization tests.The tests comprehensively validate hierarchical node initialization with parent, children, ancestors, and descendants data. Each test verifies proper parsing of nested data structures, peer object creation, and initialization flags. Parametrization ensures consistency across async and sync implementations.
230-255: LGTM! Important negative test case.The test validates that non-hierarchical nodes properly reject hierarchical field access with descriptive AttributeError messages. This defensive testing ensures the hierarchical feature is properly gated and prevents confusion when working with non-hierarchical nodes.
257-396: LGTM! Comprehensive query generation tests.The tests thoroughly validate query generation for hierarchical fields across multiple scenarios: individual field inclusion, prefetch_relationships, and exclude parameters. The schema cache pre-population is a good practice to avoid HTTP requests during tests. Both async and sync implementations are tested, and assertions verify proper fragment usage for hierarchical peer data.
497-546: LGTM! Critical infinite recursion prevention tests.These tests validate a crucial safety mechanism: including hierarchical children relationships does not cause infinite recursion in query generation. The assertions verify that hierarchical fields (parent, children, ancestors, descendants) are NOT nested in the peer data of children nodes, preventing potential infinite loops in self-referential hierarchical structures. Both async and sync implementations are tested.
infrahub_sdk/node/node.py (12)
10-17: LGTM!The added imports for
ProfileSchemaAPI,TemplateSchemaAPI, andRelationshipSchemaAPIare correctly introduced to support the hierarchical node feature.
409-411: LGTM!The
_validate_hierarchy_supportmethod correctly follows the established pattern from_validate_artifact_support. It's properly scaffolded for future use when hierarchical field fetch operations need validation.
508-508: LGTM!The
_hierarchical_datacontainer is correctly initialized before the superclass__init__call, ensuring it's available when_init_relationshipsis invoked.
562-630: LGTM!The hierarchical field initialization correctly creates pseudo-schemas for
parent,children,ancestors, anddescendantswith appropriate cardinality and read-only settings. The structure follows the existing relationship initialization pattern.
639-640: LGTM!The
__getattr__extension correctly exposes hierarchical fields following the established pattern for attributes and relationships.
729-778: LGTM!The
_process_hierarchical_fieldsmethod is well-implemented with proper recursion prevention by excluding hierarchical fields from peer data. The fragment-based GraphQL query structure correctly mirrors the existing hierarchy relationship handling.
907-936: LGTM!The integration of hierarchical field processing into
generate_query_data_nodeis correctly placed after regular relationship processing. The explicit type annotation and defensiveelse: continuebranch improve code clarity.
1297-1297: LGTM!The sync variant correctly uses
RelatedNodeSync | RelationshipManagerSynctypes.
1352-1423: LGTM!The sync variant correctly mirrors the async implementation with appropriate sync types, following the dual async/sync pattern per coding guidelines.
1432-1433: LGTM!The sync
__getattr__correctly mirrors the async implementation for hierarchical field access.
1515-1564: LGTM!The sync
_process_hierarchical_fieldscorrectly mirrors the async version with appropriate synchronous calls and types, adhering to the dual client pattern.
1688-1721: LGTM!The sync
generate_query_data_nodechanges correctly mirror the async version, with proper integration of hierarchical field processing.
|
@BeArchiTek I've opened a new pull request, #688, to work on those changes. Once the pull request is ready, I'll request review from you. |
Fixed incorrect boolean logic that used 'or' instead of tuple syntax in isinstance check for ProfileSchemaAPI, GenericSchemaAPI, and TemplateSchemaAPI. This ensures hierarchy support is correctly detected only for node schemas that are not Profile, Generic, or Template types. Co-authored-by: BeArchiTek <[email protected]>
Fix boolean logic error in hierarchy support detection
Deploying infrahub-sdk-python with
|
| Latest commit: |
e9e214a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e0de4bf3.infrahub-sdk-python.pages.dev |
| Branch Preview URL: | https://develop.infrahub-sdk-python.pages.dev |
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## stable #687 +/- ##
==========================================
- Coverage 76.54% 76.03% -0.52%
==========================================
Files 113 113
Lines 10189 9742 -447
Branches 1675 1490 -185
==========================================
- Hits 7799 7407 -392
+ Misses 1872 1840 -32
+ Partials 518 495 -23
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
infrahub_sdk/node/node.py (1)
74-78: The boolean logic fix has been correctly applied.The code now uses the tuple form
not isinstance(schema, (ProfileSchemaAPI, GenericSchemaAPI, TemplateSchemaAPI))which correctly checks that the schema is none of these types, rather than the previous buggyorcondition.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
infrahub_sdk/node/node.py(14 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
infrahub_sdk/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
infrahub_sdk/**/*.py: UseInfrahubClient(async) andInfrahubClientSync(sync) providing identical interfaces following the dual client pattern
Implement all operations following the dual async/sync pattern whereInfrahubClientprovides async methods andInfrahubClientSyncprovides sync equivalents
Files:
infrahub_sdk/node/node.py
infrahub_sdk/node/**/*.py
📄 CodeRabbit inference engine (CLAUDE.md)
Implement the Node system with lazy loading of attributes and relationships, supporting batch operations and automatic relationship handling
Files:
infrahub_sdk/node/node.py
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to infrahub_sdk/node/**/*.py : Implement the Node system with lazy loading of attributes and relationships, supporting batch operations and automatic relationship handling
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to infrahub_sdk/**/*.py : Implement all operations following the dual async/sync pattern where `InfrahubClient` provides async methods and `InfrahubClientSync` provides sync equivalents
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to tests/**/*.py : Organize unit tests in `tests/unit/` for isolated component testing and integration tests in `tests/integration/` for testing against real Infrahub instances
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to infrahub_sdk/node/**/*.py : Implement the Node system with lazy loading of attributes and relationships, supporting batch operations and automatic relationship handling
Applied to files:
infrahub_sdk/node/node.py
📚 Learning: 2025-11-25T13:23:15.190Z
Learnt from: wvandeun
Repo: opsmill/infrahub-sdk-python PR: 637
File: infrahub_sdk/operation.py:74-76
Timestamp: 2025-11-25T13:23:15.190Z
Learning: In infrahub_sdk/operation.py, the recursive=True parameter in _process_relationships is a temporary workaround to access Proposed Changes data. This will be replaced with proper object-level metadata implementation in version 1.7.
Applied to files:
infrahub_sdk/node/node.py
📚 Learning: 2025-12-01T19:42:19.012Z
Learnt from: CR
Repo: opsmill/infrahub-sdk-python PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-01T19:42:19.012Z
Learning: Applies to infrahub_sdk/**/*.py : Use `InfrahubClient` (async) and `InfrahubClientSync` (sync) providing identical interfaces following the dual client pattern
Applied to files:
infrahub_sdk/node/node.py
⏰ 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). (15)
- GitHub Check: unit-tests (3.10)
- GitHub Check: unit-tests (3.14)
- GitHub Check: unit-tests (3.12)
- GitHub Check: unit-tests (3.13)
- GitHub Check: unit-tests (3.11)
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: documentation
- GitHub Check: unit-tests (3.11)
- GitHub Check: unit-tests (3.14)
- GitHub Check: unit-tests (3.10)
- GitHub Check: unit-tests (3.12)
- GitHub Check: documentation
- GitHub Check: integration-tests-latest-infrahub
- GitHub Check: unit-tests (3.13)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (6)
infrahub_sdk/node/node.py (6)
10-17: LGTM: Import additions support hierarchical functionality.The new schema and relationship imports are necessary for the hierarchical node feature implementation.
405-407: LGTM: Validation method follows established pattern.The
_validate_hierarchy_supportmethod correctly follows the same pattern as_validate_artifact_supportand_validate_artifact_definition_support.
635-636: LGTM: Hierarchical data access follows established pattern.The addition of hierarchical data to
__getattr__is consistent with how attributes and relationships are exposed.
725-775: LGTM: Hierarchical field processing correctly prevents infinite recursion.The implementation properly guards against infinite recursion by excluding hierarchical fields (parent, children, ancestors, descendants) from peer data when fetching hierarchical relationships. The fragment usage for hierarchical fields mirrors the existing hierarchy relationship pattern, which is appropriate.
903-932: LGTM: Control flow improvements and hierarchical field integration.The addition of
else: continueat lines 916-917 makes the control flow explicit when relationship schema doesn't match expected cardinalities, improving code clarity. The call to_process_hierarchical_fieldsappropriately integrates hierarchical relationships into the query generation.
1293-1293: LGTM: Synchronous implementation correctly mirrors async version.The synchronous hierarchical support implementation properly mirrors the async version while using appropriate sync types (
InfrahubNodeSync,RelatedNodeSync,RelationshipManagerSync) and non-async method calls. This maintains consistency with the dual async/sync pattern required by the coding guidelines.Based on coding guidelines, implementing both async and sync variants following the dual client pattern.
Also applies to: 1348-1419, 1428-1429, 1511-1561, 1684-1718
| # Initialize parent, children, ancestors and descendants for hierarchical nodes | ||
| if self._hierarchy_support: | ||
| # Create pseudo-schema for parent (cardinality one) | ||
| parent_schema = RelationshipSchemaAPI( | ||
| name="parent", | ||
| peer=self._schema.hierarchy, # type: ignore[union-attr, arg-type] | ||
| kind=RelationshipKind.HIERARCHY, | ||
| cardinality="one", | ||
| optional=True, | ||
| ) | ||
| parent_data = data.get("parent", None) if isinstance(data, dict) else None | ||
| self._hierarchical_data["parent"] = RelatedNode( | ||
| name="parent", | ||
| client=self._client, | ||
| branch=self._branch, | ||
| schema=parent_schema, | ||
| data=parent_data, | ||
| ) | ||
| # Create pseudo-schema for children (many cardinality) | ||
| children_schema = RelationshipSchemaAPI( | ||
| name="children", | ||
| peer=self._schema.hierarchy, # type: ignore[union-attr, arg-type] | ||
| kind=RelationshipKind.HIERARCHY, | ||
| cardinality="many", | ||
| optional=True, | ||
| ) | ||
| children_data = data.get("children", None) if isinstance(data, dict) else None | ||
| self._hierarchical_data["children"] = RelationshipManager( | ||
| name="children", | ||
| client=self._client, | ||
| node=self, | ||
| branch=self._branch, | ||
| schema=children_schema, | ||
| data=children_data, | ||
| ) | ||
| # Create pseudo-schema for ancestors (read-only, many cardinality) | ||
| ancestors_schema = RelationshipSchemaAPI( | ||
| name="ancestors", | ||
| peer=self._schema.hierarchy, # type: ignore[union-attr, arg-type] | ||
| cardinality="many", | ||
| read_only=True, | ||
| optional=True, | ||
| ) | ||
| ancestors_data = data.get("ancestors", None) if isinstance(data, dict) else None | ||
| self._hierarchical_data["ancestors"] = RelationshipManager( | ||
| name="ancestors", | ||
| client=self._client, | ||
| node=self, | ||
| branch=self._branch, | ||
| schema=ancestors_schema, | ||
| data=ancestors_data, | ||
| ) | ||
| # Create pseudo-schema for descendants (read-only, many cardinality) | ||
| descendants_schema = RelationshipSchemaAPI( | ||
| name="descendants", | ||
| peer=self._schema.hierarchy, # type: ignore[union-attr, arg-type] | ||
| cardinality="many", | ||
| read_only=True, | ||
| optional=True, | ||
| ) | ||
| descendants_data = data.get("descendants", None) if isinstance(data, dict) else None | ||
| self._hierarchical_data["descendants"] = RelationshipManager( | ||
| name="descendants", | ||
| client=self._client, | ||
| node=self, | ||
| branch=self._branch, | ||
| schema=descendants_schema, | ||
| data=descendants_data, | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Consider extracting hierarchical relationship initialization to reduce duplication.
The hierarchical relationship initialization code (parent, children, ancestors, descendants) is duplicated between the async version here and the sync version at lines 1348-1419. While the dual async/sync pattern requires separate classes, the pseudo-schema creation logic is identical and could be extracted to a helper method or to the base class to improve maintainability.
Additionally, the hardcoded field names ("parent", "children", "ancestors", "descendants") appear in multiple locations throughout the file. Consider defining these as module-level constants to prevent typos and facilitate future changes.
Based on coding guidelines, the dual async/sync pattern requires maintaining both implementations.
🤖 Prompt for AI Agents
In infrahub_sdk/node/node.py around lines 558 to 626, the hierarchical
relationship initialization for parent/children/ancestors/descendants is
duplicated elsewhere (sync version at 1348-1419); extract the repeated
pseudo-schema and Relationship/RelatedNode construction into a shared helper
method on the common base class (or a private module-level function) that
accepts the node instance, schema.hierarchy, branch, client and incoming data
dict and returns the prepared hierarchical entries; also define module-level
constants for the field names ("PARENT", "CHILDREN", "ANCESTORS", "DESCENDANTS"
or similar) and replace the hardcoded strings in both async and sync
implementations to use those constants so both paths call the same helper and
avoid duplication and typos.
Summary by CodeRabbit
New Features
Chores
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.