Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions backend/infrahub/core/node/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -212,16 +212,17 @@ async def extract_peer_data(
obj_peer_data[rel_name] = parent_obj
continue

rel_peer_ids = []
rel_peers = []
for relationship in relationships:
# deeper templates are handled in the next level of recursion
if await _peer_is_a_template(db=db, relationship=relationship):
continue
rel_peer_ids.append({"id": relationship.peer_id})
# The peer is already read, so an id would send the checks and the write back to the database.
rel_peers.append({"id": relationship.get_peer_in_hand() or relationship.peer_id})

# Only set the relationship data if there are actual peers to set
if rel_peer_ids:
obj_peer_data[rel_name] = rel_peer_ids
if rel_peers:
obj_peer_data[rel_name] = rel_peers

if rel_manager.schema.kind == RelationshipKind.PROFILE:
obj_peer_data[rel_name] = peer_ids
Expand Down
17 changes: 10 additions & 7 deletions backend/infrahub/core/relationship/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,17 +192,20 @@ def get_peer_id(self) -> str:
return self.peer_id

def get_peer_kind(self) -> str:
if not self._peer or isinstance(self._peer, str):
return self.schema.peer

return self._peer.get_kind()
peer = self.get_peer_in_hand()
return peer.get_kind() if peer is not None else self.schema.peer

def get_concrete_peer_kind(self) -> str | None:
"""Return the peer's concrete kind, or None when only the schema's (possibly generic) peer kind is known."""
if self._peer and not isinstance(self._peer, str):
return self._peer.get_kind()
peer = self.get_peer_in_hand()
return peer.get_kind() if peer is not None else self._resolved_peer_kind

return self._resolved_peer_kind
def get_peer_in_hand(self) -> Node | None:
"""Return the peer as the node it is, or None when this relationship holds only its id."""
if self._peer is None or isinstance(self._peer, str):
return None

return self._peer

@property
def node_id(self) -> str:
Expand Down
101 changes: 90 additions & 11 deletions backend/tests/component/templates/test_template_reads.py
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
from __future__ import annotations

from typing import TYPE_CHECKING
from copy import deepcopy
from typing import TYPE_CHECKING, Any

import pytest

from infrahub.core.constants import RelationshipCardinality, RelationshipKind
from infrahub.core.manager import NodeManager
from infrahub.core.node import Node
from infrahub.core.node.create import create_node
from infrahub.core.query.node import NodeListGetInfoQuery, NodeListGetRelationshipsQuery
from infrahub.core.query.node import NodeListGetAttributeQuery, NodeListGetInfoQuery, NodeListGetRelationshipsQuery
from infrahub.core.query.relationship import RelationshipGetPeerQuery
from infrahub.core.registry import registry
from infrahub.core.schema import RelationshipSchema
from tests.constants import TestKind
from tests.helpers.db_query_counter import CountingInfrahubDatabase
from tests.helpers.schema import DEVICE_SCHEMA, load_schema
from tests.helpers.schema import DEVICE_SCHEMA, TAG, load_schema

if TYPE_CHECKING:
from infrahub.core.branch import Branch
Expand All @@ -24,8 +27,33 @@ async def device_schema(db: InfrahubDatabase, default_branch: Branch, register_c
await load_schema(db=db, schema=DEVICE_SCHEMA, branch_name=default_branch.name)


@pytest.fixture
async def device_schema_with_tagged_interfaces(
db: InfrahubDatabase, default_branch: Branch, register_core_models_schema: None
) -> None:
"""The device schema, with an interface able to point at a tag: a peer that is not a subtemplate."""
schema = deepcopy(DEVICE_SCHEMA)
schema.nodes.append(deepcopy(TAG))
physical_interface = next(node for node in schema.nodes if node.kind == TestKind.PHYSICAL_INTERFACE)
physical_interface.relationships.append(
RelationshipSchema(
name="tag",
kind=RelationshipKind.ATTRIBUTE,
optional=True,
peer=TestKind.TAG,
cardinality=RelationshipCardinality.ONE,
)
)
await load_schema(db=db, schema=schema, branch_name=default_branch.name)


async def _build_template(
db: InfrahubDatabase, branch: Branch, name: str, nbr_interfaces: int, with_sfp: bool = False
db: InfrahubDatabase,
branch: Branch,
name: str,
nbr_interfaces: int,
with_sfp: bool = False,
tag_id: str | None = None,
) -> Node:
"""Build a device template, optionally giving each of its interfaces a subtemplate of its own."""
template = await Node.init(db=db, schema=f"Template{TestKind.DEVICE}", branch=branch)
Expand All @@ -34,13 +62,15 @@ async def _build_template(

for idx in range(nbr_interfaces):
interface = await Node.init(db=db, schema=f"Template{TestKind.PHYSICAL_INTERFACE}", branch=branch)
await interface.new(
db=db,
template_name=f"{name}-eth{idx}",
name=f"eth{idx}",
phys_type="SFP+ (10GE)",
device=template.id,
)
interface_data: dict[str, Any] = {
"template_name": f"{name}-eth{idx}",
"name": f"eth{idx}",
"phys_type": "SFP+ (10GE)",
"device": template.id,
}
if tag_id:
interface_data["tag"] = tag_id
await interface.new(db=db, **interface_data)
await interface.save(db=db)

if not with_sfp:
Expand Down Expand Up @@ -265,3 +295,52 @@ async def test_a_level_of_a_template_is_read_once_however_many_parents_it_hangs_
# read has no way to be told a node is already in hand. What matters here is that neither figure
# grows with the width of a level.
assert set(node_reads.values()) == {4}, f"reading the subtemplates grew with the width of a level: {node_reads}"


async def test_a_peer_a_component_carries_is_not_read_back_for_every_component(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a good test 👍 . I put it on the base commit and ran it against the unfixed code. It seems to prove what it tries to.

db: InfrahubDatabase, default_branch: Branch, device_schema_with_tagged_interfaces: None
) -> None:
"""A peer a subtemplate names is handed to the object created from it, not read again per object."""
tag = await Node.init(db=db, schema=TestKind.TAG, branch=default_branch)
await tag.new(db=db, name="uplink")
await tag.save(db=db)

device_schema_obj = registry.schema.get_node_schema(name=TestKind.DEVICE, branch=default_branch)
node_reads = {}
attribute_reads = {}
counts = {}

for nbr_interfaces in (1, 3, 5):
name = f"tagged-{nbr_interfaces}"
template = await _build_template(
db=db, branch=default_branch, name=name, nbr_interfaces=nbr_interfaces, tag_id=tag.id
)
counting_db = CountingInfrahubDatabase.from_db(db=db)

device = await create_node(
data={"name": f"{name}-device", "object_template": {"id": template.id}},
db=counting_db,
branch=default_branch,
schema=device_schema_obj,
)

reloaded = await NodeManager.get_one(db=db, id=device.id, branch=default_branch, raise_on_error=True)
interfaces = await reloaded.interfaces.get_peers(db=db)
assert len(interfaces) == nbr_interfaces
assert {await interface.tag.get_peer_id(db=db) for interface in interfaces.values()} == {tag.id}, (
"the peer the template names was not written on the objects created from it"
)

node_reads[nbr_interfaces] = counting_db.count_for(NodeListGetInfoQuery.name)
attribute_reads[nbr_interfaces] = counting_db.count_for(NodeListGetAttributeQuery.name)
counts[nbr_interfaces] = sum(counting_db.query_counts.values())

assert counting_db.count_for(RelationshipGetPeerQuery.name) == 0
# The template, the peers its relationships name, and the peers the subtemplate level names:
# three reads, whatever the number of components carrying the tag.
assert set(node_reads.values()) == {3}, f"the tag was read once per component: {node_reads}"
assert set(attribute_reads.values()) == {3}, f"the tag's attributes were read again: {attribute_reads}"
per_component = (counts[3] - counts[1]) / 2
assert per_component == (counts[5] - counts[3]) / 2 == 2, (
f"a component carrying a peer costs {per_component} queries rather than a check and a write: {counts}"
)
1 change: 1 addition & 0 deletions changelog/+template-component-peer-handover.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Creating objects from an object template is now faster when the objects it creates share a related object, such as the transceiver model used by every interface of a device template.
Loading