fix: hydrate related nodes via include= on host fetch#74
Open
wvandeun wants to merge 2 commits into
Open
Conversation
wvandeun
force-pushed
the
fix-ip-relationship
branch
from
0631754 to
2a4f4b6
Compare
Deploying nornir-infrahub with
|
| Latest commit: |
b4ef0f3
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d834f3fd.nornir-infrahub.pages.dev |
| Branch Preview URL: | https://fix-ip-relationship.nornir-infrahub.pages.dev |
wvandeun
force-pushed
the
fix-ip-relationship
branch
from
2a4f4b6 to
ba0e251
Compare
Resolving a schema_mapping like `primary_address.address` raised `AttributeError: 'NoneType' object has no attribute 'ip'` because the SDK store was returning a stub peer with `address.value = None`. Root cause: the inventory pre-fetched every related kind separately with `populate_store=True`, then fetched host nodes — also with `populate_store=True`. The host fetch returned each relation peer as a minimal stub (id/typename only) and overwrote the fully-hydrated peer that had just been cached. `resolve_node_mapping` then walked `<relation>.peer` into the stub and read `None`. Replace the two-phase fetch with a single host-node fetch that uses `include=` to hydrate exactly the relations referenced by the schema_mappings and group_mappings. Drop `get_related_nodes`, the `extra_nodes` attribute, and the pre-fetch loop. `include=` on the SDK is strictly one-hop, so reject multi-hop mappings and mappings that reference a name that is not a relationship on the host kind, both with clear errors at init time.
lancamat1
added a commit
that referenced
this pull request
May 25, 2026
PR #74 (wvandeun, open since 2026-04-23) fixes the same bug with a more comprehensive approach: deletes the redundant pre-fetch loop and adds construction-time validation for multi-hop and unknown-relationship mappings. Defer to that PR. Reverts: - the 3-line auto-include in InfrahubInventory.__init__ - the two unit tests asserting the auto-include behavior - the AGENTS.md note describing the auto-include - the 4 user-doc files (configuring-schema-mappings.mdx, references/plugins/infrahub_inventory.mdx, getting-started.mdx, your-first-automation-workflow.mdx) restored to origin/stable Integration tests for schema_mappings/group_mappings remain. They will fail locally without PR #74 applied; that's expected and documents the problem #74 fixes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
3 tasks
The example told users to manually set host_node.include for relations referenced in mappings, which was a workaround for the bug the parent commit fixes. The include set is now derived automatically. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
lancamat1
force-pushed
the
fix-ip-relationship
branch
from
ba0e251 to
b4ef0f3
Compare
lancamat1
added a commit
that referenced
this pull request
May 26, 2026
These tests previously assumed the auto-include behavior we reverted (deferring to PR #74). Without auto-include, the SDK overwrites the hydrated peer cached by the pre-fetch loop with a stub on the host fetch, so resolve_node_mapping reads None. Passing include explicitly on the host_node tells the SDK to hydrate those relations directly, which works on both current code and PR #74's branch (where it gets merged with auto-derived). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
minitriga
pushed a commit
that referenced
this pull request
Jun 4, 2026
test: add integration test suite for InfrahubInventory (NORNIR-5) (#79) Add an integration test suite under tests/integration/ that validates InfrahubInventory against a live Infrahub instance spun up via infrahub-testcontainers. The suite covers 14 tests across 6 classes: - Basic inventory loading (host count, host names) - schema_mappings resolving relationship attributes (primary_address.address, platform.nornir_platform) - group_mappings creating groups from related nodes (site.name) - CoreStandardGroup membership via member_of_groups - host_node filters (by site, by platform) - Branch routing (hosts on a feature branch are only visible when branch= is set) Test infrastructure: - NornirInfrahubIntegration base class (tests/integration/conftest.py) spins up a fresh Infrahub container per test class and bootstraps schema plus seed data (devices, sites, platforms, IPs, a standard group, and a feature branch) - Admin token is read from infrahub_testcontainers PROJECT_ENV_VARIABLES, not hardcoded - Tests are marked "integration" and excluded from the default pytest run via addopts ("-m 'not integration'"), so existing CI is unaffected; run them with `pytest -m integration` (requires Docker) - The pytest-infrahub-performance-test plugin is disabled in addopts to work around a psutil.cpu_freq() crash on macOS arm64 / Python 3.13 Notes: - Mapping tests pass explicit host_node.include to hydrate relations; the auto-include fix for relationship mappings is deferred to #74 (NORNIR-5) - Artifact task tests and CI wiring (huge-runners) are follow-ups - Adds infrahub-testcontainers>=1.9.3 as a dev dependency Closes #78
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
include=on the host-node fetch, derived from schema/group mappings.get_related_nodes,extra_nodes, and the separate pre-fetch loop.relationship, with clear error messages at init time.
Root cause
Both
filters(kind=<peer>, populate_store=True)and the subsequentfilters(kind=<host>, populate_store=True)write to the SDK store. Thehost-node fetch returns each relation peer as a minimal stub and
overwrites the fully-hydrated peer that was cached a moment earlier.
resolve_node_mappingthen readspeer.<attr>.valueasNoneandip_interface_to_ip_string(None)raisesAttributeError: 'NoneType' object has no attribute 'ip'.SDK
include=is one-hop only(
infrahub_sdk/node/node.py:1033— plain list-membership check onrel_name, peer hydration does not propagateincludeorprefetch_relationships). Since the previous implementation did notsupport multi-hop mappings either (
get_related_nodesonly walked one level), this change does not regress the feature surface.Test plan
uv run pytest tests/unit/— all tests pass, including four newtests covering include merging, single-segment no-op, multi-hop
rejection, and unknown-relationship rejection.
uv run python flow.pyagainst a live Infrahub returns alldevices with non-null
hostname(IP),platform, andsite.*groups.