Skip to content

Commit

Permalink
Fix too many relationship error
Browse files Browse the repository at this point in the history
  • Loading branch information
LucasG0 committed Jan 21, 2025
1 parent 522d5ed commit eaa8f89
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 2 deletions.
10 changes: 8 additions & 2 deletions backend/infrahub/core/relationship/model.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import asyncio
import copy
import sys
from dataclasses import dataclass
Expand Down Expand Up @@ -719,6 +720,7 @@ def __init__(self, schema: RelationshipSchema, branch: Branch, at: Timestamp, no
)
self._relationship_id_details: Optional[RelationshipUpdateDetails] = None
self.has_fetched_relationships: bool = False
self.lock = asyncio.Lock()

@classmethod
async def init(
Expand Down Expand Up @@ -972,8 +974,12 @@ async def get(self, db: InfrahubDatabase) -> Relationship | list[Relationship] |
async def get_relationships(
self, db: InfrahubDatabase, branch_agnostic: bool = False, force_refresh: bool = False
) -> list[Relationship]:
if force_refresh or not self.has_fetched_relationships:
await self._fetch_relationships(db=db, branch_agnostic=branch_agnostic, force_refresh=force_refresh)
# Use lock to avoid concurrent mutations on this object. See https://github.com/opsmill/infrahub/issues/5474.
# It also prevents extra relationships fetching. Ideally, should we make sure all relationships are prefetched
# within DataLoader instead?
async with self.lock:
if force_refresh or not self.has_fetched_relationships:
await self._fetch_relationships(db=db, branch_agnostic=branch_agnostic, force_refresh=force_refresh)

return self._relationships.as_list()

Expand Down
21 changes: 21 additions & 0 deletions backend/tests/functional/ipam/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,24 @@ async def ipam_schema() -> SchemaRoot:
}

return SchemaRoot(**SCHEMA)


@pytest.fixture(scope="class")
async def prefix_with_rel_in_hfid_schema() -> SchemaRoot:
SCHEMA: dict[str, Any] = {
"nodes": [
{
"name": "Prefix",
"namespace": "Infra",
"description": "IPv4 or IPv6 network (with mask)",
"icon": "mdi:ip-network",
"include_in_menu": False,
"label": "Prefix",
"uniqueness_constraints": [["ip_namespace", "prefix__value"]],
"human_friendly_id": ["prefix__value", "ip_namespace__name__value"],
"inherit_from": ["BuiltinIPPrefix"],
}
]
}

return SchemaRoot(**SCHEMA)
17 changes: 17 additions & 0 deletions backend/tests/functional/ipam/test_load_concurrent_prefixes.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import ipaddress
from ipaddress import IPv4Network

from infrahub.database import InfrahubDatabase
from tests.functional.ipam.base import TestIpam
from tests.helpers.schema import load_schema


# See https://github.com/opsmill/infrahub/issues/4523
Expand Down Expand Up @@ -32,3 +34,18 @@ async def test_load_concurrent_prefixes(
if n.prefix.value != network_8:
# Without locking mechanism server side, parent might not be present
assert n.parent.peer.prefix.value == network_8

async def test_too_many_relationships(
self, db: InfrahubDatabase, default_branch, client, default_ipnamespace, prefix_with_rel_in_hfid_schema
):
await load_schema(db=db, schema=prefix_with_rel_in_hfid_schema)

prefixes = [IPv4Network("10.0.0.0/8"), IPv4Network("10.0.0.0/16"), IPv4Network("10.1.0.0/16")]

for prefix_val in prefixes:
prefix = await client.create("InfraPrefix", prefix=f"{prefix_val}")
await prefix.save()

results = await client.all("InfraPrefix")
prefixes_results = [prefix.prefix.value for prefix in results]
assert prefixes_results == prefixes

0 comments on commit eaa8f89

Please sign in to comment.