From cf262c2e2520e6cb5bfb415d63aa12bc8ae45bb4 Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 01:49:26 +0200 Subject: [PATCH 01/19] feat(db)!: key component relations by name; rewrite migration 017 By Digitl --- .../migrations/versions/017_relation_name.py | 75 +++++++++++++++++++ .../versions/017_upstream_relation.py | 46 ------------ .../src/interloper_db/models/components.py | 41 ++-------- .../src/interloper_db/store/components.py | 14 ++-- .../tests/models/test_components.py | 32 +++++++- 5 files changed, 119 insertions(+), 89 deletions(-) create mode 100644 packages/interloper-db/src/interloper_db/migrations/versions/017_relation_name.py delete mode 100644 packages/interloper-db/src/interloper_db/migrations/versions/017_upstream_relation.py diff --git a/packages/interloper-db/src/interloper_db/migrations/versions/017_relation_name.py b/packages/interloper-db/src/interloper_db/migrations/versions/017_relation_name.py new file mode 100644 index 00000000..7dcd8d39 --- /dev/null +++ b/packages/interloper-db/src/interloper_db/migrations/versions/017_relation_name.py @@ -0,0 +1,75 @@ +"""Key component relations by name. + +A relation row used to carry a ``type`` (the vocabulary entry) and a ``slot`` +(empty for list-shaped types). The framework now declares one named +``Relation`` per link, so a row is ``(src_id, name, dst_id)``: ``name`` is the +slot for slotted rows and the plural field name for the others. Whether a +relation is single-valued is a class rule the store enforces, so the partial +unique index on resource slots goes. + +The four types the old vocabulary ever wrote (``resource``, ``dependency``/ +``upstream``, ``target``, ``watch``) are backfilled into ``name`` below; the +final ``DELETE ... WHERE name IS NULL`` is defensive only; production has no +rows outside that vocabulary, so it is a no-op there. + +Revision ID: 017 +Revises: 016 +""" + +from __future__ import annotations + +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "017" +down_revision: str | None = "016" +branch_labels: str | None = None +depends_on: str | None = None + +_TABLE = "component_relations" +_PLURAL = {"destination": "destinations", "target": "targets", "watch": "watches"} + + +def upgrade() -> None: + op.add_column(_TABLE, sa.Column("name", sa.String(), nullable=True)) + op.execute("UPDATE component_relations SET name = slot WHERE type IN ('resource', 'dependency', 'upstream')") + for type_, name in _PLURAL.items(): + op.execute(f"UPDATE component_relations SET name = '{name}' WHERE type = '{type_}'") + op.execute("DELETE FROM component_relations WHERE name IS NULL OR name = ''") + op.alter_column(_TABLE, "name", nullable=False) + op.drop_index("uq_component_relations_slot", table_name=_TABLE) + op.drop_index("ix_component_relations_org_id_type", table_name=_TABLE) + op.drop_index("ix_component_relations_dst_id_type", table_name=_TABLE) + op.drop_constraint("component_relations_pkey", _TABLE, type_="primary") + op.drop_column(_TABLE, "type") + op.drop_column(_TABLE, "slot") + op.create_primary_key("component_relations_pkey", _TABLE, ["src_id", "name", "dst_id"]) + op.create_index("ix_component_relations_org_id_name", _TABLE, ["org_id", "name"]) + op.create_index("ix_component_relations_dst_id_name", _TABLE, ["dst_id", "name"]) + + +def downgrade() -> None: + op.add_column(_TABLE, sa.Column("type", sa.String(), nullable=True)) + op.add_column(_TABLE, sa.Column("slot", sa.String(), nullable=True, server_default="")) + for type_, name in _PLURAL.items(): + op.execute(f"UPDATE component_relations SET type = '{type_}' WHERE name = '{name}'") + op.execute( + "UPDATE component_relations SET type = 'upstream', slot = name WHERE type IS NULL AND dst_kind = 'asset'" + ) + op.execute("UPDATE component_relations SET type = 'resource', slot = name WHERE type IS NULL") + op.alter_column(_TABLE, "type", nullable=False) + op.drop_index("ix_component_relations_org_id_name", table_name=_TABLE) + op.drop_index("ix_component_relations_dst_id_name", table_name=_TABLE) + op.drop_constraint("component_relations_pkey", _TABLE, type_="primary") + op.drop_column(_TABLE, "name") + op.create_primary_key("component_relations_pkey", _TABLE, ["src_id", "type", "slot", "dst_id"]) + op.create_index( + "uq_component_relations_slot", + _TABLE, + ["src_id", "type", "slot"], + unique=True, + postgresql_where=sa.text("type = 'resource'"), + ) + op.create_index("ix_component_relations_org_id_type", _TABLE, ["org_id", "type"]) + op.create_index("ix_component_relations_dst_id_type", _TABLE, ["dst_id", "type"]) diff --git a/packages/interloper-db/src/interloper_db/migrations/versions/017_upstream_relation.py b/packages/interloper-db/src/interloper_db/migrations/versions/017_upstream_relation.py deleted file mode 100644 index fd5221d9..00000000 --- a/packages/interloper-db/src/interloper_db/migrations/versions/017_upstream_relation.py +++ /dev/null @@ -1,46 +0,0 @@ -"""Rename the asset-to-asset relation to ``upstream`` and let its slots hold several edges. - -The relation type ``dependency`` named the role too loosely (every relation -is a dependency of sorts); ``upstream`` says which way the edge points and -matches the vocabulary the app, the lineage tools and the executor already -use. Persisted rows follow the rename. - -A many-valued upstream slot binds every matching upstream, so the per-slot -uniqueness that made re-binding repoint an edge can no longer be a schema -rule for that type. Single-valued upstream slots keep repointing in -``RelationStore``, which knows the slot contract; resources stay unique by -schema. - -Revision ID: 017 -Revises: 016 -""" - -from __future__ import annotations - -import sqlalchemy as sa -from alembic import op - -# revision identifiers, used by Alembic. -revision: str = "017" -down_revision: str | None = "016" -branch_labels: str | None = None -depends_on: str | None = None - -_INDEX = "uq_component_relations_slot" -_TABLE = "component_relations" -_COLUMNS = ["src_id", "type", "slot"] - - -def upgrade() -> None: - op.execute("UPDATE component_relations SET type = 'upstream' WHERE type = 'dependency'") - op.drop_index(_INDEX, table_name=_TABLE) - op.create_index(_INDEX, _TABLE, _COLUMNS, unique=True, postgresql_where=sa.text("type = 'resource'")) - - -def downgrade() -> None: - # Fails if a slot holds several upstream edges; remove the extra legs first. - op.drop_index(_INDEX, table_name=_TABLE) - op.create_index( - _INDEX, _TABLE, _COLUMNS, unique=True, postgresql_where=sa.text("type IN ('resource', 'dependency')") - ) - op.execute("UPDATE component_relations SET type = 'dependency' WHERE type = 'upstream'") diff --git a/packages/interloper-db/src/interloper_db/models/components.py b/packages/interloper-db/src/interloper_db/models/components.py index f3a8a9ec..1a59059a 100644 --- a/packages/interloper-db/src/interloper_db/models/components.py +++ b/packages/interloper-db/src/interloper_db/models/components.py @@ -128,14 +128,12 @@ def stamp_state(self, **fields: Any) -> None: class ComponentRelation(SQLModel, table=True): - """A typed, directed relation between two components. + """A named, directed relation between two components. - ``type`` names the relation; ``slot`` disambiguates multiple relations of - the same type on one source component (a resource slot name, an upstream - parameter name, empty when the relation has no slot semantics). - - Checks constrain the types the schema knows and permit any type they - don't, so new relation types need no schema change. + ``name`` is the relation name as declared on the owning class: the field + a ``Relation`` is bound to. Whether a given name is single-valued or + many-valued is a class rule the store enforces, not a schema constraint, + so no uniqueness is declared here. """ __tablename__: ClassVar[str] = "component_relations" @@ -152,36 +150,13 @@ class ComponentRelation(SQLModel, table=True): ondelete="CASCADE", name="fk_component_relations_dst", ), - # Relation shapes (which types a kind may declare, which kinds they may - # point at, slotted or not) are enforced by the store from the class - # vocabulary — an open set, so it is deliberately not mirrored in CHECKs. - # Resource slots are single-valued by schema. Upstream slots are - # single-valued only when their class says so, which the store - # enforces from the slot contract (a many-valued slot fans in). - Index( - "uq_component_relations_slot", - "src_id", - "type", - "slot", - unique=True, - postgresql_where=text("type = 'resource'"), - sqlite_where=text("type = 'resource'"), - ), - Index("ix_component_relations_org_id_type", "org_id", "type"), - Index("ix_component_relations_dst_id_type", "dst_id", "type"), + Index("ix_component_relations_org_id_name", "org_id", "name"), + Index("ix_component_relations_dst_id_name", "dst_id", "name"), ) src_id: UUID = SQLField(primary_key=True) - type: str = SQLField(primary_key=True) - slot: str = SQLField(default="", primary_key=True) + name: str = SQLField(primary_key=True) dst_id: UUID = SQLField(primary_key=True) org_id: UUID src_kind: str dst_kind: str - - dst: Component = Relationship( - sa_relationship_kwargs={ - "primaryjoin": "foreign(ComponentRelation.dst_id) == Component.id", - "viewonly": True, - }, - ) diff --git a/packages/interloper-db/src/interloper_db/store/components.py b/packages/interloper-db/src/interloper_db/store/components.py index a50fbb2e..62bca5b9 100644 --- a/packages/interloper-db/src/interloper_db/store/components.py +++ b/packages/interloper-db/src/interloper_db/store/components.py @@ -47,17 +47,15 @@ from interloper_db.store.relations import Binding, RelationStore, _add_relation from interloper_db.store.status import ComponentStatus, asset_status, source_status -# Eager-load set for rows returned to API consumers: the parent, children -# with their relations, and two hops (component → destination → resources). +# Eager-load set for rows returned to API consumers: the parent and children +# with their relations. The two-hop (component -> destination -> resources) +# legs are dropped for now: they walked ComponentRelation.dst, a relationship +# the relation-by-name rewrite removed. The rewritten store restores +# whatever eager loading it needs. COMPONENT_LOAD_OPTIONS = [ selectinload(Component.parent), # ty: ignore[invalid-argument-type] selectinload(Component.children) # ty: ignore[invalid-argument-type] - .selectinload(Component.out_relations) # ty: ignore[invalid-argument-type] - .selectinload(ComponentRelation.dst), # ty: ignore[invalid-argument-type] - selectinload(Component.out_relations) # ty: ignore[invalid-argument-type] - .selectinload(ComponentRelation.dst) # ty: ignore[invalid-argument-type] - .selectinload(Component.out_relations) # ty: ignore[invalid-argument-type] - .selectinload(ComponentRelation.dst), # ty: ignore[invalid-argument-type] + .selectinload(Component.out_relations), # ty: ignore[invalid-argument-type] ] diff --git a/packages/interloper-db/tests/models/test_components.py b/packages/interloper-db/tests/models/test_components.py index 73490ef2..ec336984 100644 --- a/packages/interloper-db/tests/models/test_components.py +++ b/packages/interloper-db/tests/models/test_components.py @@ -13,9 +13,9 @@ import pydantic import pytest from sqlalchemy import Engine -from sqlmodel import Session +from sqlmodel import Session, select -from interloper_db.models import Component +from interloper_db.models import Component, ComponentRelation _ORG = uuid4() @@ -97,3 +97,31 @@ def test_a_shape_the_kind_does_not_declare_is_rejected(self): with pytest.raises(pydantic.ValidationError): row.stamp_state(next_run_at=object()) + + +class TestComponentRelation: + """A relation row is keyed by ``(src_id, name, dst_id)``, no ``type``/``slot``.""" + + def test_relation_row_is_keyed_by_name(self, component_db: Engine) -> None: + with Session(component_db) as session: + src = Component(org_id=_ORG, kind="source", key="s") + dst = Component(org_id=_ORG, kind="destination", key="d") + session.add_all([src, dst]) + session.flush() + session.add( + ComponentRelation( + src_id=src.id, + name="destinations", + dst_id=dst.id, + org_id=_ORG, + src_kind="source", + dst_kind="destination", + ) + ) + session.commit() + + row = session.exec(select(ComponentRelation)).one() + + assert row.name == "destinations" + assert not hasattr(row, "slot") + assert not hasattr(row, "type") From 8366ee4c09363864f3794213bc409cd52455c728 Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 02:04:03 +0200 Subject: [PATCH 02/19] feat(db)!: RelationStore binds by name through Relation.accepts By Digitl --- .../src/interloper_db/store/components.py | 30 +- .../src/interloper_db/store/relations.py | 545 +++++++----------- .../tests/store/test_relations.py | 523 +++++++++-------- 3 files changed, 527 insertions(+), 571 deletions(-) diff --git a/packages/interloper-db/src/interloper_db/store/components.py b/packages/interloper-db/src/interloper_db/store/components.py index 62bca5b9..1814b2dc 100644 --- a/packages/interloper-db/src/interloper_db/store/components.py +++ b/packages/interloper-db/src/interloper_db/store/components.py @@ -44,7 +44,7 @@ from interloper_db.session import commit, session_scope from interloper_db.store.hydration import Hydrator from interloper_db.store.quotas import QUOTA_MAX_ASSETS_PER_SOURCE, QUOTA_MAX_SOURCES, QuotaStore -from interloper_db.store.relations import Binding, RelationStore, _add_relation +from interloper_db.store.relations import Binding, RelationStore from interloper_db.store.status import ComponentStatus, asset_status, source_status # Eager-load set for rows returned to API consumers: the parent and children @@ -115,7 +115,7 @@ def create( ``False`` opts into plaintext storage. children: Source kinds only — which child asset keys to enable (``None`` enables all the catalog class declares). - relations: ``{type: [(dst_id, slot), …]}`` — synced per type. + relations: ``{name: [dst_id, …]}`` — replaced per name. Returns: The created component row, eager-loaded. @@ -206,7 +206,7 @@ def update( ``False`` opts into plaintext storage. children: Source kinds only — the exact set of child asset keys to keep enabled. - relations: ``{type: [(dst_id, slot), …]}`` — synced per type. + relations: ``{name: [dst_id, …]}`` — replaced per name. Returns: The updated component row, eager-loaded. @@ -793,37 +793,37 @@ def _wire_intra_upstreams( source_cls: type[il.Source], children_by_key: dict[str, Component], ) -> None: - """Top up missing intra-source upstream relations from class metadata. + """Top up missing intra-source asset relations from class metadata. Idempotent over the full child set — assets enabled after their - siblings still get the edges *into* them wired. Slots that already + siblings still get the edges *into* them wired. Names that already hold an edge are never touched, so manual bindings survive. Args: session: Open session the relations are added to. - source_cls: The catalog class declaring the upstreams. + source_cls: The catalog class declaring the relations. children_by_key: The source's enabled child rows, keyed by asset key. """ - source_key = source_cls.key child_ids = [child.id for child in children_by_key.values()] bound = { (row[0], row[1]) for row in session.exec( - select(ComponentRelation.src_id, ComponentRelation.slot).where( - col(ComponentRelation.src_id).in_(child_ids), - ComponentRelation.type == "upstream", + select(ComponentRelation.src_id, ComponentRelation.name).where( + col(ComponentRelation.src_id).in_(child_ids) ) ).all() } for asset_type in source_cls.asset_types: - asset_key = asset_type.key - child = children_by_key.get(asset_key) + child = children_by_key.get(asset_type.key) if child is None: continue - for param_name, sibling_key in asset_type.sibling_upstreams(source_key, children_by_key).items(): - if (child.id, param_name) in bound: + for name, relation in asset_type.relations.items(): + if (child.id, name) in bound or "asset" not in relation.kinds() or not relation.source_local: continue - _add_relation(session, child, children_by_key[sibling_key], "upstream", param_name) + declared_keys = relation.keys() + for sibling_key in declared_keys: + if (sibling := children_by_key.get(sibling_key)) is not None: + RelationStore._insert(session, child, sibling, name) def job_partition_granularity(self, session: Session, job_id: UUID) -> TimeGranularity | None: """Resolve the granularity a job's partitioned targets share. diff --git a/packages/interloper-db/src/interloper_db/store/relations.py b/packages/interloper-db/src/interloper_db/store/relations.py index 0ceef34c..5e7625e5 100644 --- a/packages/interloper-db/src/interloper_db/store/relations.py +++ b/packages/interloper-db/src/interloper_db/store/relations.py @@ -1,36 +1,34 @@ -"""Relation policy: validated reads and writes for the edge table. - -The class vocabulary is the contract. It resolves parent-aware, a -source-owned asset's definition (upstream slots, ``optional`` flags) -lives on the parent source's definition, with the kind's anchor as the -drift fallback. Writes enforce the declared shape: relation type, dst -kind, slot names, and each slot's expected destination identity (resolved -through :meth:`~interloper.component.relation.ComponentIdentity.resolve` for upstream -slots). Unbinding follows the vocabulary's ``on_unbind`` semantics: bound -required slots of a blocking type refuse it. Rows are stamped with the -denormalized ``org_id``/``src_kind``/``dst_kind`` triple the composite -foreign keys verify. +"""Relation policy: validated reads and writes for the named edge table. + +An edge is a name, and the class declaring that name is the contract. The +vocabulary resolves parent-aware, since a source-owned asset's declaration +lives on the source that owns it, with the kind's anchor as the drift +fallback. Every write asks the declared +:class:`~interloper.component.relation.Relation` whether it accepts the +candidate, so the database enforces exactly what the framework enforces in +memory: the declared kinds, and the identity the declared keys expect +(bare, qualified or wildcard). Single-valued names hold one edge and +repoint on rewrite; ``many`` names accumulate. A non-optional name cannot +be emptied, only repointed. Rows are stamped with the denormalized +``org_id``/``src_kind``/``dst_kind`` triple the composite foreign keys +verify. """ from __future__ import annotations -from collections.abc import Iterable from uuid import UUID import interloper as il from interloper.catalog.base import Catalog -from interloper.component.relation import ComponentIdentity from interloper.errors import ConfigError, NotFoundError from sqlalchemy import Engine -from sqlalchemy.exc import IntegrityError from sqlmodel import Session, select from interloper_db.models import Component, ComponentRelation from interloper_db.session import commit, session_scope -# One relation binding: (destination component id, slot). Slot is "" for -# slotless relation types. -Binding = tuple[UUID, str] +# One relation binding: the destination component id. +Binding = UUID class RelationStore: @@ -48,405 +46,302 @@ def __init__(self, engine: Engine, catalog: Catalog) -> None: # -- Public API ------------------------------------------------------------ - def list_all(self, org_id: UUID, *, type: str | None = None) -> list[ComponentRelation]: - """List an organisation's component relations, optionally by type. + def list_all( + self, + org_id: UUID, + *, + name: str | None = None, + src_kind: str | None = None, + dst_kind: str | None = None, + ) -> list[ComponentRelation]: + """List an organisation's component relations, optionally filtered. Args: org_id: Organisation whose relations are listed. - type: Relation type to restrict the listing to. None (the default) - lists every type. + name: Relation name to restrict the listing to. None (the default) + lists every name. + src_kind: Kind the source component must have. None (the default) + accepts any kind. + dst_kind: Kind the destination component must have. None (the + default) accepts any kind. Returns: - The organisation's relation rows, in no guaranteed order. + The organisation's matching relation rows, in no guaranteed order. """ with session_scope(self._engine) as session: statement = select(ComponentRelation).where(ComponentRelation.org_id == org_id) - if type: - statement = statement.where(ComponentRelation.type == type) + if name: + statement = statement.where(ComponentRelation.name == name) + if src_kind: + statement = statement.where(ComponentRelation.src_kind == src_kind) + if dst_kind: + statement = statement.where(ComponentRelation.dst_kind == dst_kind) return list(session.exec(statement).all()) - def add(self, component_id: UUID, *, type: str, dst_id: UUID, slot: str = "") -> ComponentRelation: - """Add one relation from a component. + def add(self, component_id: UUID, *, name: str, dst_id: UUID) -> ComponentRelation: + """Bind one component to another under a declared relation name. - Slotted types upsert per slot: re-binding an already-bound slot - repoints it to the new destination, and re-adding an identical - relation is a no-op returning the existing row. + A ``many`` name accumulates; a single-valued one repoints, so + rebinding it needs no :meth:`remove` first. Re-adding an edge that is + already there returns it untouched. An undeclared name, a missing or + cross-org endpoint and a destination the declared relation refuses all + propagate from the checks this delegates to (:meth:`_lock`, + :meth:`_relation`, :meth:`_resolve`). Args: component_id: Source component the relation originates from. - type: Relation type, which the source kind's vocabulary must - declare. + name: Relation name, which the source's class must declare. dst_id: Destination component the relation points at. Must belong to the same organisation as the source. - slot: Slot the relation fills, for slotted types. Empty (the - default) for slotless ones. Returns: - The relation (created, repointed, or already present). - - Raises: - NotFoundError: If either endpoint is missing or cross-org. - ConfigError: If the kind's vocabulary doesn't declare the type or - the slot, or the destination doesn't match the slot's shape. + The relation row (created, repointed, or already present). """ with session_scope(self._engine) as session: - src = session.get(Component, component_id) - if not src: - raise NotFoundError(f"Component {component_id} not found") - definition = self._check_vocabulary(session, src, type) - dst = self._resolve_dst(session, src, definition, type, slot, dst_id) - relation = self._upsert_relation(session, src, dst, type, slot, per_slot=definition.slotted) - try: - commit(session) - except IntegrityError: - raise ConfigError( - f"Relation '{type}'{f' slot {slot!r}' if slot else ''} on '{src.key}' " - f"was modified concurrently; retry" - ) - return relation + src = self._lock(session, component_id) + relation = self._relation(session, src, name) + dst = self._resolve(session, src, relation, name, dst_id) + existing = self._rows(session, src.id, name) + if match := next((row for row in existing if row.dst_id == dst.id), None): + return match + if not relation.many: + for row in existing: + session.delete(row) + session.flush() + row = self._insert(session, src, dst, name) + commit(session) + return row - def remove(self, component_id: UUID, *, type: str, dst_id: UUID) -> None: - """Remove a component's relations of one type toward one destination. + def remove(self, component_id: UUID, *, name: str, dst_id: UUID) -> None: + """Detach one component from another under a declared relation name. Args: - component_id: Source component the relations originate from. - type: Relation type to remove. - dst_id: Destination the removed relations point at. Every slot - bound to it under *type* is removed. + component_id: Source component the relation originates from. + name: Relation name the edge is filed under. + dst_id: Destination the removed edge points at. An edge that isn't + there is a no-op. Raises: - ConfigError: If a matching edge fills a slot the vocabulary - refuses to unbind (a bound required dependency) — repoint - the slot or remove the dependent asset instead. + ConfigError: If the edge is the last one of a non-optional + relation, which may be repointed but not emptied. """ with session_scope(self._engine) as session: - statement = select(ComponentRelation).where( - ComponentRelation.src_id == component_id, - ComponentRelation.type == type, - ComponentRelation.dst_id == dst_id, - ) - relations = session.exec(statement).all() - src = session.get(Component, component_id) if relations else None - if src is not None: - definition = self._relation_vocabulary(session, src).get(type) - if blocked := self._blocked_unbinds(definition, (relation.slot for relation in relations)): - raise ConfigError( - f"Required '{type}' slot(s) {blocked} of '{src.key}' cannot be unbound; " - f"repoint them or remove the dependent asset instead" - ) - for relation in relations: - session.delete(relation) + rows = self._rows(session, component_id, name) + row = next((candidate for candidate in rows if candidate.dst_id == dst_id), None) + if row is None: + return + src = session.get(Component, component_id) + relation = self._vocabulary(session, src).get(name) if src is not None else None + if len(rows) == 1 and relation is not None and not relation.optional: + raise ConfigError( + f"'{src.key if src else component_id}'.{name} is non-optional and cannot be emptied; " + f"repoint it or remove the dependent component instead" + ) + session.delete(row) commit(session) # -- Internals ------------------------------------------------------------- - def _sync_relations( - self, - session: Session, - src: Component, - relations: dict[str, list[Binding]] | None, - ) -> None: - """Replace the relation types present in *relations* (empty list clears). - - Bound slots the vocabulary refuses to unbind (required dependencies) - must stay bound — repointing (same slot, different destination) is - allowed. + def _sync_relations(self, session: Session, src: Component, bindings: dict[str, list[Binding]] | None) -> None: + """Replace the relation names present in *bindings* (empty list clears). Args: session: Open session the replacement is written through; deletes and inserts are flushed, never committed here. src: Source component whose relations are replaced. - relations: Bindings to install, keyed by relation type. Only the - types present are touched; an empty binding list clears that - type. None leaves every relation untouched. + bindings: Destination ids to install, keyed by relation name. Only + the names present are touched, each replaced wholesale. + None leaves every relation untouched. Raises: - ConfigError: If dropping a binding would unbind a required slot - the vocabulary refuses to unbind. + ConfigError: If a name is not declared by the source's class, if a + single-valued name is given several destinations, if the + relation does not accept one of them, or if the replacement + would empty a non-optional relation. """ - for relation_type, bindings in (relations or {}).items(): - definition = self._check_vocabulary(session, src, relation_type) - existing = session.exec( - select(ComponentRelation).where( - ComponentRelation.src_id == src.id, ComponentRelation.type == relation_type - ) - ).all() - kept = {slot for _, slot in bindings} - dropped = (relation.slot for relation in existing if relation.slot not in kept) - if blocked := self._blocked_unbinds(definition, dropped): + for name, dst_ids in (bindings or {}).items(): + relation = self._relation(session, src, name) + if not relation.many and len(dst_ids) > 1: + raise ConfigError(f"'{src.key}'.{name} is single-valued and takes one target at a time") + existing = self._rows(session, src.id, name) + if existing and not dst_ids and not relation.optional: raise ConfigError( - f"Required '{relation_type}' slot(s) {blocked} of '{src.key}' cannot be unbound; " - f"repoint them or remove the dependent asset instead" + f"'{src.key}'.{name} is non-optional and cannot be emptied; " + f"repoint it or remove the dependent component instead" ) - for relation in existing: - session.delete(relation) + kept = set(dst_ids) + for row in existing: + if row.dst_id not in kept: + session.delete(row) session.flush() - for dst_id, slot in bindings: - dst = self._resolve_dst(session, src, definition, relation_type, slot, dst_id) - _add_relation(session, src, dst, relation_type, slot) + held = {row.dst_id for row in existing if row.dst_id in kept} + for dst_id in dst_ids: + if dst_id in held: + continue + held.add(dst_id) + self._insert(session, src, self._resolve(session, src, relation, name, dst_id), name) + + def _relation_detaches(self, session: Session, referrer: Component, relation_row: ComponentRelation) -> bool: + """Whether an edge detaches (rather than blocks) when its destination is deleted. + + Consults the referrer's own vocabulary: a name declared + ``on_delete="detach"`` detaches, as does an optional one, since the + referrer keeps working with nothing bound there. Anything + unresolvable (an undeclared name, a drifted key) blocks, keeping the + guard fail-closed. - def _check_vocabulary(self, session: Session, src: Component, relation_type: str) -> il.RelationDefinition: - """Reject relation types the row's class vocabulary doesn't declare. + Args: + session: Open session the referrer's vocabulary is resolved + through. + referrer: Referrer row holding the edge. + relation_row: Edge whose destination is about to be deleted. + + Returns: + True if the edge may be detached, False if it blocks the deletion. + """ + relation = self._vocabulary(session, referrer).get(relation_row.name) + if relation is None: + return False + return relation.on_delete == "detach" or relation.optional + + def _vocabulary(self, session: Session, row: Component) -> dict[str, il.Relation]: + """The relation vocabulary governing a component row. Args: - session: Open session used to resolve the row's vocabulary. - src: Source component whose vocabulary is consulted. - relation_type: Relation type to check against that vocabulary. + session: Open session the parent row is read through. + row: Component row whose vocabulary is resolved. Returns: - The type's relation definition (slots included for owned assets). + The declared relations keyed by name, read from the owning + source's declaration for a source-owned asset. Empty when nothing + resolves. + """ + return self._catalog.vocabulary(row.kind, row.key, parent_key=row.parent_key(session)) + + def _relation(self, session: Session, src: Component, name: str) -> il.Relation: + """The relation a row's class declares under *name*. + + Args: + session: Open session the vocabulary is resolved through. + src: Component row whose class declares the relation. + name: Relation name to look up. + + Returns: + The declared relation. Raises: - ConfigError: If the source kind's vocabulary declares no relation - of that type. + ConfigError: If the row's class declares no relation of that name. """ - vocabulary = self._relation_vocabulary(session, src) - if relation_type not in vocabulary: + vocabulary = self._vocabulary(session, src) + if name not in vocabulary: raise ConfigError( - f"Components of kind '{src.kind}' ('{src.key}') declare no '{relation_type}' relations " - f"(allowed: {sorted(vocabulary) or 'none'})" + f"'{src.key}' ({src.kind}) declares no relation '{name}' (declared: {sorted(vocabulary)})" ) - return vocabulary[relation_type] + return vocabulary[name] - def _resolve_dst( - self, - session: Session, - src: Component, - definition: il.RelationDefinition, - relation_type: str, - slot: str, - dst_id: UUID, - ) -> Component: - """Resolve a relation destination, enforcing existence, same-org, and the vocabulary's shape. + def _identity(self, session: Session, row: Component) -> il.ComponentIdentity: + """What a component row is, for relation matching. + + Args: + session: Open session the parent row is read through. + row: Component row to identify. + + Returns: + The row's identity: its owning source's key (None when it has no + parent) and its own key. + """ + return il.ComponentIdentity(row.parent_key(session), row.key) + + def _resolve(self, session: Session, src: Component, relation: il.Relation, name: str, dst_id: UUID) -> Component: + """Load a relation destination and check the declared relation accepts it. Args: session: Open session the destination row is loaded through. src: Source component the relation originates from. - definition: Relation definition the destination must satisfy - (allowed kinds, slotted-ness, declared slots). - relation_type: Relation type, used for error messages. - slot: Slot the relation fills. Empty for slotless types. + relation: Declared relation the destination must satisfy. + name: Relation name, used for error messages. dst_id: Destination component to resolve. Returns: - The destination row, validated against the definition. + The destination row, accepted by the relation. Raises: NotFoundError: If the destination is missing or belongs to another organisation. - ConfigError: If the destination is the source itself, its kind is - not allowed, a slot is given for a slotless type, or the slot - is not declared by the definition. + ConfigError: If the relation does not accept the destination's kind + or identity. """ dst = session.get(Component, dst_id) if dst is None or dst.org_id != src.org_id: - raise NotFoundError( - f"Component {dst_id} not found (relation '{relation_type}'{f'/{slot}' if slot else ''})" - ) - if dst.id == src.id: - raise ConfigError(f"Relation '{relation_type}' on '{src.key}' may not point at the component itself") - if dst.kind not in definition.kinds: + raise NotFoundError(f"Component {dst_id} not found (relation '{name}')") + if not relation.accepts(dst.kind, self._identity(session, dst), owner=self._identity(session, src)): raise ConfigError( - f"Relation '{relation_type}' on kind '{src.kind}' may not point at a '{dst.kind}' " - f"component (allowed: {definition.kinds})" + f"'{src.key}'.{name} does not accept {dst.kind} '{dst.key}' " + f"(declared: kind {relation.kinds()}, key {relation.keys() or 'any'})" ) - if not definition.slotted and slot: - raise ConfigError(f"Relation '{relation_type}' on kind '{src.kind}' is not slotted (got slot '{slot}')") - # A definition with no declared slots (a kind anchor reached through - # the drift fallback) can't vet slot names or targets — skip, fail-open. - if definition.slotted and definition.slots: - slot_def = definition.slots.get(slot) - if slot_def is None: - raise ConfigError( - f"Relation '{relation_type}' on '{src.key}' declares no slot '{slot}' " - f"(declared: {sorted(definition.slots)})" - ) - self._check_slot_target(session, src, relation_type, slot, slot_def, dst) return dst - def _check_slot_target( - self, session: Session, src: Component, relation_type: str, slot: str, slot_def: il.Dependency, dst: Component - ) -> None: - """Enforce a slot's declared destination identity, when it declares one. - - Upstream slot keys resolve through ``ComponentIdentity.resolve``: an - intra-source dep (the declarer's own source) must bind a sibling of - the same source instance; a cross-source one accepts the named - source's asset from any instance. Other slotted types (resources) - declare a plain component key. - - Args: - session: Open session the parent rows are loaded through. - src: Source component declaring the slot. - relation_type: Relation type the slot belongs to. ``"upstream"`` selects - the asset-identity resolution described above. - slot: Slot name, used for error messages. - slot_def: Slot definition. An empty ``key`` declares no expected - identity and the check is skipped. - dst: Destination component to validate against the slot. - - Raises: - ConfigError: If the destination's key, or the source instance it - belongs to, doesn't match the slot's declared identity. - """ - if not slot_def.key: - return - if relation_type != "upstream": - if dst.key != slot_def.key: - raise ConfigError( - f"Relation '{relation_type}' slot '{slot}' of '{src.key}' expects a '{slot_def.key}' " - f"component, got '{dst.key}'" - ) - return - src_parent = session.get(Component, src.parent_id) if src.parent_id else None - own_source_key = src_parent.key if src_parent else None - expected = ComponentIdentity.resolve(slot_def.key, own_source_key=own_source_key) - if dst.key != expected.key: - raise ConfigError( - f"Upstream slot '{slot}' of '{src.key}' expects asset '{slot_def.key}', got '{dst.key}'" - ) - if expected.source_key == own_source_key: - if src.parent_id is not None and dst.parent_id != src.parent_id: - raise ConfigError( - f"Upstream slot '{slot}' of '{src.key}' must bind a sibling asset " - f"of the same source instance" - ) - else: - dst_parent = session.get(Component, dst.parent_id) if dst.parent_id else None - if dst_parent is None or dst_parent.key != expected.source_key: - raise ConfigError( - f"Upstream slot '{slot}' of '{src.key}' expects an asset of source " - f"'{expected.source_key}', got one of '{dst_parent.key if dst_parent else 'none'}'" - ) + def _lock(self, session: Session, component_id: UUID) -> Component: + """Load the source row of a write, holding it for the transaction. - def _relation_vocabulary(self, session: Session, db_source: Component) -> dict[str, il.RelationDefinition]: - """A referrer row's relation vocabulary, slots included. + The row lock serializes concurrent writes to the same source, so two + rebinds of one single-valued name cannot both read an empty edge set + and both insert. SQLite has no row locks and needs none: its writes + are serialized database-wide. Args: - session: Open session the parent row is loaded through. - db_source: Referrer row whose vocabulary is resolved. + session: Open session the row is loaded through. + component_id: Source component the write originates from. Returns: - The declared relation definitions keyed by relation type, from the - owning source's declaration for a source-owned asset. Empty when - nothing resolves. - """ - return self._catalog.vocabulary(db_source.kind, db_source.key, parent_key=db_source.parent_key(session)) - - def _relation_detaches(self, session: Session, db_source: Component, relation: ComponentRelation) -> bool: - """Whether a relation detaches (rather than blocks) when its destination is deleted. + The source row, locked on backends that support it. - Consults the referrer's own vocabulary: a type declared - ``on_delete="detach"`` detaches, as does a slot the referrer declares - optional (``Dependency.optional=True``), such as an optional upstream - dependency. Anything unresolvable (unknown type, drifted key, - undeclared slot) blocks, keeping the guard fail-closed. - - Args: - session: Open session the referrer's vocabulary is resolved - through. - db_source: Referrer row holding the relation. - relation: Relation whose destination is about to be deleted. - - Returns: - True if the relation may be detached, False if it blocks the - deletion. + Raises: + NotFoundError: If the component does not exist. """ - definition = self._relation_vocabulary(session, db_source).get(relation.type) - if definition is None: - return False - if definition.on_delete == "detach": - return True - slot = definition.slots.get(relation.slot) - return slot is not None and slot.optional + statement = select(Component).where(Component.id == component_id) + if session.bind is not None and session.bind.dialect.name == "postgresql": + statement = statement.with_for_update() + src = session.exec(statement).first() + if src is None: + raise NotFoundError(f"Component {component_id} not found") + return src @staticmethod - def _blocked_unbinds(definition: il.RelationDefinition | None, slots: Iterable[str]) -> list[str]: - """Bound slots whose explicit unbinding the vocabulary refuses. - - A slot blocks when its type declares ``on_unbind="block"`` and the slot - is not optional. Unknown definitions or slots (drift) don't block. + def _rows(session: Session, src_id: UUID, name: str) -> list[ComponentRelation]: + """The edges a component currently holds under one relation name. Args: - definition: Relation definition the slots belong to. None (an - unresolvable type) blocks nothing. - slots: Slot names about to be unbound. Duplicates are collapsed. + session: Open session the rows are read through. + src_id: Source component the edges originate from. + name: Relation name the edges are filed under. Returns: - The blocking slot names, sorted. Empty when the unbind is allowed. + The matching edge rows, in no guaranteed order. """ - if definition is None or definition.on_unbind != "block": - return [] - return sorted( - {slot for slot in slots if (slot_def := definition.slots.get(slot)) is not None and not slot_def.optional} - ) + statement = select(ComponentRelation).where(ComponentRelation.src_id == src_id, ComponentRelation.name == name) + return list(session.exec(statement).all()) @staticmethod - def _upsert_relation( - session: Session, src: Component, dst: Component, relation_type: str, slot: str, *, per_slot: bool - ) -> ComponentRelation: - """Idempotent relation write: an identical edge is returned as-is. - - With *per_slot* (slotted types), a slot bound to a different destination - is repointed — the old edge is replaced by the new one. + def _insert(session: Session, src: Component, dst: Component, name: str) -> ComponentRelation: + """Add one edge, stamping the denormalized org/kind triple from the rows. Args: - session: Open session the write goes through; flushed, never - committed here. + session: Open session the row is added to; not flushed or committed. src: Source component the relation originates from. dst: Destination component the relation points at. - relation_type: Relation type to write. - slot: Slot the relation fills. Empty for slotless types. - per_slot: Whether the type is slotted, making the slot alone the - identity of the edge (so re-binding repoints it). When False, - the destination is part of that identity. + name: Relation name to file the edge under. Returns: - The matching relation: the untouched existing row, or the newly - added one (pending, not flushed). + The pending edge (added to the session, not flushed). """ - statement = select(ComponentRelation).where( - ComponentRelation.src_id == src.id, - ComponentRelation.type == relation_type, - ComponentRelation.slot == slot, + row = ComponentRelation( + src_id=src.id, + name=name, + dst_id=dst.id, + org_id=src.org_id, + src_kind=src.kind, + dst_kind=dst.kind, ) - if not per_slot: - statement = statement.where(ComponentRelation.dst_id == dst.id) - existing = session.exec(statement).all() - if match := next((relation for relation in existing if relation.dst_id == dst.id), None): - return match - for relation in existing: - session.delete(relation) - if existing: - session.flush() - return _add_relation(session, src, dst, relation_type, slot) - - -def _add_relation( - session: Session, src: Component, dst: Component, relation_type: str, slot: str = "" -) -> ComponentRelation: - """Add one relation, stamping the denormalized org/kind triple from the rows. - - Args: - session: Open session the row is added to; not flushed or committed. - src: Source component the relation originates from. - dst: Destination component the relation points at. - relation_type: Relation type to write. - slot: Slot the relation fills. Empty (the default) for slotless types. - - Returns: - The pending relation (added to the session, not flushed). - """ - relation = ComponentRelation( - src_id=src.id, - type=relation_type, - slot=slot, - dst_id=dst.id, - org_id=src.org_id, - src_kind=src.kind, - dst_kind=dst.kind, - ) - session.add(relation) - return relation - - + session.add(row) + return row diff --git a/packages/interloper-db/tests/store/test_relations.py b/packages/interloper-db/tests/store/test_relations.py index 601ec0d1..49856d97 100644 --- a/packages/interloper-db/tests/store/test_relations.py +++ b/packages/interloper-db/tests/store/test_relations.py @@ -2,14 +2,11 @@ from __future__ import annotations -from typing import Any, ClassVar -from uuid import uuid4 +from uuid import UUID, uuid4 import interloper as il import pytest from interloper.errors import ConfigError, NotFoundError -from interloper_assets.demo.source import DemoSource -from interloper_assets.facebook_ads.source import FacebookAds from sqlalchemy import Engine from sqlmodel import Session, select @@ -19,20 +16,10 @@ _ORG = uuid4() -@pytest.fixture -def store(component_db: Engine) -> Store: - """A store over the in-memory database (no catalog needed for these). - - Returns: - A store with an empty catalog, reading and writing the fixture database. - """ - return Store(catalog=il.Catalog(components={})) - - -def _relations(session: Session, src_id, type: str | None = None) -> list[ComponentRelation]: +def _relations(session: Session, src_id: UUID, name: str | None = None) -> list[ComponentRelation]: statement = select(ComponentRelation).where(ComponentRelation.src_id == src_id) - if type: - statement = statement.where(ComponentRelation.type == type) + if name: + statement = statement.where(ComponentRelation.name == name) return list(session.exec(statement).all()) @@ -40,292 +27,366 @@ def _child(source: Component, key: str) -> Component: return next(child for child in source.children if child.key == key) +class WireConnection(il.Connection): + """Connection the wire sources bind.""" + + class GuardUpstream(il.Asset): - """Upstream asset for the unbind-guard dependency tests.""" + """Upstream asset for the unbind-guard tests.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class GuardOther(il.Asset): + """Asset no guard relation declares a key for.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] class GuardRequired(il.Asset): - """Asset with a required dependency on ``guard_upstream``.""" + """Asset with a required upstream on ``guard_upstream``.""" - depends_on: ClassVar[dict[str, Any]] = {"up": "guard_upstream"} + up = il.Relation("asset", "guard_upstream") + + def data(self, context: il.ExecutionContext, up: il.Upstream) -> list[dict]: + return [] class GuardOptional(il.Asset): - """Asset with an optional dependency on ``guard_upstream``.""" + """Asset with an optional upstream on ``guard_upstream``.""" + + up = il.Relation("asset", "guard_upstream", optional=True) + + def data(self, context: il.ExecutionContext, up: il.Upstream | None) -> list[dict]: + return [] + + +class Matcher(il.Asset): + """Asset fanning in the ``campaigns`` asset of every source.""" + + campaigns: list[il.Asset] = il.Relation("asset", "*.campaigns", many=True) - depends_on: ClassVar[dict[str, Any]] = {"up": il.Dependency(key="guard_upstream", optional=True)} + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] class WireUpSource(il.Source): - """Upstream source for the cross-source dependency tests.""" + """Upstream source whose ``totals`` reads its sibling ``rows``.""" + + class Rows(il.Asset): + """Root asset of the source.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + class Totals(il.Asset): + """Asset reading the source's own ``rows``.""" + + rows = il.Relation("asset", "rows") + + def data(self, context: il.ExecutionContext, rows: il.Upstream) -> list[dict]: + return [] + + +class WireOtherSource(il.Source): + """Another source declaring a ``rows`` asset, of a different source key.""" class Rows(il.Asset): - """Upstream asset (key ``rows``).""" + """Homonym of ``WireUpSource.Rows``, owned by another source.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] class WireDownSource(il.Source): - """Downstream source whose asset requires ``wire_up_source.rows``.""" + """Downstream source: a bound connection and a cross-source upstream.""" + + connection: WireConnection class Consumer(il.Asset): - """Asset with a required cross-source dependency.""" + """Asset reading ``wire_up_source.rows``.""" - depends_on: ClassVar[dict[str, Any]] = {"rows": "wire_up_source.rows"} + rows = il.Relation("asset", "wire_up_source.rows") + def data(self, context: il.ExecutionContext, rows: il.Upstream) -> list[dict]: + return [] -class TestRelations: - """Replace semantics, vocabulary validation, denormalized stamping.""" - def test_sync_stamps_org_and_kinds(self, store: Store, component_db: Engine): - dest = store.components.create(_ORG, kind="destination", key="dest") - asset = store.components.create(_ORG, kind="asset", key="a", relations={"destination": [(dest.id, "")]}) +class FirstCampaignSource(il.Source): + """Source declaring a ``campaigns`` asset.""" - with Session(component_db) as session: - (relation,) = _relations(session, asset.id) - assert (relation.type, relation.slot, relation.dst_id) == ("destination", "", dest.id) - assert (relation.org_id, relation.src_kind, relation.dst_kind) == (_ORG, "asset", "destination") - - def test_update_replaces_only_the_given_type(self, store: Store, component_db: Engine): - dest = store.components.create(_ORG, kind="destination", key="dest") - first = store.components.create(_ORG, kind="connection", key="first", config={}, encrypted=False) - second = store.components.create(_ORG, kind="connection", key="second", config={}, encrypted=False) - asset = store.components.create( - _ORG, - kind="asset", - key="a", - relations={"destination": [(dest.id, "")], "resource": [(first.id, "conn")]}, - ) + class Campaigns(il.Asset): + """Campaign entities of the first provider.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class SecondCampaignSource(il.Source): + """Another source declaring a ``campaigns`` asset.""" + + class Campaigns(il.Asset): + """Campaign entities of the second provider.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +@pytest.fixture +def store(component_db: Engine) -> Store: + """A store whose catalog carries every class the relation tests declare. + + Returns: + A store reading and writing the fixture database. + """ + catalog = il.Catalog.from_assets( + [ + GuardUpstream, + GuardOther, + GuardRequired, + GuardOptional, + Matcher, + WireUpSource, + WireOtherSource, + WireDownSource, + FirstCampaignSource, + SecondCampaignSource, + ] + ) + return Store(catalog=catalog) + + +@pytest.fixture +def connection(store: Store) -> Component: + """A plaintext connection row the wire sources accept. + + Returns: + The created connection component. + """ + return store.components.create(_ORG, kind="connection", key="wire_connection", config={}, encrypted=False) + + +class TestAdd: + """``add`` binds by name, through the declared relation's acceptance rule.""" + + def test_single_valued_relation_repoints(self, store: Store, connection: Component, component_db: Engine): + other = store.components.create(_ORG, kind="connection", key="wire_connection", config={}, encrypted=False) + source = store.components.create(_ORG, kind="source", key="wire_down_source") - store.components.update(asset.id, relations={"resource": [(second.id, "conn")]}) + store.relations.add(source.id, name="connection", dst_id=connection.id) + store.relations.add(source.id, name="connection", dst_id=other.id) with Session(component_db) as session: - assert [r.dst_id for r in _relations(session, asset.id, "resource")] == [second.id] - assert len(_relations(session, asset.id, "destination")) == 1 + assert [r.dst_id for r in _relations(session, source.id, "connection")] == [other.id] + + def test_many_valued_relation_accumulates(self, store: Store, component_db: Engine): + matcher = store.components.create(_ORG, kind="asset", key="matcher") + first = store.components.create(_ORG, kind="source", key="first_campaign_source") + second = store.components.create(_ORG, kind="source", key="second_campaign_source") + + store.relations.add(matcher.id, name="campaigns", dst_id=_child(first, "campaigns").id) + store.relations.add(matcher.id, name="campaigns", dst_id=_child(second, "campaigns").id) - def test_empty_list_clears_the_type(self, store: Store, component_db: Engine): - dest = store.components.create(_ORG, kind="destination", key="dest") - asset = store.components.create(_ORG, kind="asset", key="a", relations={"destination": [(dest.id, "")]}) - store.components.update(asset.id, relations={"destination": []}) with Session(component_db) as session: - assert _relations(session, asset.id) == [] + assert len(_relations(session, matcher.id, "campaigns")) == 2 - def test_rejects_types_outside_the_kind_vocabulary(self, store: Store): - dest = store.components.create(_ORG, kind="destination", key="dest") - with pytest.raises(ConfigError): - store.components.create(_ORG, kind="destination", key="d2", relations={"target": [(dest.id, "")]}) + def test_wildcard_key_refuses_a_parentless_asset(self, store: Store): + matcher = store.components.create(_ORG, kind="asset", key="matcher") + standalone = store.components.create(_ORG, kind="asset", key="campaigns") - def test_rejects_missing_and_cross_org_destinations(self, store: Store): - other = store.components.create(uuid4(), kind="destination", key="dest") - with pytest.raises(NotFoundError): - store.components.create(_ORG, kind="asset", key="a", relations={"destination": [(uuid4(), "")]}) - with pytest.raises(NotFoundError): - store.components.create(_ORG, kind="asset", key="b", relations={"destination": [(other.id, "")]}) + with pytest.raises(ConfigError, match="does not accept"): + store.relations.add(matcher.id, name="campaigns", dst_id=standalone.id) - def test_add_and_remove_relation(self, store: Store): - upstream = store.components.create(_ORG, kind="asset", key="a") - downstream = store.components.create(_ORG, kind="asset", key="b") + def test_rejects_a_kind_the_relation_does_not_declare(self, store: Store): + source = store.components.create(_ORG, kind="source", key="wire_down_source") + destination = store.components.create(_ORG, kind="destination", key="dest") - relation = store.relations.add(downstream.id, type="upstream", dst_id=upstream.id, slot="a") - assert (relation.src_id, relation.dst_id, relation.slot) == (downstream.id, upstream.id, "a") - assert len(store.relations.list_all(_ORG, type="upstream")) == 1 + with pytest.raises(ConfigError, match="does not accept"): + store.relations.add(source.id, name="connection", dst_id=destination.id) - store.relations.remove(downstream.id, type="upstream", dst_id=upstream.id) - assert store.relations.list_all(_ORG) == [] + def test_rejects_an_undeclared_name(self, store: Store, connection: Component): + source = store.components.create(_ORG, kind="source", key="wire_down_source") + with pytest.raises(ConfigError, match="declares no relation 'nope'"): + store.relations.add(source.id, name="nope", dst_id=connection.id) -class TestRelationKindEnforcement: - """Relation writes are checked against the vocabulary's allowed kinds.""" + def test_rejects_an_undeclared_key(self, store: Store): + upstream = store.components.create(_ORG, kind="asset", key="guard_other") + required = store.components.create(_ORG, kind="asset", key="guard_required") - @pytest.fixture - def demo_store(self, component_db: Engine) -> Store: - from interloper_assets.demo.source import DemoSource, demo_asset + with pytest.raises(ConfigError, match="does not accept"): + store.relations.add(required.id, name="up", dst_id=upstream.id) - return Store(catalog=il.Catalog.from_assets([DemoSource, demo_asset])) + def test_checks_a_declared_key_against_the_parent_source(self, store: Store): + wire_up = store.components.create(_ORG, kind="source", key="wire_up_source") + other = store.components.create(_ORG, kind="source", key="wire_other_source") + down = store.components.create(_ORG, kind="source", key="wire_down_source", config={}) + consumer = _child(down, "consumer") - def test_class_vocabulary_governs_writes(self, demo_store: Store): - db_job = demo_store.components.create(_ORG, kind="job", key="cron_job", name="J") - # TriggerHook declares `target`; WebhookHook does not. - ok = demo_store.components.create( - _ORG, kind="hook", key="trigger_hook", name="T", relations={"target": [(db_job.id, "")]} - ) - assert ok.id is not None - with pytest.raises(ConfigError, match="'webhook_hook'.*declare no 'target' relations"): - demo_store.components.create( - _ORG, kind="hook", key="webhook_hook", name="W", relations={"target": [(db_job.id, "")]} - ) + with pytest.raises(ConfigError, match="does not accept"): + store.relations.add(consumer.id, name="rows", dst_id=_child(other, "rows").id) - def test_relation_to_disallowed_kind_rejected(self, demo_store: Store): - db_source = demo_store.components.create(_ORG, kind="source", key="demo_source", name="Demo") - db_job = demo_store.components.create(_ORG, kind="job", key="cron_job", name="Job") - # A job's 'target' may point at sources/assets — never at another job. - with pytest.raises(ConfigError, match="may not point at a 'job'"): - demo_store.components.create( - _ORG, kind="job", key="cron_job", name="Bad", relations={"target": [(db_job.id, "")]} - ) - # Sanity: the allowed kind passes. - ok = demo_store.components.create( - _ORG, kind="job", key="cron_job", name="Good", relations={"target": [(db_source.id, "")]} - ) - assert ok.id is not None + relation = store.relations.add(consumer.id, name="rows", dst_id=_child(wire_up, "rows").id) + assert relation.dst_id == _child(wire_up, "rows").id + def test_identical_add_returns_the_existing_row(self, store: Store, connection: Component): + source = store.components.create(_ORG, kind="source", key="wire_down_source") -class TestDependencySlotValidation: - """Dependency writes are checked against the declared slots and their target identity.""" + first = store.relations.add(source.id, name="connection", dst_id=connection.id) + second = store.relations.add(source.id, name="connection", dst_id=connection.id) - @pytest.fixture - def demo_store(self, component_db: Engine) -> Store: - from interloper_assets.demo.source import DemoSource + assert (second.src_id, second.name, second.dst_id) == (first.src_id, first.name, first.dst_id) + assert len(store.relations.list_all(_ORG, name="connection")) == 1 - return Store(catalog=il.Catalog.from_assets([DemoSource])) + def test_stamps_the_denormalized_org_and_kinds(self, store: Store, connection: Component): + source = store.components.create(_ORG, kind="source", key="wire_down_source") - @pytest.fixture - def wire_store(self, component_db: Engine) -> Store: - return Store(catalog=il.Catalog.from_assets([WireUpSource, WireDownSource])) + relation = store.relations.add(source.id, name="connection", dst_id=connection.id) - def test_undeclared_slot_rejected(self, demo_store: Store): - source = demo_store.components.create(_ORG, kind="source", key="demo_source") - with pytest.raises(ConfigError, match="declares no slot 'nope'"): - demo_store.relations.add( - _child(source, "b").id, type="upstream", dst_id=_child(source, "a").id, slot="nope" - ) + assert (relation.org_id, relation.src_kind, relation.dst_kind) == (_ORG, "source", "connection") - def test_wrong_target_key_rejected(self, demo_store: Store): - source = demo_store.components.create(_ORG, kind="source", key="demo_source") - with pytest.raises(ConfigError, match="expects asset 'demo_source.a', got 'e'"): - demo_store.relations.add( - _child(source, "b").id, type="upstream", dst_id=_child(source, "e").id, slot="a" - ) + def test_a_missing_source_raises(self, store: Store, connection: Component): + missing = uuid4() - def test_self_edge_rejected(self, store: Store): - asset = store.components.create(_ORG, kind="asset", key="a") - with pytest.raises(ConfigError, match="itself"): - store.relations.add(asset.id, type="upstream", dst_id=asset.id, slot="x") - - def test_cross_instance_sibling_rejected(self, demo_store: Store): - first = demo_store.components.create(_ORG, kind="source", key="demo_source") - second = demo_store.components.create(_ORG, kind="source", key="demo_source", config={"dataset": "other"}) - with pytest.raises(ConfigError, match="sibling asset of the same source instance"): - demo_store.relations.add( - _child(first, "b").id, type="upstream", dst_id=_child(second, "a").id, slot="a" - ) + with pytest.raises(NotFoundError, match=f"Component {missing} not found"): + store.relations.add(missing, name="connection", dst_id=connection.id) - def test_cross_source_dep_accepts_any_instance(self, wire_store: Store): - up_two = wire_store.components.create(_ORG, kind="source", key="wire_up_source", config={"dataset": "two"}) - down = wire_store.components.create(_ORG, kind="source", key="wire_down_source") - relation = wire_store.relations.add( - _child(down, "consumer").id, type="upstream", dst_id=_child(up_two, "rows").id, slot="rows" - ) - assert relation.dst_id == _child(up_two, "rows").id + def test_a_cross_org_target_raises(self, store: Store): + source = store.components.create(_ORG, kind="source", key="wire_down_source") + foreign = store.components.create(uuid4(), kind="connection", key="wire_connection", config={}, encrypted=False) - def test_cross_source_dep_rejects_wrong_source(self, component_db: Engine): - from interloper_assets.demo.source import DemoSource + with pytest.raises(NotFoundError, match=f"Component {foreign.id} not found"): + store.relations.add(source.id, name="connection", dst_id=foreign.id) - store = Store(catalog=il.Catalog.from_assets([DemoSource, WireDownSource])) - demo = store.components.create(_ORG, kind="source", key="demo_source") - down = store.components.create(_ORG, kind="source", key="wire_down_source") - with pytest.raises(ConfigError, match="expects asset 'wire_up_source.rows'"): - store.relations.add( - _child(down, "consumer").id, type="upstream", dst_id=_child(demo, "a").id, slot="rows" - ) +class TestRemove: + """``remove`` refuses to empty a non-optional relation.""" -class TestRelationUpsert: - """Slotted relation writes upsert per slot.""" + def test_last_row_of_a_required_relation_is_refused(self, store: Store): + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream") + required = store.components.create(_ORG, kind="asset", key="guard_required", relations={"up": [upstream.id]}) - @pytest.fixture - def wire_store(self, component_db: Engine) -> Store: - return Store(catalog=il.Catalog.from_assets([WireUpSource, WireDownSource])) + with pytest.raises(ConfigError, match="non-optional"): + store.relations.remove(required.id, name="up", dst_id=upstream.id) + assert len(store.relations.list_all(_ORG, name="up")) == 1 - def test_rebinding_a_slot_repoints_it(self, wire_store: Store): - up_one = wire_store.components.create(_ORG, kind="source", key="wire_up_source") - up_two = wire_store.components.create(_ORG, kind="source", key="wire_up_source", config={"dataset": "two"}) - down = wire_store.components.create(_ORG, kind="source", key="wire_down_source") - consumer = _child(down, "consumer") + def test_last_row_of_an_optional_relation_detaches(self, store: Store): + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream") + optional = store.components.create(_ORG, kind="asset", key="guard_optional", relations={"up": [upstream.id]}) + + store.relations.remove(optional.id, name="up", dst_id=upstream.id) - wire_store.relations.add(consumer.id, type="upstream", dst_id=_child(up_one, "rows").id, slot="rows") - wire_store.relations.add(consumer.id, type="upstream", dst_id=_child(up_two, "rows").id, slot="rows") + assert store.relations.list_all(_ORG, name="up") == [] - (edge,) = wire_store.relations.list_all(_ORG, type="upstream") - assert edge.dst_id == _child(up_two, "rows").id + def test_an_absent_row_is_a_no_op(self, store: Store): + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream") + required = store.components.create(_ORG, kind="asset", key="guard_required") - def test_identical_add_is_a_noop(self, wire_store: Store): - up = wire_store.components.create(_ORG, kind="source", key="wire_up_source") - down = wire_store.components.create(_ORG, kind="source", key="wire_down_source") - consumer, rows = _child(down, "consumer"), _child(up, "rows") + store.relations.remove(required.id, name="up", dst_id=upstream.id) - first = wire_store.relations.add(consumer.id, type="upstream", dst_id=rows.id, slot="rows") - second = wire_store.relations.add(consumer.id, type="upstream", dst_id=rows.id, slot="rows") + assert store.relations.list_all(_ORG) == [] - assert (second.src_id, second.dst_id, second.slot) == (first.src_id, first.dst_id, first.slot) - assert len(wire_store.relations.list_all(_ORG, type="upstream")) == 1 +class TestListAll: + """``list_all`` filters by name and by either endpoint's kind.""" -class TestRequiredDependencyUnbindGuard: - """Bound required dependency slots refuse unbinding (repoint instead).""" + def test_filters_by_kinds(self, store: Store, connection: Component): + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream") + store.components.create(_ORG, kind="asset", key="guard_required", relations={"up": [upstream.id]}) + source = store.components.create(_ORG, kind="source", key="wire_down_source") + store.relations.add(source.id, name="connection", dst_id=connection.id) - @pytest.fixture - def dep_store(self, component_db: Engine) -> Store: - return Store(catalog=il.Catalog.from_assets([GuardUpstream, GuardRequired, GuardOptional])) + rows = store.relations.list_all(_ORG, src_kind="asset", dst_kind="asset") - def test_remove_required_dependency_refused(self, dep_store: Store): - up = dep_store.components.create(_ORG, kind="asset", key="guard_upstream") - down = dep_store.components.create( - _ORG, kind="asset", key="guard_required", relations={"upstream": [(up.id, "up")]} - ) - with pytest.raises(ConfigError, match="cannot be unbound"): - dep_store.relations.remove(down.id, type="upstream", dst_id=up.id) - assert len(dep_store.relations.list_all(_ORG, type="upstream")) == 1 - - def test_remove_optional_dependency_allowed(self, dep_store: Store): - up = dep_store.components.create(_ORG, kind="asset", key="guard_upstream") - down = dep_store.components.create( - _ORG, kind="asset", key="guard_optional", relations={"upstream": [(up.id, "up")]} - ) - dep_store.relations.remove(down.id, type="upstream", dst_id=up.id) - assert dep_store.relations.list_all(_ORG, type="upstream") == [] + assert {row.name for row in rows} == {"up"} - def test_sync_clear_of_required_dependency_refused(self, dep_store: Store): - up = dep_store.components.create(_ORG, kind="asset", key="guard_upstream") - down = dep_store.components.create( - _ORG, kind="asset", key="guard_required", relations={"upstream": [(up.id, "up")]} + def test_filters_by_name(self, store: Store, connection: Component): + source = store.components.create(_ORG, kind="source", key="wire_down_source") + store.relations.add(source.id, name="connection", dst_id=connection.id) + + assert len(store.relations.list_all(_ORG, name="connection")) == 1 + assert store.relations.list_all(_ORG, name="up") == [] + + def test_is_scoped_to_the_organisation(self, store: Store, connection: Component): + source = store.components.create(_ORG, kind="source", key="wire_down_source") + store.relations.add(source.id, name="connection", dst_id=connection.id) + + assert store.relations.list_all(uuid4()) == [] + + +class TestSyncRelations: + """``_sync_relations`` replaces each listed name wholesale.""" + + def test_replaces_a_many_valued_relation_wholesale(self, store: Store, component_db: Engine): + first = store.components.create(_ORG, kind="source", key="first_campaign_source") + second = store.components.create(_ORG, kind="source", key="second_campaign_source") + matcher = store.components.create( + _ORG, + kind="asset", + key="matcher", + relations={"campaigns": [_child(first, "campaigns").id, _child(second, "campaigns").id]}, ) - with pytest.raises(ConfigError, match="cannot be unbound"): - dep_store.components.update(down.id, relations={"upstream": []}) - - def test_sync_repoint_of_required_dependency_allowed(self, dep_store: Store): - up_one = dep_store.components.create(_ORG, kind="asset", key="guard_upstream") - up_two = dep_store.components.create(_ORG, kind="asset", key="guard_upstream") - down = dep_store.components.create( - _ORG, kind="asset", key="guard_required", relations={"upstream": [(up_one.id, "up")]} + + store.components.update(matcher.id, relations={"campaigns": [_child(second, "campaigns").id]}) + + with Session(component_db) as session: + assert [r.dst_id for r in _relations(session, matcher.id, "campaigns")] == [_child(second, "campaigns").id] + + def test_leaves_the_names_it_is_not_given_alone(self, store: Store, connection: Component, component_db: Engine): + destination = store.components.create(_ORG, kind="destination", key="dest") + source = store.components.create( + _ORG, + kind="source", + key="wire_down_source", + relations={"connection": [connection.id], "destinations": [destination.id]}, ) - dep_store.components.update(down.id, relations={"upstream": [(up_two.id, "up")]}) - (edge,) = dep_store.relations.list_all(_ORG, type="upstream") - assert edge.dst_id == up_two.id + store.components.update(source.id, relations={"destinations": []}) -class TestAddValidation: - """``add`` vets the source, the vocabulary and the slot before writing.""" + with Session(component_db) as session: + assert len(_relations(session, source.id, "connection")) == 1 + assert _relations(session, source.id, "destinations") == [] - def test_a_missing_source_raises(self, store: Store): - missing = uuid4() + def test_refuses_to_empty_a_non_optional_relation(self, store: Store): + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream") + required = store.components.create(_ORG, kind="asset", key="guard_required", relations={"up": [upstream.id]}) - with pytest.raises(NotFoundError, match=f"Component {missing} not found"): - store.relations.add(missing, type="resource", dst_id=uuid4(), slot="connection") + with pytest.raises(ConfigError, match="non-optional"): + store.components.update(required.id, relations={"up": []}) + + def test_repointing_a_non_optional_relation_is_allowed(self, store: Store): + first = store.components.create(_ORG, kind="asset", key="guard_upstream") + second = store.components.create(_ORG, kind="asset", key="guard_upstream") + required = store.components.create(_ORG, kind="asset", key="guard_required", relations={"up": [first.id]}) + + store.components.update(required.id, relations={"up": [second.id]}) + + (row,) = store.relations.list_all(_ORG, name="up") + assert row.dst_id == second.id + + def test_rejects_an_undeclared_name(self, store: Store, connection: Component): + with pytest.raises(ConfigError, match="declares no relation 'nope'"): + store.components.create(_ORG, kind="source", key="wire_down_source", relations={"nope": [connection.id]}) + + def test_rejects_several_targets_on_a_single_valued_relation(self, store: Store, connection: Component): + other = store.components.create(_ORG, kind="connection", key="wire_connection", config={}, encrypted=False) + + with pytest.raises(ConfigError, match="single-valued"): + store.components.create( + _ORG, + kind="source", + key="wire_down_source", + relations={"connection": [connection.id, other.id]}, + ) - def test_a_slot_on_an_unslotted_relation_is_refused(self, component_db: Engine): - store = Store(catalog=il.Catalog.from_assets([DemoSource])) - source = store.components.create(_ORG, kind="source", key="demo_source") - destination = store.components.create(_ORG, kind="destination", key="dest") - with pytest.raises(ConfigError, match="is not slotted"): - store.relations.add(source.id, type="destination", dst_id=destination.id, slot="nope") +class TestIntraSourceWiring: + """A source's own sibling relations are wired from the class declaration.""" - def test_a_slot_expecting_another_key_is_refused(self, component_db: Engine): - # The slot declares the component key it accepts; anything else would - # be wired into a resource the source cannot use. - catalog = il.Catalog.from_assets([FacebookAds]) - store = Store(catalog=catalog, encrypt=lambda b: b, decrypt=lambda b: b) - source = store.components.create(_ORG, kind="source", key="facebook_ads") - wrong = store.components.create(_ORG, kind="destination", key="dest") + def test_creating_a_source_wires_its_sibling_relations(self, store: Store): + source = store.components.create(_ORG, kind="source", key="wire_up_source") - with pytest.raises(ConfigError): - store.relations.add(source.id, type="resource", dst_id=wrong.id, slot="connection") + (row,) = store.relations.list_all(_ORG, name="rows") + assert (row.src_id, row.dst_id) == (_child(source, "totals").id, _child(source, "rows").id) From c0b1fdea406bcf3142c6321b385bd15dfb758309 Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 02:05:49 +0200 Subject: [PATCH 03/19] docs(core): docstring corrections parked from the phase 1 review By Digitl --- packages/interloper-core/src/interloper/asset/base.py | 3 ++- packages/interloper-core/src/interloper/asset/upstream.py | 4 ++-- packages/interloper-core/src/interloper/catalog/base.py | 6 +++--- packages/interloper-core/src/interloper/component/base.py | 5 +++++ packages/interloper-core/src/interloper/source/base.py | 2 ++ 5 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/interloper-core/src/interloper/asset/base.py b/packages/interloper-core/src/interloper/asset/base.py index 202ee9f5..0e177141 100644 --- a/packages/interloper-core/src/interloper/asset/base.py +++ b/packages/interloper-core/src/interloper/asset/base.py @@ -337,7 +337,8 @@ def __call__( Two exceptions: ``normalizer``, whose sentinel default lets an explicit ``None`` clear the configured normalizer, and a name in **relations, where ``None`` clears the binding, so only leaving the name out - entirely leaves it as is. + entirely leaves it as is. Clearing a non-optional relation is refused + with a ``ConfigError``, since it cannot be left empty. The copy carries this asset's own bindings and parent, so a copy made to flip one field (the non-materializable parents of a mini-DAG, say) diff --git a/packages/interloper-core/src/interloper/asset/upstream.py b/packages/interloper-core/src/interloper/asset/upstream.py index 357a25ba..6cf3ad72 100644 --- a/packages/interloper-core/src/interloper/asset/upstream.py +++ b/packages/interloper-core/src/interloper/asset/upstream.py @@ -1,4 +1,4 @@ -"""The leg object handed to ``data()`` for many-valued upstream slots.""" +"""The leg object handed to ``data()`` for many-valued upstream relations.""" from __future__ import annotations @@ -11,7 +11,7 @@ @dataclass(frozen=True) class Upstream: - """One leg of a many-valued slot as handed to ``data()``. + """One leg of a many-valued relation as handed to ``data()``. Attributes: asset: The upstream asset the data was read from; its ``source``, diff --git a/packages/interloper-core/src/interloper/catalog/base.py b/packages/interloper-core/src/interloper/catalog/base.py index 159468d8..ff0cb71c 100644 --- a/packages/interloper-core/src/interloper/catalog/base.py +++ b/packages/interloper-core/src/interloper/catalog/base.py @@ -62,7 +62,7 @@ def get(self, key: str, default: Any = None, *, parent_key: str | None = None) - :attr:`SourceDefinition.assets`. Pass *parent_key* to resolve such an asset the way its owner declares it: the concrete :class:`AssetDefinition`, carrying the composite import path, the - partitioning and the dependency slots the flat key cannot name. A + partitioning and the relations the flat key cannot name. A parent that does not resolve, or does not declare the key, falls back to the flat lookup. @@ -96,11 +96,11 @@ def vocabulary(self, kind: str, key: str, *, parent_key: str | None = None) -> d kind: The row's component kind. key: The row's catalog key. parent_key: Key of the owning source, for a source-owned asset, - whose declaration carries the dependency slots. Defaults to + whose declaration carries the asset's relations. Defaults to ``None``, a flat lookup. Returns: - Relation type → definition. + The declared relations keyed by name. """ definition = self.get(key, parent_key=parent_key) if definition is not None and definition.kind == kind: diff --git a/packages/interloper-core/src/interloper/component/base.py b/packages/interloper-core/src/interloper/component/base.py index dda230e3..7a73278d 100644 --- a/packages/interloper-core/src/interloper/component/base.py +++ b/packages/interloper-core/src/interloper/component/base.py @@ -346,6 +346,11 @@ def bind(self, name: str, *targets: Component) -> None: single-valued one replaces what it holds, so repointing it is a second :meth:`bind` and needs no :meth:`unbind` first. + A target whose kind or key the relation does not accept, and more than + one target at once on a single-valued relation, are both refused with a + ``ConfigError`` from :meth:`_replace_binding`, which every write path + goes through. + Args: name: The relation name as declared on the class. *targets: The components to bind. Binding nothing is a no-op, so a diff --git a/packages/interloper-core/src/interloper/source/base.py b/packages/interloper-core/src/interloper/source/base.py index 3b9aecd3..45d1b30a 100644 --- a/packages/interloper-core/src/interloper/source/base.py +++ b/packages/interloper-core/src/interloper/source/base.py @@ -396,6 +396,8 @@ def __call__( name in **relations follows a different rule: passing it at all changes it, since ``None`` there clears the binding rather than leaving it alone; only leaving the name out entirely leaves it as is. + Clearing a non-optional relation is refused with a ``ConfigError``, + since it cannot be left empty. Args: dataset: Replacement dataset. Assets that inherited the source's From c22a4f573bfd4fa28226ec34bd73fcc20fd3c9d7 Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 02:22:54 +0200 Subject: [PATCH 04/19] fix(db): sibling rows come from Source.sibling_bindings; locks on every write; self-edge and cross-instance guards By Digitl --- .../src/interloper/component/base.py | 28 ++- .../src/interloper/resource/__init__.py | 2 +- .../tests/component/test_base.py | 15 ++ .../src/interloper_db/store/components.py | 40 +--- .../src/interloper_db/store/relations.py | 111 ++++++++-- .../tests/store/test_relations.py | 190 +++++++++++++++++- 6 files changed, 315 insertions(+), 71 deletions(-) diff --git a/packages/interloper-core/src/interloper/component/base.py b/packages/interloper-core/src/interloper/component/base.py index 7a73278d..67ad0bac 100644 --- a/packages/interloper-core/src/interloper/component/base.py +++ b/packages/interloper-core/src/interloper/component/base.py @@ -346,10 +346,10 @@ def bind(self, name: str, *targets: Component) -> None: single-valued one replaces what it holds, so repointing it is a second :meth:`bind` and needs no :meth:`unbind` first. - A target whose kind or key the relation does not accept, and more than - one target at once on a single-valued relation, are both refused with a - ``ConfigError`` from :meth:`_replace_binding`, which every write path - goes through. + A target whose kind or key the relation does not accept, this + component itself, and more than one target at once on a single-valued + relation are all refused with a ``ConfigError`` from + :meth:`_replace_binding`, which every write path goes through. Args: name: The relation name as declared on the class. @@ -688,11 +688,14 @@ def _discriminator_fields(cls) -> list[str]: def _check_targets(self, name: str, relation: Relation, targets: tuple[Component, ...]) -> None: """Check that *targets* are legal for one of this component's relations. - Every target must be one the relation :meth:`~Relation.accepts`, and a - single-valued relation may not receive more than one target at once. - Performs no mutation, so :meth:`_replace_binding` can call it before - touching ``_bound`` and a rejected replacement leaves the existing - binding untouched. + Every target must be one the relation :meth:`~Relation.accepts` and + must be another component: a relation whose declared keys match the + declaring component's own key (a wildcard, or an asset named after + the key it reads) would otherwise let it fill itself. A single-valued + relation may not receive more than one target at once. Performs no + mutation, so :meth:`_replace_binding` can call it before touching + ``_bound`` and a rejected replacement leaves the existing binding + untouched. Args: name: The relation name as declared on the class. @@ -700,11 +703,14 @@ def _check_targets(self, name: str, relation: Relation, targets: tuple[Component targets: The candidate components to check. Raises: - ConfigError: If a target's kind or key is not one the relation - accepts, or if a single-valued relation is given more than one target. + ConfigError: If a target is this component itself, if a target's + kind or key is not one the relation accepts, or if a + single-valued relation is given more than one target. """ owner = self.identity for target in targets: + if target is self: + raise ConfigError(f"{type(self).__name__}.{name} cannot point at the component itself") if not relation.accepts(target.kind, target.identity, owner=owner): raise ConfigError( f"{type(self).__name__}.{name} does not accept {target.kind} '{target.qualified_key}' " diff --git a/packages/interloper-core/src/interloper/resource/__init__.py b/packages/interloper-core/src/interloper/resource/__init__.py index ef08c8f5..8bb75615 100644 --- a/packages/interloper-core/src/interloper/resource/__init__.py +++ b/packages/interloper-core/src/interloper/resource/__init__.py @@ -1,4 +1,4 @@ -"""Resources: injectable dependencies, their typed slots, and their field helpers.""" +"""Resources: injectable dependencies, their typed relations, and their field helpers.""" from interloper.resource.base import Resource, ResourceDefinition from interloper.resource.fields import ( diff --git a/packages/interloper-core/tests/component/test_base.py b/packages/interloper-core/tests/component/test_base.py index 292c0c26..e2b22b1e 100644 --- a/packages/interloper-core/tests/component/test_base.py +++ b/packages/interloper-core/tests/component/test_base.py @@ -274,6 +274,12 @@ class Gadget(il.Source): store: NeedyDest = il.Relation(NeedyDest) +class Peer(il.Source): + """Source whose ``peers`` relation declares no key, so it would accept itself.""" + + peers: list[il.Source] = il.Relation("source", many=True, optional=True) + + class TestCollect: def test_annotation_becomes_relation(self): assert Widget.relations["connection"].kind == "connection" @@ -353,6 +359,15 @@ def test_wrong_kind_rejected(self): with pytest.raises(ConfigError, match="connection"): Widget(connection=Cfg()) # ty: ignore[invalid-argument-type] + def test_a_relation_refuses_the_component_itself(self): + peer = Peer() + other = Peer() + + with pytest.raises(ConfigError, match="cannot point at the component itself"): + peer.bind("peers", peer) + peer.bind("peers", other) + assert peer.peers == [other] + def test_single_relation_rejects_two_targets_at_once(self): widget = Widget(connection=Conn(api_secret="s")) with pytest.raises(ConfigError, match="single"): diff --git a/packages/interloper-db/src/interloper_db/store/components.py b/packages/interloper-db/src/interloper_db/store/components.py index 1814b2dc..9c57af6b 100644 --- a/packages/interloper-db/src/interloper_db/store/components.py +++ b/packages/interloper-db/src/interloper_db/store/components.py @@ -785,45 +785,7 @@ def _ensure_children(self, session: Session, db_source: Component, child_keys: l session.add(children[key]) session.flush() - self._wire_intra_upstreams(session, source_cls, children) - - @staticmethod - def _wire_intra_upstreams( - session: Session, - source_cls: type[il.Source], - children_by_key: dict[str, Component], - ) -> None: - """Top up missing intra-source asset relations from class metadata. - - Idempotent over the full child set — assets enabled after their - siblings still get the edges *into* them wired. Names that already - hold an edge are never touched, so manual bindings survive. - - Args: - session: Open session the relations are added to. - source_cls: The catalog class declaring the relations. - children_by_key: The source's enabled child rows, keyed by asset key. - """ - child_ids = [child.id for child in children_by_key.values()] - bound = { - (row[0], row[1]) - for row in session.exec( - select(ComponentRelation.src_id, ComponentRelation.name).where( - col(ComponentRelation.src_id).in_(child_ids) - ) - ).all() - } - for asset_type in source_cls.asset_types: - child = children_by_key.get(asset_type.key) - if child is None: - continue - for name, relation in asset_type.relations.items(): - if (child.id, name) in bound or "asset" not in relation.kinds() or not relation.source_local: - continue - declared_keys = relation.keys() - for sibling_key in declared_keys: - if (sibling := children_by_key.get(sibling_key)) is not None: - RelationStore._insert(session, child, sibling, name) + self._relations.bind_siblings(session, source_cls, children) def job_partition_granularity(self, session: Session, job_id: UUID) -> TimeGranularity | None: """Resolve the granularity a job's partitioned targets share. diff --git a/packages/interloper-db/src/interloper_db/store/relations.py b/packages/interloper-db/src/interloper_db/store/relations.py index 5e7625e5..36723c54 100644 --- a/packages/interloper-db/src/interloper_db/store/relations.py +++ b/packages/interloper-db/src/interloper_db/store/relations.py @@ -7,11 +7,14 @@ :class:`~interloper.component.relation.Relation` whether it accepts the candidate, so the database enforces exactly what the framework enforces in memory: the declared kinds, and the identity the declared keys expect -(bare, qualified or wildcard). Single-valued names hold one edge and -repoint on rewrite; ``many`` names accumulate. A non-optional name cannot -be emptied, only repointed. Rows are stamped with the denormalized -``org_id``/``src_kind``/``dst_kind`` triple the composite foreign keys -verify. +(bare, qualified or wildcard). No component fills its own relation, and a +bare key stays inside the owner's own source instance, which identities +alone cannot tell apart. Single-valued names hold one edge and repoint on +rewrite; ``many`` names accumulate. A non-optional name cannot be +emptied, only repointed. Every write takes the source row's lock, so the +guards cannot be read around concurrently. Rows are stamped with the +denormalized ``org_id``/``src_kind``/``dst_kind`` triple the composite +foreign keys verify. """ from __future__ import annotations @@ -22,7 +25,7 @@ from interloper.catalog.base import Catalog from interloper.errors import ConfigError, NotFoundError from sqlalchemy import Engine -from sqlmodel import Session, select +from sqlmodel import Session, col, select from interloper_db.models import Component, ComponentRelation from interloper_db.session import commit, session_scope @@ -115,6 +118,11 @@ def add(self, component_id: UUID, *, name: str, dst_id: UUID) -> ComponentRelati def remove(self, component_id: UUID, *, name: str, dst_id: UUID) -> None: """Detach one component from another under a declared relation name. + Takes the source row's lock before reading the edges, so the + non-optional guard below cannot be read around by a concurrent write. + A missing source propagates as a ``NotFoundError`` from + :meth:`_lock`. + Args: component_id: Source component the relation originates from. name: Relation name the edge is filed under. @@ -126,25 +134,73 @@ def remove(self, component_id: UUID, *, name: str, dst_id: UUID) -> None: relation, which may be repointed but not emptied. """ with session_scope(self._engine) as session: - rows = self._rows(session, component_id, name) + src = self._lock(session, component_id) + rows = self._rows(session, src.id, name) row = next((candidate for candidate in rows if candidate.dst_id == dst_id), None) if row is None: return - src = session.get(Component, component_id) - relation = self._vocabulary(session, src).get(name) if src is not None else None + relation = self._vocabulary(session, src).get(name) if len(rows) == 1 and relation is not None and not relation.optional: raise ConfigError( - f"'{src.key if src else component_id}'.{name} is non-optional and cannot be emptied; " + f"'{src.key}'.{name} is non-optional and cannot be emptied; " f"repoint it or remove the dependent component instead" ) session.delete(row) commit(session) + def bind_siblings( + self, + session: Session, + source_cls: type[il.Source], + children_by_key: dict[str, Component], + ) -> None: + """Top up the intra-source edges a source class binds between its own assets. + + The wiring is read from + :meth:`~interloper.source.base.Source.sibling_bindings`, the same + classmethod a live source binds its assets from, so a persisted source + holds exactly the edges its in-memory counterpart does: one edge per + relation name, never onto the declaring asset itself, and nothing for a + declared key that reaches outside the source. Idempotent over the full + child set, so an asset enabled after its siblings still gets the edges + into it wired, while a name that already holds an edge is left alone + and a binding made by hand survives. + + Args: + session: Open session the edges are added to; neither flushed nor + committed here. + source_cls: Catalog class whose declaration decides the wiring. + children_by_key: The source's enabled child rows, keyed by asset key. + """ + bound = { + (row[0], row[1]) + for row in session.exec( + select(ComponentRelation.src_id, ComponentRelation.name).where( + col(ComponentRelation.src_id).in_([child.id for child in children_by_key.values()]) + ) + ).all() + } + for asset_key, names in source_cls.sibling_bindings().items(): + child = children_by_key.get(asset_key) + if child is None: + continue + for name, sibling_key in names.items(): + sibling = children_by_key.get(sibling_key) + if sibling is None or (child.id, name) in bound: + continue + self._insert(session, child, sibling, name) + # -- Internals ------------------------------------------------------------- def _sync_relations(self, session: Session, src: Component, bindings: dict[str, list[Binding]] | None) -> None: """Replace the relation names present in *bindings* (empty list clears). + Takes the source row's lock, like every other write path, so two + concurrent replacements of one name cannot interleave their deletes + and inserts. An undeclared name and a destination the declared + relation refuses both propagate from the checks this delegates to + (:meth:`_relation`, :meth:`_resolve`). + Args: session: Open session the replacement is written through; deletes and inserts are flushed, never committed here. @@ -154,12 +210,14 @@ def _sync_relations(self, session: Session, src: Component, bindings: dict[str, None leaves every relation untouched. Raises: - ConfigError: If a name is not declared by the source's class, if a - single-valued name is given several destinations, if the - relation does not accept one of them, or if the replacement - would empty a non-optional relation. + ConfigError: If a single-valued name is given several + destinations, or if the replacement would empty a + non-optional relation. """ - for name, dst_ids in (bindings or {}).items(): + if not bindings: + return + src = self._lock(session, src.id) + for name, dst_ids in bindings.items(): relation = self._relation(session, src, name) if not relation.many and len(dst_ids) > 1: raise ConfigError(f"'{src.key}'.{name} is single-valued and takes one target at a time") @@ -255,6 +313,13 @@ def _identity(self, session: Session, row: Component) -> il.ComponentIdentity: def _resolve(self, session: Session, src: Component, relation: il.Relation, name: str, dst_id: UUID) -> Component: """Load a relation destination and check the declared relation accepts it. + Two rules the identity match cannot see are checked here. A component + never fills its own relation, whatever the declared keys match. And a + bare declared key names a component of the owner's *own* source + instance, while identities carry only the catalog key of the owning + source: two instances of one source have the same identity, so the + instance is compared by parent row. + Args: session: Open session the destination row is loaded through. src: Source component the relation originates from. @@ -268,17 +333,23 @@ def _resolve(self, session: Session, src: Component, relation: il.Relation, name Raises: NotFoundError: If the destination is missing or belongs to another organisation. - ConfigError: If the relation does not accept the destination's kind - or identity. + ConfigError: If the destination is the source itself, if the + relation does not accept the destination's kind or identity, + or if a sibling relation is pointed at another source + instance's component. """ dst = session.get(Component, dst_id) if dst is None or dst.org_id != src.org_id: raise NotFoundError(f"Component {dst_id} not found (relation '{name}')") + if dst.id == src.id: + raise ConfigError(f"'{src.key}'.{name} cannot point at the component itself") if not relation.accepts(dst.kind, self._identity(session, dst), owner=self._identity(session, src)): raise ConfigError( f"'{src.key}'.{name} does not accept {dst.kind} '{dst.key}' " f"(declared: kind {relation.kinds()}, key {relation.keys() or 'any'})" ) + if relation.source_local and src.parent_id is not None and dst.parent_id != src.parent_id: + raise ConfigError(f"'{src.key}'.{name} names a sibling; '{dst.key}' belongs to another source instance") return dst def _lock(self, session: Session, component_id: UUID) -> Component: @@ -286,8 +357,10 @@ def _lock(self, session: Session, component_id: UUID) -> Component: The row lock serializes concurrent writes to the same source, so two rebinds of one single-valued name cannot both read an empty edge set - and both insert. SQLite has no row locks and needs none: its writes - are serialized database-wide. + and both insert. Every write path goes through here, which is the + only guarantee available to a reader: SQLite, what the tests run on, + has no row locks and needs none, since its writes are serialized + database-wide, so no test can observe the lock being taken. Args: session: Open session the row is loaded through. diff --git a/packages/interloper-db/tests/store/test_relations.py b/packages/interloper-db/tests/store/test_relations.py index 49856d97..9d544df9 100644 --- a/packages/interloper-db/tests/store/test_relations.py +++ b/packages/interloper-db/tests/store/test_relations.py @@ -134,6 +134,112 @@ def data(self, context: il.ExecutionContext) -> list[dict]: return [] +class TwoKeySource(il.Source): + """Source naming two of its own assets on one single-valued relation.""" + + class First(il.Asset): + """One of the two candidate siblings.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + class Second(il.Asset): + """The other candidate sibling.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + class Totals(il.Asset): + """Asset declaring both sibling keys under one single-valued name.""" + + rows = il.Relation("asset", ["first", "second"]) + + def data(self, context: il.ExecutionContext, rows: il.Upstream) -> list[dict]: + return [] + + +class SelfKeySource(il.Source): + """Source whose only asset declares a bare key naming its own key.""" + + class Rows(il.Asset): + """Asset whose ``peers`` relation names ``rows``, which is its own key.""" + + peers: list[il.Asset] = il.Relation("asset", "rows", many=True) + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class MixedKeySource(il.Source): + """Source mixing a bare sibling key and a qualified cross-source one on one relation.""" + + class Rows(il.Asset): + """The sibling the bare key names.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + class Totals(il.Asset): + """Asset fanning in its own source's ``rows`` and another source's.""" + + rows: list[il.Asset] = il.Relation("asset", ["rows", "wire_up_source.rows"], many=True) + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class SelfCampaignSource(il.Source): + """Source whose ``campaigns`` asset fans in every source's ``campaigns``, its own included.""" + + class Campaigns(il.Asset): + """Asset whose wildcard relation matches its own identity.""" + + peers: list[il.Asset] = il.Relation("asset", "*.campaigns", many=True) + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class InstanceSource(il.Source): + """Discriminated source, so two of its instances coexist in one organisation.""" + + account_id: str = il.InputField(default="", discriminator=True) + + class Rows(il.Asset): + """Root asset of the instance.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + class Totals(il.Asset): + """Asset reading its own instance's ``rows``.""" + + rows = il.Relation("asset", "rows") + + def data(self, context: il.ExecutionContext, rows: il.Upstream) -> list[dict]: + return [] + + +class ConsumerSource(il.Source): + """Source reading another source's ``rows`` by qualified key and by wildcard.""" + + class Qualified(il.Asset): + """Asset pinned to ``instance_source.rows``, whichever instance it comes from.""" + + rows: list[il.Asset] = il.Relation("asset", "instance_source.rows", many=True) + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + class Wild(il.Asset): + """Asset fanning in the ``rows`` asset of any source.""" + + rows: list[il.Asset] = il.Relation("asset", "*.rows", many=True) + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + @pytest.fixture def store(component_db: Engine) -> Store: """A store whose catalog carries every class the relation tests declare. @@ -153,6 +259,12 @@ def store(component_db: Engine) -> Store: WireDownSource, FirstCampaignSource, SecondCampaignSource, + TwoKeySource, + SelfKeySource, + MixedKeySource, + SelfCampaignSource, + InstanceSource, + ConsumerSource, ] ) return Store(catalog=catalog) @@ -382,11 +494,87 @@ def test_rejects_several_targets_on_a_single_valued_relation(self, store: Store, ) +class TestSelfEdge: + """No component fills its own relation, whatever the declared keys match.""" + + def test_add_refuses_the_component_itself(self, store: Store): + source = store.components.create(_ORG, kind="source", key="self_campaign_source") + campaigns = _child(source, "campaigns") + + with pytest.raises(ConfigError, match="cannot point at the component itself"): + store.relations.add(campaigns.id, name="peers", dst_id=campaigns.id) + + def test_a_replacement_refuses_the_component_itself(self, store: Store): + source = store.components.create(_ORG, kind="source", key="self_campaign_source") + campaigns = _child(source, "campaigns") + + with pytest.raises(ConfigError, match="cannot point at the component itself"): + store.components.update(campaigns.id, relations={"peers": [campaigns.id]}) + + +class TestSiblingInstanceScope: + """A relation declaring only bare keys stays inside the owner's own source instance.""" + + def test_refuses_another_instances_asset(self, store: Store): + first = store.components.create(_ORG, kind="source", key="instance_source", config={"account_id": "a"}) + second = store.components.create(_ORG, kind="source", key="instance_source", config={"account_id": "b"}) + + with pytest.raises(ConfigError, match="belongs to another source instance"): + store.relations.add(_child(first, "totals").id, name="rows", dst_id=_child(second, "rows").id) + + def test_a_qualified_key_accepts_either_instance(self, store: Store, component_db: Engine): + first = store.components.create(_ORG, kind="source", key="instance_source", config={"account_id": "a"}) + second = store.components.create(_ORG, kind="source", key="instance_source", config={"account_id": "b"}) + qualified = _child(store.components.create(_ORG, kind="source", key="consumer_source"), "qualified") + + store.relations.add(qualified.id, name="rows", dst_id=_child(first, "rows").id) + store.relations.add(qualified.id, name="rows", dst_id=_child(second, "rows").id) + + with Session(component_db) as session: + assert {row.dst_id for row in _relations(session, qualified.id, "rows")} == { + _child(first, "rows").id, + _child(second, "rows").id, + } + + def test_a_wildcard_key_accepts_either_instance(self, store: Store, component_db: Engine): + first = store.components.create(_ORG, kind="source", key="instance_source", config={"account_id": "a"}) + second = store.components.create(_ORG, kind="source", key="instance_source", config={"account_id": "b"}) + wild = _child(store.components.create(_ORG, kind="source", key="consumer_source"), "wild") + + store.relations.add(wild.id, name="rows", dst_id=_child(first, "rows").id) + store.relations.add(wild.id, name="rows", dst_id=_child(second, "rows").id) + + with Session(component_db) as session: + assert {row.dst_id for row in _relations(session, wild.id, "rows")} == { + _child(first, "rows").id, + _child(second, "rows").id, + } + + class TestIntraSourceWiring: - """A source's own sibling relations are wired from the class declaration.""" + """A source's own sibling edges come from ``Source.sibling_bindings``.""" def test_creating_a_source_wires_its_sibling_relations(self, store: Store): source = store.components.create(_ORG, kind="source", key="wire_up_source") (row,) = store.relations.list_all(_ORG, name="rows") assert (row.src_id, row.dst_id) == (_child(source, "totals").id, _child(source, "rows").id) + + def test_two_declared_keys_on_a_single_valued_relation_wire_one_edge(self, store: Store): + source = store.components.create(_ORG, kind="source", key="two_key_source") + + (row,) = store.relations.list_all(_ORG, name="rows") + sibling_key = TwoKeySource.sibling_bindings()["totals"]["rows"] + assert (row.src_id, row.dst_id) == (_child(source, "totals").id, _child(source, sibling_key).id) + + def test_a_key_naming_the_assets_own_key_wires_no_self_edge(self, store: Store): + store.components.create(_ORG, kind="source", key="self_key_source") + + assert SelfKeySource.sibling_bindings() == {} + assert store.relations.list_all(_ORG) == [] + + def test_a_relation_mixing_bare_and_qualified_keys_wires_the_bare_one(self, store: Store): + source = store.components.create(_ORG, kind="source", key="mixed_key_source") + + (row,) = store.relations.list_all(_ORG, name="rows") + assert (row.src_id, row.dst_id) == (_child(source, "totals").id, _child(source, "rows").id) From beebb7df1e0841e4e055576cea029771b182857f Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 02:38:16 +0200 Subject: [PATCH 05/19] feat(db): component store reads relations by name; guards through on_delete By Digitl --- .../src/interloper_db/store/components.py | 55 +- .../tests/store/test_components.py | 578 +++++++++--------- .../tests/store/test_organisations.py | 2 +- 3 files changed, 324 insertions(+), 311 deletions(-) diff --git a/packages/interloper-db/src/interloper_db/store/components.py b/packages/interloper-db/src/interloper_db/store/components.py index 9c57af6b..cfe89505 100644 --- a/packages/interloper-db/src/interloper_db/store/components.py +++ b/packages/interloper-db/src/interloper_db/store/components.py @@ -7,7 +7,8 @@ encrypted into the ``data`` column (fail-closed without a key) and decoded on read — callers only ever see ``config``. - **source**: child asset rows are kept in sync with the catalog class's - ``asset_types`` after every write, including intra-source dependency wiring. + ``asset_types`` after every write, including the sibling relations the + class declares between its own assets. - **asset**: a source-owned asset hydrates through its parent; its drift status cascades through the parent's. - **job**: hydration drift-checks every target before reconstruction. @@ -44,16 +45,15 @@ from interloper_db.session import commit, session_scope from interloper_db.store.hydration import Hydrator from interloper_db.store.quotas import QUOTA_MAX_ASSETS_PER_SOURCE, QUOTA_MAX_SOURCES, QuotaStore -from interloper_db.store.relations import Binding, RelationStore +from interloper_db.store.relations import RelationStore from interloper_db.store.status import ComponentStatus, asset_status, source_status -# Eager-load set for rows returned to API consumers: the parent and children -# with their relations. The two-hop (component -> destination -> resources) -# legs are dropped for now: they walked ComponentRelation.dst, a relationship -# the relation-by-name rewrite removed. The rewritten store restores -# whatever eager loading it needs. +# Eager-load set for rows returned to API consumers: the parent, the row's +# own relations, and the children with theirs, so the whole unit reads off a +# detached row. COMPONENT_LOAD_OPTIONS = [ selectinload(Component.parent), # ty: ignore[invalid-argument-type] + selectinload(Component.out_relations), # ty: ignore[invalid-argument-type] selectinload(Component.children) # ty: ignore[invalid-argument-type] .selectinload(Component.out_relations), # ty: ignore[invalid-argument-type] ] @@ -100,7 +100,7 @@ def create( config: dict[str, Any] | None = None, encrypted: bool | None = None, children: list[str] | None = None, - relations: dict[str, list[Binding]] | None = None, + relations: dict[str, list[UUID]] | None = None, ) -> Component: """Create a component of any kind. @@ -185,7 +185,7 @@ def update( config: dict[str, Any] | None = None, encrypted: bool | None = None, children: list[str] | None = None, - relations: dict[str, list[Binding]] | None = None, + relations: dict[str, list[UUID]] | None = None, ) -> Component: """Update a component's spec. ``None`` leaves a facet untouched. @@ -194,7 +194,7 @@ def update( ``state`` column is left alone, with one exception: a job whose config changes has its cached ``next_run_at`` cleared, so the scheduler re-derives the schedule from the new spec on its next tick instead of - firing once more at the slot the old spec produced. + firing once more at the moment the old spec produced. Args: component_id: The component UUID. @@ -240,11 +240,12 @@ def update( def delete(self, component_id: UUID) -> None: """Delete a component. Children and out-bound relations cascade via FK. - In-bound relations follow their declared ``on_delete`` semantics: - consumption relations (a bound connection, a required dependency) - block the deletion; orchestration pointers (a job's ``target``, a - hook's ``watch``, optional dependency slots) detach — the relation - row cascades away and the referrer keeps working with reduced scope. + In-bound relations follow the semantics the referrer's class + declares for the name they are filed under: a consuming relation (a + bound ``connection``, a required upstream) blocks the deletion; one + declared ``on_delete="detach"`` or optional (a job's ``targets``, a + hook's ``watches``) detaches, its row cascading away while the + referrer keeps working with reduced scope. Args: component_id: The component UUID. @@ -274,13 +275,13 @@ def delete(self, component_id: UUID) -> None: def _blocking_referrers(self, session: Session, db_component: Component) -> list[dict[str, str | None]]: """Components outside a component's subtree whose relations into it block deletion. - Deleting a relation destination cascades the binding row, which would - leave a *consuming* referrer silently broken at its next run — those - relations refuse the deletion. Relations whose vocabulary declares - ``on_delete="detach"`` (and optional slots) are skipped: cascading - them is the intended outcome. Relations internal to the subtree (a - source's own asset dependencies) don't count, and a referrer that is - a source-owned asset is reported as its parent source — the unit the + Deleting a relation destination cascades the edge row, which would + leave a *consuming* referrer silently broken at its next run, so + those relations refuse the deletion. An edge whose name the referrer + declares ``on_delete="detach"`` or optional is skipped: cascading it + is the intended outcome. Edges internal to the subtree (a source's + own sibling relations) don't count, and a referrer that is a + source-owned asset is reported as its parent source, the unit the user can act on. Args: @@ -720,10 +721,10 @@ def _ensure_children(self, session: Session, db_source: Component, child_keys: l When ``child_keys`` is provided, only those assets will exist — missing ones are created, extra ones are removed. Removal follows the delete guard's semantics: blocking relations from outside the source - (a required cross-source dependency) raise ``InUseError``; detaching + (a required cross-source upstream) raise ``InUseError``; detaching ones and intra-source edges cascade. ``None`` is the source-creation default and enables every asset the catalog class declares. Existing - rows keep their IDs (and therefore their cross-source deps, event + rows keep their IDs (and therefore their cross-source upstreams, event references, and per-asset overrides). Args: @@ -810,7 +811,7 @@ def job_partition_granularity(self, session: Session, job_id: UUID) -> TimeGranu """ granularities: set[TimeGranularity] = set() targets = session.exec( - select(ComponentRelation).where(ComponentRelation.src_id == job_id, ComponentRelation.type == "target") + select(ComponentRelation).where(ComponentRelation.src_id == job_id, ComponentRelation.name == "targets") ).all() for relation in targets: target = session.get(Component, relation.dst_id) @@ -858,7 +859,9 @@ def _check_job_targets(self, session: Session, db_job: Component) -> None: ComponentDriftError: If a target's catalog key is disabled or missing. """ targets = session.exec( - select(ComponentRelation).where(ComponentRelation.src_id == db_job.id, ComponentRelation.type == "target") + select(ComponentRelation).where( + ComponentRelation.src_id == db_job.id, ComponentRelation.name == "targets" + ) ).all() for relation in targets: target = session.get(Component, relation.dst_id) diff --git a/packages/interloper-db/tests/store/test_components.py b/packages/interloper-db/tests/store/test_components.py index 1a9ab405..819d3cfa 100644 --- a/packages/interloper-db/tests/store/test_components.py +++ b/packages/interloper-db/tests/store/test_components.py @@ -4,7 +4,7 @@ import json from collections.abc import Callable -from typing import Any, ClassVar +from typing import ClassVar from uuid import uuid4 import interloper as il @@ -24,7 +24,7 @@ from sqlalchemy import Engine, create_engine from sqlmodel import Session, select -from interloper_db.models import Component, ComponentRelation +from interloper_db.models import Component from interloper_db.store import Store from interloper_db.store.components import ComponentStore from interloper_db.store.status import ComponentStatus @@ -32,21 +32,141 @@ _ORG = uuid4() +# -- Test components ----------------------------------------------------------- + + +class WireConnection(il.Connection): + """Connection the wire sources bind.""" + + +class PublicToggleConnection(il.Connection): + """Renewable test connection whose ``auto_renew`` is schema-marked public.""" + + api_key: str = il.SecretField() + + def renew(self) -> il.Renewal: + """Keep the class renewable so ``auto_renew`` stays in its schema. + + Returns: + An effectless renewal. + """ + return il.Renewal() + + +class GuardUpstream(il.Asset): + """Upstream asset of the delete-guard tests.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class GuardRequired(il.Asset): + """Asset whose required ``up`` relation blocks its upstream's deletion.""" + + up = il.Relation("asset", "guard_upstream") + + def data(self, context: il.ExecutionContext, up: il.Upstream) -> list[dict]: + return [] + + +class GuardOptional(il.Asset): + """Asset whose optional ``up`` relation detaches when its upstream goes.""" + + up = il.Relation("asset", "guard_upstream", optional=True) + + def data(self, context: il.ExecutionContext, up: il.Upstream | None) -> list[dict]: + return [] + + +class WireUpSource(il.Source): + """Upstream source whose ``totals`` reads its sibling ``rows``.""" + + class Rows(il.Asset): + """Root asset of the source.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + class Totals(il.Asset): + """Asset reading the source's own ``rows``.""" + + rows = il.Relation("asset", "rows") + + def data(self, context: il.ExecutionContext, rows: il.Upstream) -> list[dict]: + return [] + + +class WireDownSource(il.Source): + """Downstream source: a required connection and a required cross-source upstream.""" + + connection: WireConnection + + class Consumer(il.Asset): + """Asset reading ``wire_up_source.rows``.""" + + rows = il.Relation("asset", "wire_up_source.rows") + + def data(self, context: il.ExecutionContext, rows: il.Upstream) -> list[dict]: + return [] + + +class WireDownOptionalSource(il.Source): + """Downstream source whose asset optionally reads ``wire_up_source.rows``.""" + + class Reader(il.Asset): + """Asset with an optional cross-source upstream.""" + + rows = il.Relation("asset", "wire_up_source.rows", optional=True) + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class DiscriminatedSource(il.Source): + """Source class whose instances are discriminated by ``account_id``.""" + + account_id: str = il.InputField(default="", discriminator=True) + + class DiscriminatedRows(il.Asset): + """Asset whose table name carries the instance discriminator.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + @pytest.fixture def store(component_db: Engine) -> Store: - """A store over the in-memory database (no catalog needed for these). + """A store whose catalog carries every class declared in this module. + + Returns: + A store reading and writing the fixture database. + """ + catalog = il.Catalog.from_assets( + [ + GuardUpstream, + GuardRequired, + GuardOptional, + WireUpSource, + WireDownSource, + WireDownOptionalSource, + DiscriminatedSource, + ] + ) + return Store(catalog=catalog) + + +@pytest.fixture +def connection(store: Store) -> Component: + """A plaintext connection row the wire sources accept. Returns: - A store with an empty catalog, reading and writing the fixture database. + The created connection component. """ - return Store(catalog=il.Catalog(components={})) + return store.components.create(_ORG, kind="connection", key="wire_connection", config={}, encrypted=False) -def _relations(session: Session, src_id, type: str | None = None) -> list[ComponentRelation]: - statement = select(ComponentRelation).where(ComponentRelation.src_id == src_id) - if type: - statement = statement.where(ComponentRelation.type == type) - return list(session.exec(statement).all()) +def _child(source: Component, key: str) -> Component: + return next(child for child in source.children if child.key == key) class TestCrud: @@ -68,12 +188,22 @@ def test_children_rejected_for_childless_kinds(self, store: Store): store.components.create(_ORG, kind="destination", key="dest", children=["a"]) def test_unknown_child_keys_rejected(self, component_db: Engine): - from interloper_assets.demo.source import DemoSource - store = Store(catalog=il.Catalog.from_assets([DemoSource])) with pytest.raises(ConfigError, match=r"declares no asset\(s\) \['typo'\]"): store.components.create(_ORG, kind="source", key="demo_source", children=["a", "typo"]) + def test_create_with_relations_by_name(self, store: Store, connection: Component): + destination = store.components.create(_ORG, kind="destination", key="dest") + + source = store.components.create( + _ORG, + kind="source", + key="wire_down_source", + relations={"connection": [connection.id], "destinations": [destination.id]}, + ) + + assert {relation.name for relation in source.out_relations} == {"connection", "destinations"} + def test_delete_refuses_source_owned_assets(self, store: Store, component_db: Engine): job = store.components.create(_ORG, kind="job", key="cron_job") # any parentable stand-in row with Session(component_db) as session: @@ -85,8 +215,6 @@ def test_delete_refuses_source_owned_assets(self, store: Store, component_db: En store.components.delete(child_id) def test_delete_source_removes_child_rows(self, component_db: Engine): - from interloper_assets.demo.source import DemoSource - store = Store(catalog=il.Catalog.from_assets([DemoSource])) source = store.components.create(_ORG, kind="source", key="demo_source") assert source.children @@ -100,8 +228,8 @@ def test_delete_source_removes_child_rows(self, component_db: Engine): def test_delete_cascades_outbound_relations(self, store: Store): job = store.components.create(_ORG, kind="job", key="cron_job", name="J") - asset = store.components.create(_ORG, kind="asset", key="a") - store.relations.add(job.id, type="target", dst_id=asset.id) + asset = store.components.create(_ORG, kind="asset", key="guard_upstream") + store.relations.add(job.id, name="targets", dst_id=asset.id) store.components.delete(job.id) assert store.relations.list_all(_ORG) == [] @@ -123,193 +251,143 @@ def test_get_component_checks_kind(self, store: Store): class TestDeleteInUseGuard: - """A relation destination cannot be deleted while external referrers exist.""" + """A relation destination cannot be deleted while consuming referrers exist.""" - def _connection(self, store: Store) -> Component: - return store.components.create(_ORG, kind="connection", key="conn", name="Conn", config={}, encrypted=False) - - def test_bound_connection_blocks_delete_and_names_referrer(self, store: Store): - conn = self._connection(store) - asset = store.components.create(_ORG, kind="asset", key="a", name="A", relations={"resource": [(conn.id, "c")]}) + def test_bound_connection_blocks_delete_and_names_referrer(self, store: Store, connection: Component): + source = store.components.create( + _ORG, kind="source", key="wire_down_source", name="Down", relations={"connection": [connection.id]} + ) with pytest.raises(InUseError) as excinfo: - store.components.delete(conn.id) - assert excinfo.value.referrers == [{"id": str(asset.id), "kind": "asset", "key": "a", "name": "A"}] - assert "in use by A" in str(excinfo.value) + store.components.delete(connection.id) + assert excinfo.value.referrers == [ + {"id": str(source.id), "kind": "source", "key": "wire_down_source", "name": "Down"} + ] + assert "in use by Down" in str(excinfo.value) + + def test_delete_succeeds_after_repointing(self, store: Store, connection: Component): + other = store.components.create(_ORG, kind="connection", key="wire_connection", config={}, encrypted=False) + source = store.components.create( + _ORG, kind="source", key="wire_down_source", relations={"connection": [connection.id]} + ) - def test_delete_succeeds_after_unbinding(self, store: Store): - conn = self._connection(store) - asset = store.components.create(_ORG, kind="asset", key="a", relations={"resource": [(conn.id, "c")]}) + # A required relation is repointed rather than emptied, which is what + # releases the connection the source used to consume. + store.relations.add(source.id, name="connection", dst_id=other.id) - store.relations.remove(asset.id, type="resource", dst_id=conn.id) - store.components.delete(conn.id) + store.components.delete(connection.id) with pytest.raises(NotFoundError): - store.components.get(conn.id) + store.components.get(connection.id) - def test_job_target_detaches(self, store: Store): - asset = store.components.create(_ORG, kind="asset", key="a") - job = store.components.create( - _ORG, kind="job", key="cron_job", name="J", relations={"target": [(asset.id, "")]} - ) + def test_job_targets_detach(self, store: Store): + asset = store.components.create(_ORG, kind="asset", key="guard_upstream") + job = store.components.create(_ORG, kind="job", key="cron_job", name="J", relations={"targets": [asset.id]}) store.components.delete(asset.id) assert store.components.get(job.id).id == job.id assert store.relations.list_all(_ORG) == [] - def test_hook_watch_detaches(self, store: Store): - asset = store.components.create(_ORG, kind="asset", key="a") - hook = store.components.create( - _ORG, kind="hook", key="webhook", name="H", relations={"watch": [(asset.id, "")]} - ) + def test_hook_watches_detach(self, store: Store): + asset = store.components.create(_ORG, kind="asset", key="guard_upstream") + hook = store.components.create(_ORG, kind="hook", key="webhook", name="H", relations={"watches": [asset.id]}) store.components.delete(asset.id) assert store.components.get(hook.id).id == hook.id assert store.relations.list_all(_ORG) == [] def test_blocking_relation_wins_over_detaching(self, store: Store): - conn = self._connection(store) - asset = store.components.create(_ORG, kind="asset", key="a", name="A", relations={"resource": [(conn.id, "c")]}) - store.components.create(_ORG, kind="job", key="cron_job", relations={"target": [(asset.id, "")]}) + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream", name="Up") + consumer = store.components.create( + _ORG, kind="asset", key="guard_required", name="A", relations={"up": [upstream.id]} + ) + store.components.create(_ORG, kind="job", key="cron_job", relations={"targets": [consumer.id]}) - # The asset both consumes the connection (blocks its deletion) and is - # a job target (detachable) — deleting the asset succeeds, deleting - # the connection does not. + # The consumer both requires the upstream (blocking its deletion) and + # is a job target (detachable), so deleting the consumer succeeds + # while deleting the upstream does not. with pytest.raises(InUseError): - store.components.delete(conn.id) - store.components.delete(asset.id) + store.components.delete(upstream.id) + store.components.delete(consumer.id) - def test_referrer_through_child_reports_parent(self, store: Store, component_db: Engine): - conn = self._connection(store) - parent = store.components.create(_ORG, kind="job", key="cron_job", name="P") # parentable stand-in - with Session(component_db) as session: - child = Component(org_id=_ORG, kind="asset", key="a", parent_id=parent.id) - session.add(child) - session.commit() - child_id = child.id - store.relations.add(child_id, type="resource", dst_id=conn.id, slot="c") + def test_referrer_through_child_reports_parent(self, store: Store): + up = store.components.create(_ORG, kind="source", key="wire_up_source", name="Up") + down = store.components.create(_ORG, kind="source", key="wire_down_source", name="Down") + store.relations.add(_child(down, "consumer").id, name="rows", dst_id=_child(up, "rows").id) with pytest.raises(InUseError) as excinfo: - store.components.delete(conn.id) - assert [r["id"] for r in excinfo.value.referrers] == [str(parent.id)] + store.components.delete(up.id) + assert [r["id"] for r in excinfo.value.referrers] == [str(down.id)] - def test_intra_subtree_relations_do_not_block(self, store: Store, component_db: Engine): - parent = store.components.create(_ORG, kind="job", key="cron_job", name="P") # parentable stand-in - with Session(component_db) as session: - a = Component(org_id=_ORG, kind="asset", key="a", parent_id=parent.id) - b = Component(org_id=_ORG, kind="asset", key="b", parent_id=parent.id) - session.add(a) - session.add(b) - session.commit() - a_id, b_id = a.id, b.id - store.relations.add(b_id, type="upstream", dst_id=a_id, slot="a") + def test_intra_subtree_relations_do_not_block(self, store: Store): + source = store.components.create(_ORG, kind="source", key="wire_up_source") + assert store.relations.list_all(_ORG, name="rows") != [] # the source's own sibling edge - store.components.delete(parent.id) + store.components.delete(source.id) with pytest.raises(NotFoundError): - store.components.get(parent.id) - - -class GuardUpstream(il.Asset): - """Upstream asset for the delete-guard dependency tests.""" - - -class GuardRequired(il.Asset): - """Asset with a required dependency on ``guard_upstream``.""" - - depends_on: ClassVar[dict[str, Any]] = {"up": "guard_upstream"} - - -class GuardOptional(il.Asset): - """Asset with an optional dependency on ``guard_upstream``.""" + store.components.get(source.id) - depends_on: ClassVar[dict[str, Any]] = {"up": il.Dependency(key="guard_upstream", optional=True)} +class TestUpstreamDeleteSemantics: + """A required upstream relation blocks deletion; an optional one detaches.""" -class TestDependencyDeleteSemantics: - """Required dependency slots block deletion; optional slots detach.""" - - @pytest.fixture - def dep_store(self, component_db: Engine) -> Store: - return Store(catalog=il.Catalog.from_assets([GuardUpstream, GuardRequired, GuardOptional])) - - def test_required_dependency_blocks(self, dep_store: Store): - up = dep_store.components.create(_ORG, kind="asset", key="guard_upstream", name="Up") - down = dep_store.components.create( - _ORG, kind="asset", key="guard_required", relations={"upstream": [(up.id, "up")]} + def test_delete_blocks_on_required_referrer_and_detaches_optional(self, store: Store): + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream", name="Up") + required = store.components.create( + _ORG, kind="asset", key="guard_required", name="Req", relations={"up": [upstream.id]} ) - - with pytest.raises(InUseError) as excinfo: - dep_store.components.delete(up.id) - assert [r["id"] for r in excinfo.value.referrers] == [str(down.id)] - - def test_optional_dependency_detaches(self, dep_store: Store): - up = dep_store.components.create(_ORG, kind="asset", key="guard_upstream", name="Up") - down = dep_store.components.create( - _ORG, kind="asset", key="guard_optional", relations={"upstream": [(up.id, "up")]} + optional = store.components.create( + _ORG, kind="asset", key="guard_optional", name="Opt", relations={"up": [upstream.id]} ) - dep_store.components.delete(up.id) - assert dep_store.components.get(down.id).id == down.id - assert dep_store.relations.list_all(_ORG) == [] - - -class WireUpSource(il.Source): - """Upstream source for the cross-source dependency tests.""" - - class Rows(il.Asset): - """Upstream asset (key ``rows``).""" - - -class WireDownSource(il.Source): - """Downstream source whose asset requires ``wire_up_source.rows``.""" - - class Consumer(il.Asset): - """Asset with a required cross-source dependency.""" - - depends_on: ClassVar[dict[str, Any]] = {"rows": "wire_up_source.rows"} - - -class WireDownOptionalSource(il.Source): - """Downstream source whose asset optionally consumes ``wire_up_source.rows``.""" - - class Reader(il.Asset): - """Asset with an optional cross-source dependency.""" - - depends_on: ClassVar[dict[str, Any]] = {"rows": il.Dependency(key="wire_up_source.rows", optional=True)} + with pytest.raises(InUseError) as excinfo: + store.components.delete(upstream.id) + assert [r["id"] for r in excinfo.value.referrers] == [str(required.id)] + store.components.delete(required.id) + store.components.delete(upstream.id) -def _child(source: Component, key: str) -> Component: - return next(child for child in source.children if child.key == key) + assert store.components.get(optional.id).id == optional.id + assert store.relations.list_all(_ORG, name="up") == [] class TestIntraSourceWiring: - """Intra-source dependency edges are derived idempotently over the full child set.""" + """Sibling relation edges are derived idempotently over the full child set.""" @pytest.fixture def demo_store(self, component_db: Engine) -> Store: - from interloper_assets.demo.source import DemoSource + """A store whose catalog carries the demo source's ``a -> b,c,d -> e`` DAG. + Returns: + A store reading and writing the fixture database. + """ return Store(catalog=il.Catalog.from_assets([DemoSource])) + def test_create_source_binds_sibling_upstreams(self, store: Store): + source = store.components.create(_ORG, kind="source", key="wire_up_source") + + rows = store.relations.list_all(_ORG, src_kind="asset", dst_kind="asset") + assert {(row.name, row.dst_id) for row in rows} == {("rows", _child(source, "rows").id)} + def test_full_dag_wired_on_create(self, demo_store: Store): demo_store.components.create(_ORG, kind="source", key="demo_source") - edges = demo_store.relations.list_all(_ORG, type="upstream") + edges = demo_store.relations.list_all(_ORG, src_kind="asset", dst_kind="asset") assert len(edges) == 6 # b,c,d -> a and e -> b,c,d def test_children_enabled_later_get_inbound_edges(self, demo_store: Store): source = demo_store.components.create(_ORG, kind="source", key="demo_source", children=["b", "e"]) - assert [r.slot for r in demo_store.relations.list_all(_ORG, type="upstream")] == ["b"] # only e -> b + assert [row.name for row in demo_store.relations.list_all(_ORG)] == ["b"] # only e -> b updated = demo_store.components.update(source.id, children=["a", "b", "e"]) - edges = demo_store.relations.list_all(_ORG, type="upstream") - by_slot = {r.slot: (r.src_id, r.dst_id) for r in edges} - assert set(by_slot) == {"a", "b"} - assert by_slot["a"] == (_child(updated, "b").id, _child(updated, "a").id) + edges = demo_store.relations.list_all(_ORG) + by_name = {row.name: (row.src_id, row.dst_id) for row in edges} + assert set(by_name) == {"a", "b"} + assert by_name["a"] == (_child(updated, "b").id, _child(updated, "a").id) def test_wiring_is_idempotent(self, demo_store: Store): source = demo_store.components.create(_ORG, kind="source", key="demo_source") demo_store.components.update(source.id, name="renamed") demo_store.components.update(source.id, children=["a", "b", "c", "d", "e"]) - assert len(demo_store.relations.list_all(_ORG, type="upstream")) == 6 + assert len(demo_store.relations.list_all(_ORG)) == 6 def test_update_without_children_leaves_child_set_untouched(self, demo_store: Store): source = demo_store.components.create(_ORG, kind="source", key="demo_source", children=["b", "e"]) @@ -320,50 +398,31 @@ def test_update_without_children_leaves_child_set_untouched(self, demo_store: St class TestChildRemovalGuard: """Narrowing a source's child set honors the delete guard's semantics.""" - @pytest.fixture - def wire_store(self, component_db: Engine) -> Store: - return Store(catalog=il.Catalog.from_assets([WireUpSource, WireDownSource, WireDownOptionalSource])) - - def test_removing_child_with_required_external_dep_blocked(self, wire_store: Store): - up = wire_store.components.create(_ORG, kind="source", key="wire_up_source", name="Up") - down = wire_store.components.create(_ORG, kind="source", key="wire_down_source", name="Down") - wire_store.relations.add( - _child(down, "consumer").id, type="upstream", dst_id=_child(up, "rows").id, slot="rows" - ) + def test_removing_child_with_required_external_upstream_blocked(self, store: Store): + up = store.components.create(_ORG, kind="source", key="wire_up_source", name="Up") + down = store.components.create(_ORG, kind="source", key="wire_down_source", name="Down") + store.relations.add(_child(down, "consumer").id, name="rows", dst_id=_child(up, "rows").id) with pytest.raises(InUseError) as excinfo: - wire_store.components.update(up.id, children=[]) + store.components.update(up.id, children=[]) assert [r["id"] for r in excinfo.value.referrers] == [str(down.id)] - assert wire_store.relations.list_all(_ORG, type="upstream") != [] + assert store.relations.list_all(_ORG, name="rows") != [] - def test_removing_child_with_optional_external_dep_detaches(self, wire_store: Store): - up = wire_store.components.create(_ORG, kind="source", key="wire_up_source") - down = wire_store.components.create(_ORG, kind="source", key="wire_down_optional_source") - wire_store.relations.add( - _child(down, "reader").id, type="upstream", dst_id=_child(up, "rows").id, slot="rows" - ) + def test_removing_child_with_optional_external_upstream_detaches(self, store: Store): + up = store.components.create(_ORG, kind="source", key="wire_up_source") + down = store.components.create(_ORG, kind="source", key="wire_down_optional_source") + store.relations.add(_child(down, "reader").id, name="rows", dst_id=_child(up, "rows").id) - updated = wire_store.components.update(up.id, children=[]) + updated = store.components.update(up.id, children=[]) assert updated.children == [] - assert wire_store.relations.list_all(_ORG, type="upstream") == [] + assert store.relations.list_all(_ORG, name="rows") == [] def test_intra_source_reshape_not_blocked(self, component_db: Engine): - from interloper_assets.demo.source import DemoSource - store = Store(catalog=il.Catalog.from_assets([DemoSource])) source = store.components.create(_ORG, kind="source", key="demo_source") updated = store.components.update(source.id, children=["a"]) assert [child.key for child in updated.children] == ["a"] - assert store.relations.list_all(_ORG, type="upstream") == [] - - -class DiscriminatedSource(il.Source): - """Source class whose instances are discriminated by ``account_id``.""" - - account_id: str = il.InputField(default="", discriminator=True) - - class DiscriminatedRows(il.Asset): - """Asset whose table name carries the instance discriminator.""" + assert store.relations.list_all(_ORG) == [] class TestSourceCollisionGuard: @@ -371,8 +430,11 @@ class TestSourceCollisionGuard: @pytest.fixture def guard_store(self, component_db: Engine) -> Store: - from interloper_assets.demo.source import DemoSource + """A store carrying one discriminated and one undiscriminated source. + Returns: + A store reading and writing the fixture database. + """ return Store(catalog=il.Catalog.from_assets([DemoSource, DiscriminatedSource])) def test_same_alias_rejected(self, guard_store: Store): @@ -422,6 +484,11 @@ class TestDerivedNames: @pytest.fixture def name_store(self, component_db: Engine) -> Store: + """A store carrying the discriminated source, whose name is derivable. + + Returns: + A store reading and writing the fixture database. + """ return Store(catalog=il.Catalog.from_assets([DiscriminatedSource])) def test_blank_name_defaults_to_instance_name(self, name_store: Store): @@ -455,13 +522,32 @@ def test_unresolvable_key_leaves_name_blank(self, store: Store): row = store.components.create(_ORG, kind="destination", key="ghost") assert row.name is None + def test_a_drifted_key_derives_no_name(self, component_db: Engine): + writer = Store(catalog=il.Catalog.from_assets([DemoSource])) + row = writer.components.create(_ORG, kind="source", key="demo_source", config={}) + reader = Store(catalog=il.Catalog(components={})) + + assert reader.components._derived_name(row, row.config) is None + + def test_a_kind_mismatch_derives_no_name(self, component_db: Engine): + # The stored kind and the catalog class disagree, so nothing is derivable. + store = Store(catalog=il.Catalog.from_assets([DemoSource])) + row = store.components.create(_ORG, kind="source", key="demo_source") + row.kind = "destination" + + assert store.components._derived_name(row, {}) is None + + def test_an_unconstructable_config_derives_no_name(self, component_db: Engine): + store = Store(catalog=il.Catalog.from_assets([DemoSource])) + row = store.components.create(_ORG, kind="source", key="demo_source") + + assert store.components._derived_name(row, {"random_failure_probability": "not-a-float"}) is None + class TestTelemetry: """Hydration is traced where it happens.""" def test_load_emits_a_span_per_hydration(self, component_db: Engine, span_exporter): - from interloper_assets.demo.source import DemoSource - store = Store(catalog=il.Catalog.from_assets([DemoSource])) source = store.components.create(_ORG, kind="source", key="demo_source") @@ -479,8 +565,6 @@ class TestQuotaGates: def _store(self, **limits: int | None) -> Store: from types import SimpleNamespace - from interloper_assets.demo.source import DemoSource - return Store(catalog=il.Catalog.from_assets([DemoSource]), quota_defaults=SimpleNamespace(**limits)) def test_source_limit_blocks_creation(self, component_db: Engine): @@ -532,6 +616,11 @@ class TestStatus: @pytest.fixture def demo_store(self, component_db: Engine) -> Store: + """A store carrying the demo source. + + Returns: + A store reading and writing the fixture database. + """ return Store(catalog=il.Catalog.from_assets([DemoSource])) def test_live_source_is_ok(self, demo_store: Store): @@ -720,22 +809,8 @@ def test_update_refreshes_updated_at(self, store: Store): assert updated.updated_at >= updated.created_at -class PublicToggleConnection(il.Connection): - """Renewable test connection whose ``auto_renew`` is schema-marked public.""" - - api_key: str = il.SecretField() - - def renew(self) -> il.Renewal: - """Keep the class renewable so ``auto_renew`` stays in its schema. - - Returns: - An effectless renewal. - """ - return il.Renewal() - - class TestPublicConfig: - """The x-public projection over a secret payload.""" + """The ``x-public`` subset a collection response may disclose.""" def test_public_subset_disclosed_from_encrypted_payload(self, component_db: Engine): catalog = il.Catalog(components={PublicToggleConnection.key: PublicToggleConnection.definition()}) @@ -749,12 +824,25 @@ def test_public_subset_disclosed_from_encrypted_payload(self, component_db: Engi assert store.components.public_config(row) == {"auto_renew": False} - def test_drifted_key_discloses_nothing(self, component_db: Engine): + def test_a_schema_without_public_fields_discloses_nothing(self, component_db: Engine): + store = Store(catalog=il.Catalog.from_assets([DemoSource])) + row = store.components.create(_ORG, kind="source", key="demo_source") + + assert store.components.public_config(row) == {} + + def test_a_drifted_key_discloses_nothing(self, component_db: Engine): store = Store(catalog=il.Catalog(components={}), encrypt=lambda b: b[::-1], decrypt=lambda b: b[::-1]) row = store.components.create(_ORG, kind="connection", key="gone", config={"token": "s3cret"}) assert store.components.public_config(row) == {} + def test_a_drifted_source_key_discloses_nothing(self, component_db: Engine): + writer = Store(catalog=il.Catalog.from_assets([DemoSource])) + row = writer.components.create(_ORG, kind="source", key="demo_source") + reader = Store(catalog=il.Catalog(components={})) + + assert reader.components.public_config(row) == {} + class TestHydrateUnreadable: """Hydrating an unreadable row fails as a decryption error, not as drift.""" @@ -835,9 +923,7 @@ def test_a_drifted_key_raises_component_drift(self, component_db: Engine): def test_an_unreadable_payload_raises_hydration_error(self, component_db: Engine): # A different remedy from drift: the config needs re-entering or re-keying. - catalog = il.Catalog( - components={FacebookAdsConnection.key: FacebookAdsConnection.definition()} - ) + catalog = il.Catalog(components={FacebookAdsConnection.key: FacebookAdsConnection.definition()}) writer = Store(catalog=catalog, encrypt=lambda b: b, decrypt=lambda b: b) row = writer.components.create( _ORG, @@ -867,71 +953,6 @@ def test_an_asset_the_source_no_longer_declares_raises(self, component_db: Engin store.components.load(orphan_id) -class TestPublicConfig: - """The ``x-public`` subset a collection response may disclose.""" - - def test_a_schema_without_public_fields_discloses_nothing(self, component_db: Engine): - store = Store(catalog=il.Catalog.from_assets([DemoSource])) - row = store.components.create(_ORG, kind="source", key="demo_source") - - assert store.components.public_config(row) == {} - - def test_a_drifted_key_discloses_nothing(self, component_db: Engine): - writer = Store(catalog=il.Catalog.from_assets([DemoSource])) - row = writer.components.create(_ORG, kind="source", key="demo_source") - reader = Store(catalog=il.Catalog(components={})) - - assert reader.components.public_config(row) == {} - - -class TestDerivedName: - """The display name derived from a component's own config.""" - - def test_a_drifted_key_derives_no_name(self, component_db: Engine): - writer = Store(catalog=il.Catalog.from_assets([DemoSource])) - row = writer.components.create(_ORG, kind="source", key="demo_source", config={}) - reader = Store(catalog=il.Catalog(components={})) - - assert reader.components._derived_name(row, row.config) is None - - def test_a_kind_mismatch_derives_no_name(self, component_db: Engine): - # The stored kind and the catalog class disagree, so nothing is derivable. - store = Store(catalog=il.Catalog.from_assets([DemoSource])) - row = store.components.create(_ORG, kind="source", key="demo_source") - row.kind = "destination" - - assert store.components._derived_name(row, {}) is None - - def test_an_unconstructable_config_derives_no_name(self, component_db: Engine): - store = Store(catalog=il.Catalog.from_assets([DemoSource])) - row = store.components.create(_ORG, kind="source", key="demo_source") - - assert store.components._derived_name(row, {"random_failure_probability": "not-a-float"}) is None - - -class TestSourceCollision: - """Two instances of one source must not silently share a materialization target.""" - - def test_colliding_datasets_are_refused(self, component_db: Engine): - store = Store(catalog=il.Catalog.from_assets([DemoSource])) - store.components.create(_ORG, kind="source", key="demo_source", config={"dataset": "shared"}) - - with pytest.raises(ConfigError): - store.components.create(_ORG, kind="source", key="demo_source", config={"dataset": "shared"}) - - def test_distinct_datasets_are_allowed(self, component_db: Engine): - store = Store(catalog=il.Catalog.from_assets([DemoSource])) - store.components.create(_ORG, kind="source", key="demo_source", config={"dataset": "one"}) - - store.components.create(_ORG, kind="source", key="demo_source", config={"dataset": "two"}) - - def test_another_org_is_not_a_collision(self, component_db: Engine): - store = Store(catalog=il.Catalog.from_assets([DemoSource])) - store.components.create(_ORG, kind="source", key="demo_source", config={"dataset": "shared"}) - - store.components.create(uuid4(), kind="source", key="demo_source", config={"dataset": "shared"}) - - class TestEnsureChildrenDrift: """A source whose key no longer resolves cannot have its children reconciled.""" @@ -957,9 +978,7 @@ def test_a_job_with_no_targets_has_none(self, component_db: Engine): def test_a_partitioned_source_target_reports_its_granularity(self, component_db: Engine): store = Store(catalog=il.Catalog.from_assets([DemoSource])) source = store.components.create(_ORG, kind="source", key="demo_source") - job = store.components.create( - _ORG, kind="job", key="cron_job", relations={"target": [(source.id, "")]} - ) + job = store.components.create(_ORG, kind="job", key="cron_job", relations={"targets": [source.id]}) with Session(component_db) as session: assert store.components.job_partition_granularity(session, job.id) is TimeGranularity.DAY @@ -970,9 +989,7 @@ def test_a_partitioned_asset_target_reports_its_granularity(self, component_db: with Session(component_db) as session: asset = session.exec(select(Component).where(Component.parent_id == source.id)).one() asset_id = asset.id - job = store.components.create( - _ORG, kind="job", key="cron_job", relations={"target": [(asset_id, "")]} - ) + job = store.components.create(_ORG, kind="job", key="cron_job", relations={"targets": [asset_id]}) with Session(component_db) as session: assert store.components.job_partition_granularity(session, job.id) is TimeGranularity.DAY @@ -983,10 +1000,7 @@ def test_targets_disagreeing_on_granularity_fail_closed(self, component_db: Engi daily = store.components.create(_ORG, kind="source", key="demo_source") monthly = store.components.create(_ORG, kind="source", key="demo_monthly_source") job = store.components.create( - _ORG, - kind="job", - key="cron_job", - relations={"target": [(daily.id, ""), (monthly.id, "")]}, + _ORG, kind="job", key="cron_job", relations={"targets": [daily.id, monthly.id]} ) with Session(component_db) as session, pytest.raises( @@ -998,9 +1012,7 @@ def test_a_drifted_target_contributes_nothing(self, component_db: Engine): # Drift is the run path's problem, not the scheduler's. writer = Store(catalog=il.Catalog.from_assets([DemoSource])) source = writer.components.create(_ORG, kind="source", key="demo_source") - job = writer.components.create( - _ORG, kind="job", key="cron_job", relations={"target": [(source.id, "")]} - ) + job = writer.components.create(_ORG, kind="job", key="cron_job", relations={"targets": [source.id]}) reader = Store(catalog=il.Catalog(components={})) with Session(component_db) as session: @@ -1013,9 +1025,7 @@ class TestCheckJobTargets: def test_a_drifted_target_raises(self, component_db: Engine): writer = Store(catalog=il.Catalog.from_assets([DemoSource])) source = writer.components.create(_ORG, kind="source", key="demo_source") - job = writer.components.create( - _ORG, kind="job", key="cron_job", relations={"target": [(source.id, "")]} - ) + job = writer.components.create(_ORG, kind="job", key="cron_job", relations={"targets": [source.id]}) reader = Store(catalog=il.Catalog(components={})) with Session(component_db) as session: diff --git a/packages/interloper-db/tests/store/test_organisations.py b/packages/interloper-db/tests/store/test_organisations.py index 0c50339e..debd80e4 100644 --- a/packages/interloper-db/tests/store/test_organisations.py +++ b/packages/interloper-db/tests/store/test_organisations.py @@ -34,7 +34,7 @@ def _seed_org_data(self, session: SQLSession, org_id) -> None: session.add(asset) session.add( ComponentRelation( - src_id=asset.id, dst_id=source.id, org_id=org_id, src_kind="asset", dst_kind="source", type="owner" + src_id=asset.id, dst_id=source.id, org_id=org_id, src_kind="asset", dst_kind="source", name="owner" ) ) backfill = Backfill(org_id=org_id, start_key="2026-01-01", end_key="2026-01-02") From 0fa57d8ca7c4075c44b9f8941c9af5d3ff2c2fac Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 02:52:10 +0200 Subject: [PATCH 06/19] feat(db): hydrate relations by name; parented targets become references resolved through the store By Digitl --- .../src/interloper_db/store/components.py | 120 +++++- .../src/interloper_db/store/hydration.py | 127 ++++-- .../tests/store/test_hydration.py | 396 ++++++++++++++++-- 3 files changed, 533 insertions(+), 110 deletions(-) diff --git a/packages/interloper-db/src/interloper_db/store/components.py b/packages/interloper-db/src/interloper_db/store/components.py index cfe89505..f0633555 100644 --- a/packages/interloper-db/src/interloper_db/store/components.py +++ b/packages/interloper-db/src/interloper_db/store/components.py @@ -19,6 +19,7 @@ class declares between its own assets. from __future__ import annotations +import functools import json from typing import Any, cast from uuid import UUID @@ -339,7 +340,11 @@ def load(self, component_id: UUID) -> il.Component: """Hydrate a framework component of any kind from its row. Source-owned assets hydrate through their parent source and are - extracted from it; jobs drift-check every target first. Fails closed + extracted from it; jobs drift-check every target first. One cache + backs the whole call (see :meth:`_load`), so a component reached + several times within the same document (a job's own target and, + through a cross-source upstream, that same target again) hydrates + once and every consumer binds the identical instance. Fails closed on any catalog drift: see :meth:`_load` for what a missing row, a drifted key or a failed reconstruction raises. @@ -352,13 +357,34 @@ def load(self, component_id: UUID) -> il.Component: with tracer().start_as_current_span( "interloper.store.load", attributes={attributes.TARGET_ID: str(component_id)} ): - return self._load(component_id) + return self._load(component_id, {}) - def _load(self, component_id: UUID) -> il.Component: + def _load( + self, + component_id: UUID, + cache: dict[UUID, il.Component], + chain: tuple[tuple[UUID, str], ...] = (), + ) -> il.Component: """Hydrate a component row (the traced body of :meth:`load`). + A spec reference (``{"ref": id}``) is resolved from the document + first, which reconstruction does on its own, and only then through + :meth:`_resolve_reference`, which re-enters here with the same + *cache* and *chain* rather than starting a fresh call: a job that + targets both a source and a consumer of that source's asset holds + one instance of it, not two, and an owned asset's parent (loaded + through :meth:`_load_owned_asset`) shares the cache the same way. + *chain* is the trail of ids currently being hydrated, paired with + the catalog key each was hydrated under; a reference back onto one + of them is a cycle, reported from the trail rather than left to + exhaust the stack. + Args: component_id: The component UUID. + cache: Components already hydrated within this :meth:`load` call, + by id, consulted before doing any work. + chain: Ids currently being hydrated in this call, in resolution + order. Defaults to ``()``, the top-level call's empty trail. Returns: The reconstructed framework component. @@ -366,9 +392,17 @@ def _load(self, component_id: UUID) -> il.Component: Raises: NotFoundError: If the component is not found. ComponentDriftError: If a catalog key no longer resolves. - HydrationError: If the stored payload does not decrypt, or if - reconstruction fails. + HydrationError: If the stored payload does not decrypt, if + reconstruction fails, or if a reference revisits an id + already being hydrated earlier in the same call. """ + if component_id in cache: + return cache[component_id] + cyclic_key = next((key for id_, key in chain if id_ == component_id), None) + if cyclic_key is not None: + trail = " -> ".join(key for _, key in chain) + raise HydrationError(f"Reference cycle while hydrating: {trail} -> {cyclic_key}") + with session_scope(self._engine) as session: db_component = session.get(Component, component_id) if not db_component: @@ -394,28 +428,74 @@ def _load(self, component_id: UUID) -> il.Component: # Reconstruction happens outside the session: it imports classes and, # for owned assets, recursively loads the parent source. + chain = (*chain, (component_id, db_component.key)) if owned_asset: - return self._load_owned_asset(db_component.parent_id, db_component.key, component_id) - try: - return il.Component.from_spec(spec) - except Exception as e: - # format_exception, never str(e): a ValidationError here carries the - # decrypted payload of sensitive kinds in its input_value dumps, and - # this message is persisted into run events and shown in the UI. - raise HydrationError( - f"Failed to hydrate {db_component.kind} '{db_component.key}' ({db_component.id}): {format_exception(e)}" - ) from e - - def _load_owned_asset(self, parent_id: UUID, key: str, asset_id: UUID) -> il.Asset: + component = self._load_owned_asset(db_component.parent_id, db_component.key, component_id, cache, chain) + else: + resolve = functools.partial(self._resolve_reference, cache=cache, chain=chain) + try: + component = il.Component.from_spec(spec, resolve=resolve) + except (ComponentDriftError, NotFoundError): + # A dedicated handler (the API's drift endpoint, say) needs to + # tell these apart from a generic hydration failure, so they + # pass through untouched rather than folding into the catch-all. + raise + except Exception as e: + # format_exception, never str(e): a ValidationError here carries the + # decrypted payload of sensitive kinds in its input_value dumps, and + # this message is persisted into run events and shown in the UI. + raise HydrationError( + f"Failed to hydrate {db_component.kind} '{db_component.key}' ({db_component.id}): " + f"{format_exception(e)}" + ) from e + cache[component_id] = component + return component + + def _resolve_reference( + self, + reference: str, + *, + cache: dict[UUID, il.Component], + chain: tuple[tuple[UUID, str], ...], + ) -> il.Component: + """Hydrate the component a spec reference names from outside its document. + + Args: + reference: Id of the referenced component, as the spec carries it. + cache: Components already hydrated within the enclosing + :meth:`load` call, consulted (and extended) instead of + hydrating a fresh instance for a component reached again. + chain: Ids currently being hydrated in the enclosing call, for + :meth:`_load`'s cycle check. + + Returns: + The referenced component, hydrated through the store. + """ + return self._load(UUID(reference), cache, chain) + + def _load_owned_asset( + self, + parent_id: UUID, + key: str, + asset_id: UUID, + cache: dict[UUID, il.Component], + chain: tuple[tuple[UUID, str], ...], + ) -> il.Asset: """Hydrate a source-owned asset through its parent source. - The parent source is the unit of reconstruction — loading it binds - all its assets — and the child is picked out by key. + The parent source is the unit of reconstruction: loading it binds all + its assets, and the child is picked out by key. The parent loads + through the same *cache* and *chain* as the asset itself, so a + source hydrates once even when several of its owned assets are each + reached independently within one :meth:`load` call. Args: parent_id: UUID of the owning source component. key: Catalog key of the asset to pick out of the source. asset_id: UUID of the asset row, for the drift error message. + cache: Components already hydrated within the enclosing + :meth:`load` call. + chain: Ids currently being hydrated in the enclosing call. Returns: The bound asset instance. @@ -423,7 +503,7 @@ def _load_owned_asset(self, parent_id: UUID, key: str, asset_id: UUID) -> il.Ass Raises: ComponentDriftError: If the source no longer declares the key. """ - source = cast(il.Source, self.load(parent_id)) + source = cast(il.Source, self._load(parent_id, cache, chain)) for asset in source.assets: if asset.key == key: return asset diff --git a/packages/interloper-db/src/interloper_db/store/hydration.py b/packages/interloper-db/src/interloper_db/store/hydration.py index 1c0ff0a0..5abcf4d4 100644 --- a/packages/interloper-db/src/interloper_db/store/hydration.py +++ b/packages/interloper-db/src/interloper_db/store/hydration.py @@ -13,10 +13,17 @@ One builder covers every kind: a component's init is its ``config`` (or its decrypted ``data`` for secret-bearing kinds) plus whatever its outgoing -relations and children contribute. Relations are mapped through the row's -own vocabulary (the catalog class's definition, anchor as drift fallback), -so the walk needs no kind dispatch: an asset simply has no ``target`` -relations, a destination no ``upstream`` ones. +relations and children contribute. Relations are read by name and checked +against the row's own vocabulary (the catalog class's declaration, the +kind's anchor as drift fallback), so the walk needs no kind dispatch: an +asset simply holds no ``targets`` edges, a destination no upstream ones. + +A target is written out by the rule ``Component.to_spec`` follows: one +owned by a source travels inside that source's own spec, so it is always a +``{"ref": id}``, and a parentless one is written out in full the first time +the walk reaches it and referenced afterwards. Resolving a reference the +document does not carry is the caller's job (see +:meth:`~interloper_db.store.components.ComponentStore._load`). The Store wraps this pattern in thin ``load_*`` convenience methods, but any caller can use the hydrator directly to assemble a spec (for example, @@ -63,18 +70,28 @@ def __init__( self._catalog = catalog self._decrypt = decrypt - def build_component_spec(self, session: Session, db_component: Component) -> Spec: + def build_component_spec( + self, + session: Session, + db_component: Component, + *, + seen: set[str] | None = None, + ) -> Spec: """Build a spec for a component row of any kind. Args: session: Active DB session (used to walk relations and children). db_component: The component row. + seen: Ids the walk has already written out in full, extended with + this row's own. One set is shared by every spec of a document, + which is what turns a repeated target into a reference. + Defaults to ``None``, which starts a document of this row alone. Returns: A ``Spec`` with the row's ``id`` and a fully resolved init payload (nested components as nested specs). """ - init = self._build_init(session, db_component) + init = self._build_init(session, db_component, seen=seen) return Spec( path=self._resolve_path(session, db_component), id=str(db_component.id) if db_component.id else "", @@ -116,68 +133,85 @@ def decode_data(self, db_component: Component) -> dict[str, Any]: # -- Internals ------------------------------------------------------------- - def _build_init(self, session: Session, db_component: Component) -> dict[str, Any]: + def _build_init( + self, + session: Session, + db_component: Component, + *, + seen: set[str] | None = None, + ) -> dict[str, Any]: """Build the init payload for a component row. - Relations are mapped through the kind's own vocabulary: each type - fills its declared ``field``, shaped by its definition — slotted - types as ``{slot: value}``, unslotted ones as lists, carrying - nested specs (``inline``) or bare instance ids. - Children are the one non-relation contribution: they embed as the - ``assets`` override map, since the parent source is the unit of - reconstruction. + Each relation name the row holds edges under sits in the payload + under that name, a list when the declared relation is ``many`` and a + single value otherwise. Children are the one non-relation + contribution: they embed as the ``assets`` override map, since the + parent source is the unit of reconstruction. Args: session: Active DB session, used to read relations and children. db_component: The component row whose init payload is built. + seen: Ids the walk has already written out in full, extended with + this row's own before anything else runs (so a cycle back + onto it is caught the same way whether this is the top of the + walk or a nested call). Defaults to ``None``, which starts a + document of this row alone. Returns: A dict suitable for use as a ``Spec.init``. Raises: - HydrationError: If the row carries a relation type its kind's - vocabulary does not declare. + HydrationError: If the row holds edges under a relation name its + class does not declare, or holds more than one row under a + relation its class declares single-valued. """ + seen = set() if seen is None else seen + seen.add(str(db_component.id) if db_component.id else "") if KINDS[db_component.kind].sensitive: init = self.decode_data(db_component) else: init = dict(db_component.config or {}) - vocabulary = self._catalog.vocabulary(db_component.kind, db_component.key) - for relation_type, rels in self._relations_by_type(session, db_component.id).items(): - definition = vocabulary.get(relation_type) - if definition is None: + vocabulary = self._catalog.vocabulary( + db_component.kind, db_component.key, parent_key=db_component.parent_key(session) + ) + for name, rows in self._relations_by_name(session, db_component.id).items(): + relation = vocabulary.get(name) + if relation is None: + raise HydrationError( + f"Component {db_component.id} ({db_component.kind}) has '{name}' relations " + "its class does not declare" + ) + values = [self._dst_value(session, row, seen) for row in rows] + if relation.many: + init[name] = values + elif len(values) > 1: raise HydrationError( - f"Component {db_component.id} ({db_component.kind}) has '{relation_type}' relations, " - "which its kind's vocabulary does not declare" + f"Component {db_component.id} ({db_component.kind}) holds {len(values)} rows under " + f"single-valued relation '{name}'" ) - if definition.inline: - values = [self._dst_spec(session, rel).model_dump(mode="json") for rel in rels] else: - values = [str(rel.dst_id) for rel in rels] - init[definition.field] = ( - {rel.slot: value for rel, value in zip(rels, values)} if definition.slotted else values - ) + init[name] = values[0] children = session.exec( select(Component).where(Component.parent_id == db_component.id).order_by(Component.created_at) # ty: ignore[invalid-argument-type] ).all() if assets := { - child.key: {"id": str(child.id), **self._build_init(session, child)} for child in children + child.key: {"id": str(child.id), **self._build_init(session, child, seen=seen)} for child in children }: init["assets"] = assets return init - def _relations_by_type(self, session: Session, src_id: UUID | None) -> dict[str, list[ComponentRelation]]: - """Group a component's outgoing relations by type, ordered stably. + def _relations_by_name(self, session: Session, src_id: UUID | None) -> dict[str, list[ComponentRelation]]: + """Group a component's outgoing relations by name, ordered stably. Args: session: Active DB session used to read the relation rows. src_id: The source component's id, or ``None`` for an unflushed row. Returns: - A ``{type: relations}`` mapping ordered by ``(slot, dst_id)``, or + A ``{name: relations}`` mapping ordered by ``(name, dst_id)``, or ``{}`` when ``src_id`` is ``None``. """ if src_id is None: @@ -185,31 +219,36 @@ def _relations_by_type(self, session: Session, src_id: UUID | None) -> dict[str, rows = session.exec( select(ComponentRelation) .where(ComponentRelation.src_id == src_id) - .order_by(ComponentRelation.slot, ComponentRelation.dst_id) # ty: ignore[invalid-argument-type] + .order_by(ComponentRelation.name, ComponentRelation.dst_id) # ty: ignore[invalid-argument-type] ).all() grouped: dict[str, list[ComponentRelation]] = {} - for rel in rows: - grouped.setdefault(rel.type, []).append(rel) + for row in rows: + grouped.setdefault(row.name, []).append(row) return grouped - def _dst_spec(self, session: Session, rel: ComponentRelation) -> Spec: - """Build the spec of a relation's destination component. + def _dst_value(self, session: Session, row: ComponentRelation, seen: set[str]) -> dict[str, Any]: + """Write out one edge's destination, in full or as a reference. Args: session: Active DB session used to load the destination row. - rel: The relation whose ``dst_id`` is resolved. + row: The edge whose ``dst_id`` is written out. + seen: Ids the walk has already written out in full, extended with + the destination's own when it is written out here. Returns: - The destination component's ``Spec``. + The destination's own spec as a JSON-able mapping, or the + ``{"ref": id}`` reference standing in for it. Raises: - HydrationError: If the relation points at a component row that - does not exist. + HydrationError: If the edge points at a component row that does + not exist. """ - db_dst = session.get(Component, rel.dst_id) + db_dst = session.get(Component, row.dst_id) if db_dst is None: # defensive: FKs make this unreachable - raise HydrationError(f"Relation {rel.src_id} -[{rel.type}]-> {rel.dst_id} points at a missing component") - return self.build_component_spec(session, db_dst) + raise HydrationError(f"Relation {row.src_id} -[{row.name}]-> {row.dst_id} points at a missing component") + if db_dst.parent_id is not None or str(db_dst.id) in seen: + return Spec.reference(str(db_dst.id)) + return self.build_component_spec(session, db_dst, seen=seen).model_dump(mode="json") def _resolve_path(self, session: Session, db_component: Component) -> str: """Look up a component's import path via the catalog. diff --git a/packages/interloper-db/tests/store/test_hydration.py b/packages/interloper-db/tests/store/test_hydration.py index dbfbf6cb..3b941f3b 100644 --- a/packages/interloper-db/tests/store/test_hydration.py +++ b/packages/interloper-db/tests/store/test_hydration.py @@ -1,39 +1,255 @@ """Round-trip tests: generic store writes → generic hydration → live framework objects. -These exercise the full pipeline against a real (SQLite) database using the -real ``DemoSource`` catalog component: create rows through the generic store -surface, hydrate through the one generic spec builder, and assert on the -reconstructed framework instances. +These exercise the full pipeline against a real (SQLite) database using real +catalog classes: create rows through the generic store surface, hydrate +through the one generic spec builder, and assert on the reconstructed +framework instances. """ from __future__ import annotations -from typing import ClassVar -from uuid import uuid4 +from typing import Any +from uuid import UUID, uuid4 import interloper as il import pydantic import pytest -from interloper.errors import HydrationError +from interloper.errors import ComponentDriftError, HydrationError +from interloper.serializable import Spec from interloper_assets.demo.source import DemoSource, demo_asset from sqlalchemy import Engine +from sqlmodel import Session +from interloper_db.models import Component, ComponentRelation from interloper_db.store import Store +from interloper_db.store.hydration import Hydrator _ORG = uuid4() -_CATALOG = il.Catalog.from_assets([DemoSource, demo_asset]) + + +class ShopConnection(il.Connection): + """Connection the shop source binds.""" + + +class Warehouse(il.Destination): + """Destination the test sources write to.""" + + def read(self, context: Any) -> Any: # pragma: no cover + """Never read by these tests. + + Args: + context: The IO context of the read. + + Returns: + Nothing; the destination is a persistence fixture only. + """ + return None + + def write(self, context: Any, data: Any) -> None: # pragma: no cover + """Never written by these tests. + + Args: + context: The IO context of the write. + data: The payload that would be written. + """ + + +class Shop(il.Source): + """Upstream source: a bound connection and one asset other sources read.""" + + connection: ShopConnection + + class Orders(il.Asset): + """The asset the finance source reads by qualified key.""" + + back = il.Relation("asset", "finance.revenue", optional=True) + + def data(self, context: il.ExecutionContext) -> list[dict]: # pragma: no cover + """Never materialized by these tests. + + Args: + context: The execution context of the materialization. + + Returns: + No rows. + """ + return [] + + +class Finance(il.Source): + """Downstream source whose assets both read ``shop.orders``.""" + + class Revenue(il.Asset): + """Asset with a cross-source upstream.""" + + orders = il.Relation("asset", "shop.orders") + + def data(self, context: il.ExecutionContext, orders: il.Upstream) -> list[dict]: # pragma: no cover + """Never materialized by these tests. + + Args: + context: The execution context of the materialization. + orders: The upstream leg read from ``shop.orders``. + + Returns: + No rows. + """ + return [] + + class Cogs(il.Asset): + """A second asset reading the same cross-source upstream as ``Revenue``.""" + + orders = il.Relation("asset", "shop.orders") + + def data(self, context: il.ExecutionContext, orders: il.Upstream) -> list[dict]: # pragma: no cover + """Never materialized by these tests. + + Args: + context: The execution context of the materialization. + orders: The upstream leg read from ``shop.orders``. + + Returns: + No rows. + """ + return [] + + +_CATALOG = il.Catalog.from_assets([DemoSource, demo_asset, Shop, Finance, Warehouse]) @pytest.fixture def store(component_db: Engine) -> Store: - """A store over the in-memory database with the demo catalog. + """A store over the in-memory database with the tests' catalog. Returns: - A store carrying the demo catalog, reading and writing the fixture database. + A store carrying the demo and shop/finance classes, reading and + writing the fixture database. """ return Store(catalog=_CATALOG) +def _child(source: Component, key: str) -> Component: + """Pick one child row of a source row by asset key. + + Args: + source: The source row whose children are searched. + key: The child asset's catalog key. + + Returns: + The matching child row. + """ + return next(child for child in source.children if child.key == key) + + +def _asset(source: il.Component, key: str) -> il.Asset: + """Pick one asset instance of a hydrated source by key. + + Args: + source: The hydrated source. + key: The asset's catalog key. + + Returns: + The matching asset instance. + """ + assert isinstance(source, il.Source) + return next(asset for asset in source.assets if asset.key == key) + + +def _init(store: Store, component_id: UUID) -> dict[str, Any]: + """Build the init payload of one row, in a session of its own. + + Args: + store: The store whose hydrator builds the payload. + component_id: Id of the row to build. + + Returns: + The row's init payload. + """ + with Session(store.engine) as session: + row = session.get(Component, component_id) + assert row is not None + return store.components._hydrator._build_init(session, row) + + +class TestBuildInit: + """The one rule that decides how a relation target is written out.""" + + def test_parentless_target_nests(self, store: Store): + connection = store.components.create(_ORG, kind="connection", key="shop_connection", config={}, encrypted=False) + warehouse = store.components.create(_ORG, kind="destination", key="warehouse") + shop = store.components.create( + _ORG, + kind="source", + key="shop", + relations={"connection": [connection.id], "destinations": [warehouse.id]}, + ) + + init = _init(store, shop.id) + + assert init["connection"]["id"] == str(connection.id) + assert init["connection"]["path"].endswith("ShopConnection") + assert [target["id"] for target in init["destinations"]] == [str(warehouse.id)] + + def test_asset_target_is_a_reference(self, store: Store): + shop = store.components.create(_ORG, kind="source", key="shop") + finance = store.components.create(_ORG, kind="source", key="finance") + store.relations.add(_child(finance, "revenue").id, name="orders", dst_id=_child(shop, "orders").id) + + init = _init(store, finance.id) + + assert init["assets"]["revenue"]["orders"] == Spec.reference(str(_child(shop, "orders").id)) + + def test_a_source_owned_asset_carries_its_sibling_relations(self, store: Store): + source = store.components.create(_ORG, kind="source", key="demo_source") + + init = _init(store, source.id) + + assert init["assets"]["e"]["b"] == Spec.reference(str(_child(source, "b").id)) + assert init["assets"]["b"]["a"] == Spec.reference(str(_child(source, "a").id)) + + def test_a_target_reached_twice_is_written_out_once_then_referenced(self, store: Store): + warehouse = store.components.create(_ORG, kind="destination", key="warehouse") + shop = store.components.create(_ORG, kind="source", key="shop", relations={"destinations": [warehouse.id]}) + finance = store.components.create( + _ORG, kind="source", key="finance", relations={"destinations": [warehouse.id]} + ) + job = store.components.create( + _ORG, + kind="job", + key="cron_job", + config={"cron": "0 6 * * *"}, + relations={"targets": [shop.id, finance.id]}, + ) + + init = _init(store, job.id) + + emitted = [target["init"]["destinations"][0] for target in init["targets"]] + assert [Spec.is_reference(value) for value in emitted] == [False, True] + assert emitted[0]["id"] == str(warehouse.id) + assert emitted[1] == Spec.reference(str(warehouse.id)) + + def test_a_relation_name_the_class_does_not_declare_is_an_actionable_error( + self, store: Store, component_db: Engine + ): + shop = store.components.create(_ORG, kind="source", key="shop") + warehouse = store.components.create(_ORG, kind="destination", key="warehouse") + with Session(component_db) as session: + session.add( + ComponentRelation( + src_id=shop.id, + name="mystery", + dst_id=warehouse.id, + org_id=_ORG, + src_kind="source", + dst_kind="destination", + ) + ) + session.commit() + + with pytest.raises(HydrationError, match="has 'mystery' relations its class does not declare"): + _init(store, shop.id) + + class TestSourceRoundTrip: """Sources with child assets, intra-source deps, and overrides.""" @@ -42,11 +258,13 @@ def test_create_source_creates_children_and_intra_deps(self, store: Store): assert db_source.kind == "source" assert sorted(child.key for child in db_source.children) == ["a", "b", "c", "d", "e"] - deps = store.relations.list_all(_ORG, type="upstream") - by_child = {} + deps = store.relations.list_all(_ORG, src_kind="asset", dst_kind="asset") + by_child: dict[str, set[tuple[str, str]]] = {} children_by_id = {child.id: child.key for child in db_source.children} - for rel in deps: - by_child.setdefault(children_by_id[rel.src_id], set()).add((rel.slot, children_by_id[rel.dst_id])) + for relation in deps: + by_child.setdefault(children_by_id[relation.src_id], set()).add( + (relation.name, children_by_id[relation.dst_id]) + ) assert by_child == { "b": {("a", "a")}, "c": {("a", "a")}, @@ -65,9 +283,9 @@ def test_load_hydrates_with_stable_ids_and_deps(self, store: Store): assert source.hello == "there" rows_by_key = {child.key: str(child.id) for child in db_source.children} - assets_by_key = {type(asset).key: asset for asset in source.assets} + assets_by_key = {asset.key: asset for asset in source.assets} assert {key: asset.id for key, asset in assets_by_key.items()} == rows_by_key - assert assets_by_key["e"].upstreams == { + assert assets_by_key["e"].bound_ids() == { "b": [rows_by_key["b"]], "c": [rows_by_key["c"]], "d": [rows_by_key["d"]], @@ -75,12 +293,12 @@ def test_load_hydrates_with_stable_ids_and_deps(self, store: Store): def test_source_owned_asset_loads_through_its_parent(self, store: Store): db_source = store.components.create(_ORG, kind="source", key="demo_source", name="Demo") - child = next(child for child in db_source.children if child.key == "a") + child = _child(db_source, "a") store.components.update(child.id, config={"materializable": False}) asset = store.components.load(child.id) assert isinstance(asset, il.Asset) - assert type(asset).key == "a" + assert asset.key == "a" assert asset.materializable is False def test_children_selection_drops_rows_and_relations(self, store: Store): @@ -90,8 +308,86 @@ def test_children_selection_drops_rows_and_relations(self, store: Store): refreshed = store.components.get(db_source.id, kind="source") assert sorted(child.key for child in refreshed.children) == ["a", "b"] # e (and its dependency relations) are gone; b keeps its dep on a. - remaining = store.relations.list_all(_ORG, type="upstream") - assert [rel.slot for rel in remaining] == ["a"] + remaining = store.relations.list_all(_ORG, src_kind="asset", dst_kind="asset") + assert [relation.name for relation in remaining] == ["a"] + + +class TestCrossSourceUpstream: + """A reference no document carries is resolved through the store.""" + + def test_load_resolves_cross_source_upstream_through_store(self, store: Store): + shop = store.components.create(_ORG, kind="source", key="shop") + finance = store.components.create(_ORG, kind="source", key="finance") + shop_orders = _child(shop, "orders") + store.relations.add(_child(finance, "revenue").id, name="orders", dst_id=shop_orders.id) + + hydrated = store.components.load(finance.id) + + orders = _asset(hydrated, "revenue").bound("orders") + assert isinstance(orders, il.Asset) + assert orders.id == str(shop_orders.id) + assert orders.parent is not None + assert orders.parent.key == "shop" + + def test_an_owned_asset_referencing_another_source_loads(self, store: Store): + shop = store.components.create(_ORG, kind="source", key="shop") + finance = store.components.create(_ORG, kind="source", key="finance") + shop_orders = _child(shop, "orders") + revenue_row = _child(finance, "revenue") + store.relations.add(revenue_row.id, name="orders", dst_id=shop_orders.id) + + revenue = store.components.load(revenue_row.id) + + assert isinstance(revenue, il.Asset) + assert revenue.bound_ids() == {"orders": [str(shop_orders.id)]} + + def test_a_shared_upstream_reached_twice_hydrates_once(self, store: Store, monkeypatch: pytest.MonkeyPatch): + shop = store.components.create(_ORG, kind="source", key="shop") + finance = store.components.create(_ORG, kind="source", key="finance") + shop_orders = _child(shop, "orders") + store.relations.add(_child(finance, "revenue").id, name="orders", dst_id=shop_orders.id) + store.relations.add(_child(finance, "cogs").id, name="orders", dst_id=shop_orders.id) + + original_build = Hydrator.build_component_spec + shop_hydrations: list[UUID] = [] + + def spy(self: Hydrator, session: Session, db_component: Component, *, seen: set[str] | None = None) -> Spec: + if db_component.key == "shop": + shop_hydrations.append(db_component.id) + return original_build(self, session, db_component, seen=seen) + + monkeypatch.setattr(Hydrator, "build_component_spec", spy) + + hydrated = store.components.load(finance.id) + + revenue_orders = _asset(hydrated, "revenue").bound("orders") + cogs_orders = _asset(hydrated, "cogs").bound("orders") + assert isinstance(revenue_orders, il.Asset) + assert isinstance(cogs_orders, il.Asset) + assert revenue_orders is cogs_orders + assert revenue_orders.parent is cogs_orders.parent + assert shop_hydrations == [shop.id] + + def test_a_reference_cycle_across_sources_raises_with_its_trail(self, store: Store): + shop = store.components.create(_ORG, kind="source", key="shop") + finance = store.components.create(_ORG, kind="source", key="finance") + shop_orders = _child(shop, "orders") + revenue = _child(finance, "revenue") + store.relations.add(revenue.id, name="orders", dst_id=shop_orders.id) + store.relations.add(shop_orders.id, name="back", dst_id=revenue.id) + + with pytest.raises(HydrationError, match="Reference cycle while hydrating: shop"): + store.components.load(shop.id) + + def test_a_reference_to_a_drifted_component_raises_component_drift_error(self, store: Store): + shop = store.components.create(_ORG, kind="source", key="shop") + finance = store.components.create(_ORG, kind="source", key="finance") + store.relations.add(_child(finance, "revenue").id, name="orders", dst_id=_child(shop, "orders").id) + + reader = Store(catalog=il.Catalog.from_assets([DemoSource, demo_asset, Finance, Warehouse])) + + with pytest.raises(ComponentDriftError, match="shop"): + reader.components.load(finance.id) class TestStandaloneAsset: @@ -101,7 +397,7 @@ def test_create_and_load(self, store: Store): db_asset = store.components.create(_ORG, kind="asset", key="demo_asset", config={"materializable": False}) asset = store.components.load(db_asset.id) assert isinstance(asset, il.Asset) - assert type(asset).key == "demo_asset" + assert asset.key == "demo_asset" assert asset.id == str(db_asset.id) assert asset.materializable is False @@ -117,12 +413,12 @@ def test_create_and_read_back(self, store: Store): key="cron_job", name="Demo Daily", config={"cron": "0 6 * * *", "tags": ["daily"], "enabled": True}, - relations={"target": [(db_source.id, "")]}, + relations={"targets": [db_source.id]}, ) assert db_job.name == "Demo Daily" assert db_job.config == {"cron": "0 6 * * *", "tags": ["daily"], "enabled": True} assert db_job.state is None - assert [rel.dst_id for rel in db_job.out_relations] == [db_source.id] + assert [relation.dst_id for relation in db_job.out_relations] == [db_source.id] assert [job.id for job in store.components.list_all(_ORG, kinds=["job"])] == [db_job.id] @@ -135,23 +431,39 @@ def test_load_hydrates_targets(self, store: Store): key="cron_job", name="Demo Daily", config={"cron": "0 6 * * *"}, - relations={"target": [(db_source.id, ""), (db_asset.id, "")]}, + relations={"targets": [db_source.id, db_asset.id]}, ) job = store.components.load(db_job.id) assert isinstance(job, il.CronJob) assert job.cron == "0 6 * * *" - assert {type(target).key for target in job.targets} == {"demo_source", "demo_asset"} - assert {type(asset).key for asset in il.DAG(*job.targets).operations} == {"a", "b", "c", "d", "e", "demo_asset"} + assert {target.key for target in job.targets} == {"demo_source", "demo_asset"} + assert {asset.key for asset in il.DAG(*job.targets).operations} == {"a", "b", "c", "d", "e", "demo_asset"} + + def test_load_hydrates_a_target_that_is_a_source_owned_asset(self, store: Store): + db_source = store.components.create(_ORG, kind="source", key="demo_source", name="Demo") + owned_asset = _child(db_source, "a") + db_job = store.components.create( + _ORG, + kind="job", + key="cron_job", + name="Demo Daily", + config={"cron": "0 6 * * *"}, + relations={"targets": [owned_asset.id]}, + ) + + job = store.components.load(db_job.id) + assert isinstance(job, il.CronJob) + target = next(iter(job.targets)) + assert target.key == "a" + assert target.parent is not None + assert target.parent.key == "demo_source" def test_update_preserves_state_but_drops_the_cached_schedule(self, store: Store): db_job = store.components.create(_ORG, kind="job", key="cron_job", name="Job", config={"cron": "0 6 * * *"}) # Simulate the scheduler's targeted state write. - from sqlmodel import Session - from interloper_db.engine import get_engine - from interloper_db.models import Component with Session(get_engine()) as session: row = session.get(Component, db_job.id) @@ -168,29 +480,26 @@ def test_update_preserves_state_but_drops_the_cached_schedule(self, store: Store class FakeLinker(il.Component): """Test-only kind whose vocabulary the hydrator has never seen.""" - relation_types: ClassVar[dict[str, il.RelationDefinition]] = { - "link": il.RelationDefinition(kinds=["source"], field="links") - } - links: list[il.Component] = pydantic.Field(default_factory=list) + links: list[il.Component] = il.Relation("source", many=True, optional=True) il.KINDS.register(FakeLinker.kind, FakeLinker.anchor()) class TestOpenVocabulary: - """A novel kind + relation type persists and hydrates with no per-type code.""" + """A novel kind + relation name persists and hydrates with no per-kind code.""" - def test_custom_relation_type_round_trips(self, store: Store): + def test_a_custom_relation_name_round_trips(self, store: Store): store._catalog.components["fake_linker"] = FakeLinker.definition() db_source = store.components.create(_ORG, kind="source", key="demo_source", name="Demo") db_linker = store.components.create( - _ORG, kind="fake_linker", key="fake_linker", name="L", relations={"link": [(db_source.id, "")]} + _ORG, kind="fake_linker", key="fake_linker", name="L", relations={"links": [db_source.id]} ) linker = store.components.load(db_linker.id) assert isinstance(linker, FakeLinker) - assert [type(linked).key for linked in linker.links] == ["demo_source"] + assert [linked.key for linked in linker.links] == ["demo_source"] assert linker.links[0].id == str(db_source.id) @@ -203,7 +512,7 @@ def test_load_failure_message_omits_input_values(self, store: Store, monkeypatch class Probe(pydantic.BaseModel): app_secret: str - def raise_validation_error(spec): + def raise_validation_error(spec, catalog=None, *, resolve=None): Probe.model_validate({"token": "s3cret-value"}) monkeypatch.setattr(il.Component, "from_spec", raise_validation_error) @@ -226,9 +535,7 @@ def test_no_data_decodes_to_an_empty_dict(self, store: Store): def test_an_encrypted_row_without_a_cipher_is_an_actionable_error(self, component_db: Engine): writer = Store(catalog=_CATALOG, encrypt=lambda b: b, decrypt=lambda b: b) - row = writer.components.create( - _ORG, kind="connection", key="demo_connection", config={"token": "s3cret"} - ) + row = writer.components.create(_ORG, kind="connection", key="shop_connection", config={"token": "s3cret"}) row.data = row.data or b"payload" row.encrypted = True reader = Store(catalog=_CATALOG) @@ -237,14 +544,12 @@ def test_an_encrypted_row_without_a_cipher_is_an_actionable_error(self, componen reader.components._hydrator.decode_data(row) -class TestRelationsByType: +class TestRelationsByName: """The grouped relation lookup used while building a spec.""" def test_a_row_with_no_id_has_no_relations(self, store: Store): - from sqlmodel import Session - with Session(store.engine) as session: - assert store.components._hydrator._relations_by_type(session, None) == {} + assert store.components._hydrator._relations_by_name(session, None) == {} class TestResolvePath: @@ -252,7 +557,6 @@ class TestResolvePath: def test_a_drifted_key_is_an_actionable_error(self, component_db: Engine): from interloper.errors import CatalogKeyError - from sqlmodel import Session writer = Store(catalog=_CATALOG) row = writer.components.create(_ORG, kind="source", key="demo_source") From 711b8e6fd7c00863d551efdf77151806cb1b86f9 Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 03:21:24 +0200 Subject: [PATCH 07/19] feat(api)!: relations are addressed by name; org-wide list filters by kinds By Digitl --- .../src/interloper_api/routes/catalog.py | 4 +- .../src/interloper_api/routes/components.py | 117 +++++++------ .../tests/routes/test_components.py | 165 +++++++++++++----- 3 files changed, 187 insertions(+), 99 deletions(-) diff --git a/packages/interloper-api/src/interloper_api/routes/catalog.py b/packages/interloper-api/src/interloper_api/routes/catalog.py index dc95e5f0..97e9f335 100644 --- a/packages/interloper-api/src/interloper_api/routes/catalog.py +++ b/packages/interloper-api/src/interloper_api/routes/catalog.py @@ -30,8 +30,8 @@ def list_resource_kinds(catalog: Catalog = Depends(get_catalog)) -> list[str]: """Return distinct resource kinds from the catalog. A resource kind is any registered kind anchored under ``Resource`` - (currently ``connection`` and ``config``) — the kinds usable as - slot bindings on other components. + (currently ``connection`` and ``config``) - the kinds usable as + relation bindings on other components. Args: catalog: Injected catalog. diff --git a/packages/interloper-api/src/interloper_api/routes/components.py b/packages/interloper-api/src/interloper_api/routes/components.py index fdb0b4ff..98a73c5d 100644 --- a/packages/interloper-api/src/interloper_api/routes/components.py +++ b/packages/interloper-api/src/interloper_api/routes/components.py @@ -62,30 +62,32 @@ class RelationEntry(BaseModel): """One relation binding in a create/update request.""" dst_id: UUID - slot: str = "" class RelationCreateRequest(RelationEntry): """Request body for adding one relation.""" - type: str + name: str class RelationRef(BaseModel): """One relation binding in a component response.""" dst_id: UUID - slot: str = "" dst_kind: str class RelationResponse(BaseModel): - """An org-wide relation row (graph edges, dependency lists).""" + """An org-wide relation row (graph edges, dependency lists). + + ``src_kind`` rides along so the app's graph and upstream views can filter + asset-to-asset rows without a second lookup. + """ src_id: UUID + name: str dst_id: UUID - type: str - slot: str + src_kind: str dst_kind: str @@ -95,8 +97,8 @@ class ComponentCreateRequest(BaseModel): ``encrypted`` applies to secret kinds only: None encrypts whenever an encryption key is configured, an explicit bool forces it on or off. ``children`` applies to source kinds only and names the child asset keys - to enable (None enables all of them). Every relation type listed in - ``relations`` is replaced wholesale, so an empty list clears that type. + to enable (None enables all of them). Every relation name listed in + ``relations`` is replaced wholesale, so an empty list clears that name. """ kind: str @@ -228,35 +230,35 @@ class PartitionRowCountsResponse(BaseModel): def _relations_of(row: Component) -> dict[str, list[RelationRef]]: - """Group a component's outgoing relations by relation type. + """Group a component's outgoing relations by name. Args: row: The component row, with its ``out_relations`` eager-loaded. Returns: - A ``{type: [bindings]}`` map of the row's outgoing relations. + A ``{name: [bindings]}`` map of the row's outgoing relations. """ grouped: dict[str, list[RelationRef]] = {} for relation in row.out_relations: - grouped.setdefault(relation.type, []).append( - RelationRef(dst_id=relation.dst_id, slot=relation.slot, dst_kind=relation.dst_kind) + grouped.setdefault(relation.name, []).append( + RelationRef(dst_id=relation.dst_id, dst_kind=relation.dst_kind) ) return grouped -def _bindings(relations: dict[str, list[RelationEntry]] | None) -> dict[str, list[tuple[UUID, str]]] | None: - """Flatten a request's relation entries into the tuples the store takes. +def _bindings(relations: dict[str, list[RelationEntry]] | None) -> dict[str, list[UUID]] | None: + """Flatten a request's relation entries into the ids the store takes. Args: - relations: The request's ``{type: [entries]}`` map, or None to leave - every relation type untouched. + relations: The request's ``{name: [entries]}`` map, or None to leave + every relation name untouched. Returns: - A ``{type: [(dst_id, slot)]}`` map, or None when *relations* is None. + A ``{name: [dst_id, ...]}`` map, or None when *relations* is None. """ if relations is None: return None - return {type_: [(entry.dst_id, entry.slot) for entry in entries] for type_, entries in relations.items()} + return {name: [entry.dst_id for entry in entries] for name, entries in relations.items()} # -- Component endpoints ------------------------------------------------------- @@ -286,15 +288,19 @@ def list_components( @router.get("/relations") def list_relations( - type: str | None = None, + name: str | None = None, + src_kind: str | None = None, + dst_kind: str | None = None, user: Profile = Depends(require_viewer), org_id: UUID = Depends(get_org_id), store: Store = Depends(get_store), ) -> list[RelationResponse]: - """List the organisation's component relations, optionally by type. + """List the organisation's component relations, optionally filtered. Args: - type: The relation type to keep; None lists every type. + name: The relation name to keep; None lists every name. + src_kind: The source kind to keep; None lists every kind. + dst_kind: The destination kind to keep; None lists every kind. user: The authenticated user. org_id: The active organisation UUID. store: The Store instance. @@ -305,12 +311,12 @@ def list_relations( return [ RelationResponse( src_id=relation.src_id, + name=relation.name, dst_id=relation.dst_id, - type=relation.type, - slot=relation.slot, + src_kind=relation.src_kind, dst_kind=relation.dst_kind, ) - for relation in store.relations.list_all(org_id, type=type) + for relation in store.relations.list_all(org_id, name=name, src_kind=src_kind, dst_kind=dst_kind) ] @@ -464,7 +470,7 @@ def add_relation( Args: component_id: The source component's UUID. - body: The relation to add: its type, target ``dst_id`` and slot. + body: The relation to add: its name and target ``dst_id``. user: The authenticated user. store: The Store instance. @@ -482,45 +488,45 @@ def add_relation( if destination_row.org_id != source.org_id: raise HTTPException(status_code=404, detail=f"Component {body.dst_id} not found") try: - relation = store.relations.add(component_id, type=body.type, dst_id=body.dst_id, slot=body.slot) + relation = store.relations.add(component_id, name=body.name, dst_id=body.dst_id) except ConfigError as e: raise HTTPException(status_code=400, detail=str(e)) except NotFoundError as e: raise HTTPException(status_code=404, detail=str(e)) return RelationResponse( src_id=relation.src_id, + name=relation.name, dst_id=relation.dst_id, - type=relation.type, - slot=relation.slot, + src_kind=relation.src_kind, dst_kind=relation.dst_kind, ) -@router.delete("/{component_id}/relations/{type}/{dst_id}", status_code=204) +@router.delete("/{component_id}/relations/{name}/{dst_id}", status_code=204) def remove_relation( component_id: UUID, - type: str, + name: str, dst_id: UUID, user: Profile = Depends(get_current_user), store: Store = Depends(get_store), ) -> None: - """Remove a component's relations of one type toward one destination. + """Remove a component's relation of one name toward one destination. - Refused (400) for required dependency slots — repoint them instead. + Refused (400) for required dependency names - repoint them instead. Args: component_id: The source component's UUID. - type: The relation type to remove. + name: The relation name to remove. dst_id: The target component's UUID. user: The authenticated user. store: The Store instance. Raises: - HTTPException: 400 for a required dependency slot. + HTTPException: 400 for a required dependency name. """ load_authorized(store.components.get, component_id, user, store, label="Component", minimum="editor") try: - store.relations.remove(component_id, type=type, dst_id=dst_id) + store.relations.remove(component_id, name=name, dst_id=dst_id) except ConfigError as e: raise HTTPException(status_code=400, detail=str(e)) @@ -608,8 +614,8 @@ def handle_error(error: Exception, context: str) -> None: class ResolveRequest(BaseModel): """A request to resolve one provider-backed FetchField's options. - ``deps`` carries the credentials the form already holds, keyed by resource - slot (e.g. ``{"connection": {"access_token": ...}}``). + ``deps`` carries the credentials the form already holds, keyed by relation + name (e.g. ``{"connection": {"access_token": ...}}``). """ component_key: str @@ -626,18 +632,18 @@ async def resolve_fetch_field( """Resolve the options for a ``FetchField(provider=...)`` field. One endpoint resolves any field declared with - ``FetchField(provider=".")`` — there are no hand-written + ``FetchField(provider=".")`` - there are no hand-written per-provider routes. The component definition comes from the catalog - (authoritative — the provider reference comes from the server's schema, - never the client), the resource in ```` is instantiated from the - credentials the form already holds, and the ``@fetch_field_provider`` - method ```` is called on it. That marker is the allowlist: only - methods opted in that way may be invoked, so the browser cannot call - arbitrary attributes. + (authoritative - the provider reference comes from the server's schema, + never the client), the resource named by the relation is instantiated + from the credentials the form already holds, and the + ``@fetch_field_provider`` method ```` is called on it. That + marker is the allowlist: only methods opted in that way may be invoked, + so the browser cannot call arbitrary attributes. Args: - body: The component key, the field name, and the per-slot credentials - the form currently holds. + body: The component key, the field name, and the per-relation + credentials the form currently holds. catalog: The Catalog instance. _user: The authenticated user (viewer gate). @@ -646,8 +652,9 @@ async def resolve_fetch_field( Raises: HTTPException: 404 for an unknown component key, 400 when the field is - not a provider-backed FetchField or names an unknown resource - slot, 403 when the target method is not a fetch provider. + not a provider-backed FetchField or names an unknown or + undeclared relation, 403 when the target method is not a fetch + provider. """ defn = catalog.get(body.component_key) if defn is None: @@ -660,16 +667,20 @@ async def resolve_fetch_field( status_code=400, detail=f"Field '{body.field}' on '{body.component_key}' is not a provider-backed FetchField", ) - slot, _, method = str(provider).partition(".") + name, _, method = str(provider).partition(".") component_cls = import_from_path(defn.path) - resource_cls = getattr(component_cls, "resource_types", {}).get(slot) + relation = component_cls.relations.get(name) + resource_cls = relation.target if relation else None if resource_cls is None: - raise HTTPException(status_code=400, detail=f"Resource slot '{slot}' not found on '{body.component_key}'") + raise HTTPException( + status_code=400, + detail=f"Relation '{name}' not found on '{body.component_key}' or not declared from a component class", + ) - # Only pass through fields the resource actually declares — the form may + # Only pass through fields the resource actually declares - the form may # carry extra markers (e.g. an internal id) that the model would reject. - raw = body.deps.get(slot, {}) + raw = body.deps.get(name, {}) creds = {k: v for k, v in raw.items() if k in resource_cls.model_fields} resource = resource_cls(**creds) diff --git a/packages/interloper-api/tests/routes/test_components.py b/packages/interloper-api/tests/routes/test_components.py index 06f0c26a..edca70a7 100644 --- a/packages/interloper-api/tests/routes/test_components.py +++ b/packages/interloper-api/tests/routes/test_components.py @@ -447,19 +447,31 @@ def _load(self, component_id: UUID) -> Any: raise self.load_error return self.loaded - def _list_relations(self, org_id: UUID, type: str | None = None) -> list[Any]: - return [r for r in self.relation_rows if type is None or r.type == type] + def _list_relations( + self, + org_id: UUID, + name: str | None = None, + src_kind: str | None = None, + dst_kind: str | None = None, + ) -> list[Any]: + return [ + r + for r in self.relation_rows + if (name is None or r.name == name) + and (src_kind is None or r.src_kind == src_kind) + and (dst_kind is None or r.dst_kind == dst_kind) + ] - def _add_relation(self, src_id: UUID, *, type: str, dst_id: UUID, slot: str) -> Any: + def _add_relation(self, src_id: UUID, *, name: str, dst_id: UUID) -> Any: if self.error: raise self.error - self.added_relations.append({"src_id": src_id, "type": type, "dst_id": dst_id, "slot": slot}) - return SimpleNamespace(src_id=src_id, dst_id=dst_id, type=type, slot=slot, dst_kind="connection") + self.added_relations.append({"src_id": src_id, "name": name, "dst_id": dst_id}) + return SimpleNamespace(src_id=src_id, name=name, dst_id=dst_id, src_kind="source", dst_kind="connection") - def _remove_relation(self, src_id: UUID, *, type: str, dst_id: UUID) -> None: + def _remove_relation(self, src_id: UUID, *, name: str, dst_id: UUID) -> None: if self.error: raise self.error - self.removed_relations.append({"src_id": src_id, "type": type, "dst_id": dst_id}) + self.removed_relations.append({"src_id": src_id, "name": name, "dst_id": dst_id}) @pytest.fixture @@ -514,13 +526,13 @@ def test_no_components_is_an_empty_list(self, crud_client: TestClient) -> None: class TestListRelations: - """``GET /components/relations`` — optionally narrowed by type.""" + """``GET /components/relations`` — optionally narrowed by name and kinds.""" def test_lists_every_relation(self, crud_client: TestClient, crud_store: CrudStore) -> None: source_id, destination_id = uuid4(), uuid4() crud_store.relation_rows = [ SimpleNamespace( - src_id=source_id, dst_id=destination_id, type="resource", slot="connection", dst_kind="connection" + src_id=source_id, name="connection", dst_id=destination_id, src_kind="source", dst_kind="connection" ) ] @@ -529,22 +541,40 @@ def test_lists_every_relation(self, crud_client: TestClient, crud_store: CrudSto assert response.json() == [ { "src_id": str(source_id), + "name": "connection", "dst_id": str(destination_id), - "type": "resource", - "slot": "connection", + "src_kind": "source", "dst_kind": "connection", } ] - def test_the_type_filter_narrows_the_result(self, crud_client: TestClient, crud_store: CrudStore) -> None: + def test_the_name_filter_narrows_the_result(self, crud_client: TestClient, crud_store: CrudStore) -> None: crud_store.relation_rows = [ - SimpleNamespace(src_id=uuid4(), dst_id=uuid4(), type="resource", slot="a", dst_kind="connection"), - SimpleNamespace(src_id=uuid4(), dst_id=uuid4(), type="destination", slot="b", dst_kind="destination"), + SimpleNamespace( + src_id=uuid4(), name="connection", dst_id=uuid4(), src_kind="source", dst_kind="connection" + ), + SimpleNamespace( + src_id=uuid4(), name="destinations", dst_id=uuid4(), src_kind="source", dst_kind="destination" + ), + ] + + response = crud_client.get("/components/relations?name=destinations") + + assert [r["name"] for r in response.json()] == ["destinations"] + + def test_list_relations_filters_by_kind(self, crud_client: TestClient, crud_store: CrudStore) -> None: + crud_store.relation_rows = [ + SimpleNamespace(src_id=uuid4(), name="upstreams", dst_id=uuid4(), src_kind="asset", dst_kind="asset"), + SimpleNamespace( + src_id=uuid4(), name="connection", dst_id=uuid4(), src_kind="source", dst_kind="connection" + ), ] - response = crud_client.get("/components/relations?type=destination") + response = crud_client.get("/components/relations", params={"src_kind": "asset", "dst_kind": "asset"}) - assert [r["type"] for r in response.json()] == ["destination"] + rows = response.json() + assert all(r["dst_kind"] == "asset" for r in rows) + assert [r["name"] for r in rows] == ["upstreams"] class TestCreateComponent: @@ -569,12 +599,12 @@ def test_relations_are_flattened_into_store_bindings( body = { "kind": "source", "key": "fb", - "relations": {"resource": [{"dst_id": str(destination_id), "slot": "connection"}]}, + "relations": {"connection": [{"dst_id": str(destination_id)}]}, } crud_client.post("/components/", json=body) - assert crud_store.created[0]["relations"] == {"resource": [(destination_id, "connection")]} + assert crud_store.created[0]["relations"] == {"connection": [destination_id]} def test_omitted_relations_stay_none(self, crud_client: TestClient, crud_store: CrudStore) -> None: # None means "leave every relation type untouched", which is not the @@ -710,14 +740,27 @@ class TestAddRelation: def test_adds_the_relation(self, crud_client: TestClient, crud_store: CrudStore) -> None: source_id, destination_id = uuid4(), uuid4() - body = {"type": "resource", "dst_id": str(destination_id), "slot": "connection"} + body = {"name": "connection", "dst_id": str(destination_id)} response = crud_client.post(f"/components/{source_id}/relations", json=body) assert response.status_code == 201 assert response.json()["dst_kind"] == "connection" assert crud_store.added_relations == [ - {"src_id": source_id, "type": "resource", "dst_id": destination_id, "slot": "connection"} + {"src_id": source_id, "name": "connection", "dst_id": destination_id} + ] + + def test_add_relation_by_name(self, crud_client: TestClient, crud_store: CrudStore) -> None: + source_id, destination_id = uuid4(), uuid4() + + response = crud_client.post( + f"/components/{source_id}/relations", json={"name": "destinations", "dst_id": str(destination_id)} + ) + + assert response.status_code == 201 + assert response.json()["name"] == "destinations" + assert crud_store.added_relations == [ + {"src_id": source_id, "name": "destinations", "dst_id": destination_id} ] def test_a_target_in_another_org_is_a_404( @@ -735,7 +778,7 @@ def get(component_id: UUID) -> Any: response = crud_client.post( f"/components/{source_id}/relations", - json={"type": "resource", "dst_id": str(destination_id), "slot": "connection"}, + json={"name": "connection", "dst_id": str(destination_id)}, ) assert response.status_code == 404 @@ -752,30 +795,40 @@ def test_store_errors_map_to_statuses( response = crud_client.post( f"/components/{uuid4()}/relations", - json={"type": "resource", "dst_id": str(uuid4()), "slot": "connection"}, + json={"name": "connection", "dst_id": str(uuid4())}, ) assert response.status_code == expected class TestRemoveRelation: - """``DELETE /components/{id}/relations/{type}/{dst_id}``.""" + """``DELETE /components/{id}/relations/{name}/{dst_id}``.""" def test_removes_the_relation(self, crud_client: TestClient, crud_store: CrudStore) -> None: source_id, destination_id = uuid4(), uuid4() - response = crud_client.delete(f"/components/{source_id}/relations/resource/{destination_id}") + response = crud_client.delete(f"/components/{source_id}/relations/connection/{destination_id}") assert response.status_code == 204 assert crud_store.removed_relations == [ - {"src_id": source_id, "type": "resource", "dst_id": destination_id} + {"src_id": source_id, "name": "connection", "dst_id": destination_id} ] - def test_a_required_slot_is_refused(self, crud_client: TestClient, crud_store: CrudStore) -> None: - # Required dependency slots are repointed, never emptied. - crud_store.error = ConfigError("slot 'connection' is required") + def test_remove_relation_route_uses_name(self, crud_client: TestClient, crud_store: CrudStore) -> None: + source_id, destination_id = uuid4(), uuid4() + + response = crud_client.delete(f"/components/{source_id}/relations/destinations/{destination_id}") - response = crud_client.delete(f"/components/{uuid4()}/relations/resource/{uuid4()}") + assert response.status_code == 204 + assert crud_store.removed_relations == [ + {"src_id": source_id, "name": "destinations", "dst_id": destination_id} + ] + + def test_a_required_name_is_refused(self, crud_client: TestClient, crud_store: CrudStore) -> None: + # Required dependency names are repointed, never emptied. + crud_store.error = ConfigError("'connection' is required") + + response = crud_client.delete(f"/components/{uuid4()}/relations/connection/{uuid4()}") assert response.status_code == 400 @@ -884,35 +937,58 @@ def broken() -> dict[str, int]: class TestRelationGrouping: """``_relations_of`` and ``_bindings`` — the two shape converters.""" - def test_outgoing_relations_group_by_type(self) -> None: + def test_outgoing_relations_group_by_name(self) -> None: first, second = uuid4(), uuid4() row = _row( relations=[ - SimpleNamespace(type="resource", dst_id=first, slot="connection", dst_kind="connection"), - SimpleNamespace(type="resource", dst_id=second, slot="other", dst_kind="connection"), - SimpleNamespace(type="destination", dst_id=first, slot="", dst_kind="destination"), + SimpleNamespace(name="connection", dst_id=first, dst_kind="connection"), + SimpleNamespace(name="connection", dst_id=second, dst_kind="connection"), + SimpleNamespace(name="destinations", dst_id=first, dst_kind="destination"), ] ) grouped = components_module._relations_of(row) - assert set(grouped) == {"resource", "destination"} - assert [ref.dst_id for ref in grouped["resource"]] == [first, second] + assert set(grouped) == {"connection", "destinations"} + assert [ref.dst_id for ref in grouped["connection"]] == [first, second] def test_no_relations_is_an_empty_map(self) -> None: assert components_module._relations_of(_row()) == {} - def test_bindings_flatten_entries_to_tuples(self) -> None: + def test_bindings_flatten_entries_to_ids(self) -> None: destination_id = uuid4() - entries = {"resource": [components_module.RelationEntry(dst_id=destination_id, slot="connection")]} + entries = {"connection": [components_module.RelationEntry(dst_id=destination_id)]} - assert components_module._bindings(entries) == {"resource": [(destination_id, "connection")]} + assert components_module._bindings(entries) == {"connection": [destination_id]} def test_bindings_of_none_stay_none(self) -> None: - # None means "leave every relation type untouched". + # None means "leave every relation name untouched". assert components_module._bindings(None) is None +class TestComponentResponseRelations: + """``ComponentResponse.relations`` — keyed by name, dst_kind carried along.""" + + def test_component_response_relations_keyed_by_name(self) -> None: + connection_id, destination_id = uuid4(), uuid4() + row = _row( + relations=[ + SimpleNamespace(name="connection", dst_id=connection_id, dst_kind="connection"), + SimpleNamespace(name="destinations", dst_id=destination_id, dst_kind="destination"), + ] + ) + store = cast(Store, SimpleNamespace( + components=SimpleNamespace(status=lambda row, parent_key=None: ComponentStatus.OK) + )) + + response = components_module.ComponentResponse.from_row(row, store, include_config=False) + + assert set(response.relations) == {"connection", "destinations"} + assert response.relations["connection"] == [ + components_module.RelationRef(dst_id=connection_id, dst_kind="connection") + ] + + class TestHandleError: """``handle_error`` maps a provider failure to a status, never a traceback.""" @@ -1022,13 +1098,13 @@ def test_anything_else_is_a_generic_error(self) -> None: class TestResolveEdgeCases: """``POST /components/resolve`` — the guards between the field and the provider.""" - def test_an_unknown_resource_slot_is_a_400( + def test_an_unknown_relation_name_is_a_400( self, source_catalog: il.Catalog, monkeypatch: pytest.MonkeyPatch ) -> None: - # The FetchField names a slot the component does not declare. + # The FetchField names a relation the component does not declare. from interloper_assets.facebook_ads.source import FacebookAds - monkeypatch.setattr(FacebookAds, "resource_types", {}) + monkeypatch.setattr(FacebookAds, "relations", {}) response = _client(source_catalog).post( "/components/resolve", @@ -1036,7 +1112,8 @@ def test_an_unknown_resource_slot_is_a_400( ) assert response.status_code == 400 - assert "Resource slot" in response.json()["detail"] + assert "Relation 'connection' not found" in response.json()["detail"] + assert "not declared from a component class" in response.json()["detail"] def test_a_provider_failure_is_mapped_not_raised( self, source_catalog: il.Catalog, mock_graph @@ -1054,7 +1131,7 @@ def handler(request: httpx.Request) -> httpx.Response: assert response.status_code == 500 assert response.json()["detail"].startswith("Failed resolving facebook_ads.account_id") - def test_a_slot_that_is_not_a_fetch_provider_is_a_403( + def test_a_relation_that_is_not_a_fetch_provider_is_a_403( self, source_catalog: il.Catalog, monkeypatch: pytest.MonkeyPatch ) -> None: # Validated at catalog build, so this is a defensive guard. From 09315302a7e64f31b5e617d0d42ba00ba98ee0e0 Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 03:35:18 +0200 Subject: [PATCH 08/19] feat(toolkit): lineage by asset kinds; generic bind_relation and unbind_relation tools By Digitl --- .../src/interloper_toolkit/__init__.py | 8 +- .../src/interloper_toolkit/catalog.py | 2 +- .../src/interloper_toolkit/collection.py | 63 ++++++++- .../src/interloper_toolkit/lineage.py | 24 ++-- .../src/interloper_toolkit/models.py | 27 +++- .../interloper-toolkit/tests/test_toolkit.py | 129 ++++++++++++++++-- 6 files changed, 217 insertions(+), 36 deletions(-) diff --git a/packages/interloper-toolkit/src/interloper_toolkit/__init__.py b/packages/interloper-toolkit/src/interloper_toolkit/__init__.py index 399a2953..eae28b1e 100644 --- a/packages/interloper-toolkit/src/interloper_toolkit/__init__.py +++ b/packages/interloper-toolkit/src/interloper_toolkit/__init__.py @@ -1,4 +1,4 @@ -"""Read-only tool functions shared by AI surfaces (agent, MCP server). +"""Tool functions shared by AI surfaces (agent, MCP server). Every function takes a :class:`~interloper_toolkit.context.ToolkitContext` as its first argument and returns `` | ToolError`` — typed @@ -6,6 +6,12 @@ the literal ``status`` field, never raising. The docstrings are LLM-facing: both the ADK agent and the MCP server surface them verbatim as tool descriptions. + +Almost every function here is read-only; the sole exceptions are +:func:`interloper_toolkit.collection.bind_relation` and +:func:`interloper_toolkit.collection.unbind_relation`, which write. A +surface that must stay read-only (the MCP server's own registration is one) +never registers those two. """ from interloper_toolkit.context import ToolkitContext, serialize diff --git a/packages/interloper-toolkit/src/interloper_toolkit/catalog.py b/packages/interloper-toolkit/src/interloper_toolkit/catalog.py index ea271a11..0c398a13 100644 --- a/packages/interloper-toolkit/src/interloper_toolkit/catalog.py +++ b/packages/interloper-toolkit/src/interloper_toolkit/catalog.py @@ -122,7 +122,7 @@ def list_definitions( def get_definition(ctx: ToolkitContext, key: str) -> DefinitionDetail | ToolError: """Get a component definition's full catalog detail. - For a source this includes the config schema, resource slots, + For a source this includes the config schema, declared relations, destination types, and all its assets with their schemas. This is the catalog definition (the component *type*), not an instance from the org's collection. diff --git a/packages/interloper-toolkit/src/interloper_toolkit/collection.py b/packages/interloper-toolkit/src/interloper_toolkit/collection.py index 83319c8b..74458c44 100644 --- a/packages/interloper-toolkit/src/interloper_toolkit/collection.py +++ b/packages/interloper-toolkit/src/interloper_toolkit/collection.py @@ -1,15 +1,30 @@ -"""Collection tools — the org's component instances, read-only. +"""Collection tools: the org's component instances. -Creation and connection operations stay with the agent; this module is -shared with surfaces that must stay read-only. +``list_components`` is read-only, shared with surfaces that must stay +read-only (kind-specific creation and connection operations stay with the +agent). ``bind_relation`` and ``unbind_relation`` are the exception: they +write, generically over every kind's declared relations, so a caller must +never register them alongside a read-only tool set (see +``interloper_mcp.tools``, deliberately read-only), only wherever that +surface's own write tools already live. """ from __future__ import annotations +from uuid import UUID + from interloper.component import KINDS +from interloper.errors import ConfigError, NotFoundError from interloper_toolkit.context import ToolkitContext -from interloper_toolkit.models import ComponentCounts, ComponentList, ComponentSummary, ToolError +from interloper_toolkit.models import ( + BindResult, + ComponentCounts, + ComponentList, + ComponentSummary, + ToolError, + UnbindResult, +) def list_components( @@ -58,3 +73,43 @@ def list_components( return ComponentList(kind=kind, count=len(results), components=results) except Exception as e: return ToolError(error=str(e)) + + +# -- Relations (write) --------------------------------------------------------- + + +def bind_relation(ctx: ToolkitContext, component_id: str, name: str, dst_id: str) -> BindResult | ToolError: + """Bind one component to another under a declared relation name. + + Works on any kind: a source's connection, a job's watched assets, a + destination target, whatever the component's own class declares under + that name. A ``many`` name accumulates; a single-valued one repoints, so + rebinding it needs no prior unbind_relation call. + + Args: + component_id: UUID of the component the relation originates from. + name: Relation name, which the component's class must declare. + dst_id: UUID of the destination component the relation points at. + Must belong to the same organisation as the source. + """ + try: + row = ctx.store.relations.add(UUID(component_id), name=name, dst_id=UUID(dst_id)) + except (ConfigError, NotFoundError, ValueError) as e: + return ToolError(error=str(e)) + return BindResult(src_id=str(row.src_id), name=row.name, dst_id=str(row.dst_id), dst_kind=row.dst_kind) + + +def unbind_relation(ctx: ToolkitContext, component_id: str, name: str, dst_id: str) -> UnbindResult | ToolError: + """Detach one component from another under a declared relation name. + + Args: + component_id: UUID of the component the relation originates from. + name: Relation name the edge is filed under. + dst_id: UUID of the destination the removed edge points at. Removing + an edge that isn't there is a no-op. + """ + try: + ctx.store.relations.remove(UUID(component_id), name=name, dst_id=UUID(dst_id)) + except (ConfigError, NotFoundError, ValueError) as e: + return ToolError(error=str(e)) + return UnbindResult(src_id=component_id, name=name, dst_id=dst_id) diff --git a/packages/interloper-toolkit/src/interloper_toolkit/lineage.py b/packages/interloper-toolkit/src/interloper_toolkit/lineage.py index 6a6305c5..c4626bc6 100644 --- a/packages/interloper-toolkit/src/interloper_toolkit/lineage.py +++ b/packages/interloper-toolkit/src/interloper_toolkit/lineage.py @@ -10,11 +10,11 @@ AssetRef, CrossSourceDependencies, CrossSourceEdge, - DependencyEdge, DownstreamResult, ImpactAnalysis, LineageItem, LineageResult, + RelationEdge, ToolError, UpstreamResult, ) @@ -27,19 +27,19 @@ def get_upstream(ctx: ToolkitContext, asset_id: str) -> UpstreamResult | ToolErr asset_id: UUID of the asset to inspect. Returns the list of upstream assets that this asset depends on, - including the parameter name used for each dependency. + including the relation name used for each dependency. """ try: - deps = ctx.store.relations.list_all(ctx.org_id, type="upstream") + deps = ctx.store.relations.list_all(ctx.org_id, src_kind="asset", dst_kind="asset") target = UUID(asset_id) upstream = [] for dep in deps: if dep.src_id == target: asset = ctx.store.components.get(dep.dst_id, kind="asset") - upstream.append(DependencyEdge( + upstream.append(RelationEdge( asset_id=str(dep.dst_id), - param_name=dep.slot, + param_name=dep.name, asset_key=asset.key, source_id=str(asset.parent_id), )) @@ -58,16 +58,16 @@ def get_downstream(ctx: ToolkitContext, asset_id: str) -> DownstreamResult | Too Returns the list of assets that directly depend on this asset. """ try: - deps = ctx.store.relations.list_all(ctx.org_id, type="upstream") + deps = ctx.store.relations.list_all(ctx.org_id, src_kind="asset", dst_kind="asset") target = UUID(asset_id) downstream = [] for dep in deps: if dep.dst_id == target: asset = ctx.store.components.get(dep.src_id, kind="asset") - downstream.append(DependencyEdge( + downstream.append(RelationEdge( asset_id=str(dep.src_id), - param_name=dep.slot, + param_name=dep.name, asset_key=asset.key, source_id=str(asset.parent_id), )) @@ -165,7 +165,7 @@ def cross_source_dependencies(ctx: ToolkitContext) -> CrossSourceDependencies | different sources. """ try: - deps = ctx.store.relations.list_all(ctx.org_id, type="upstream") + deps = ctx.store.relations.list_all(ctx.org_id, src_kind="asset", dst_kind="asset") assets = ctx.store.components.list_all(ctx.org_id, kinds=["asset"]) asset_source: dict[UUID, UUID | None] = {} @@ -186,7 +186,7 @@ def cross_source_dependencies(ctx: ToolkitContext) -> CrossSourceDependencies | downstream=asset_info.get(dep.src_id, AssetRef()), upstream_asset_id=str(dep.dst_id), upstream=asset_info.get(dep.dst_id, AssetRef()), - param_name=dep.slot, + param_name=dep.name, )) return CrossSourceDependencies(cross_source_count=len(cross_deps), dependencies=cross_deps) @@ -201,7 +201,7 @@ def _build_adjacency( ctx: ToolkitContext, direction: str, ) -> tuple[dict[UUID, list[UUID]], dict[UUID, dict[str, str]]]: - """Build an adjacency map and asset info lookup from all dependencies. + """Build an adjacency map and asset info lookup from all asset-to-asset relations. Args: ctx: The toolkit context. @@ -211,7 +211,7 @@ def _build_adjacency( ``(adjacency_map, asset_info_map)`` — info values feed :class:`LineageItem` kwargs (asset_key, source_id, source_key). """ - deps = ctx.store.relations.list_all(ctx.org_id, type="upstream") + deps = ctx.store.relations.list_all(ctx.org_id, src_kind="asset", dst_kind="asset") assets = ctx.store.components.list_all(ctx.org_id, kinds=["asset"]) asset_info: dict[UUID, dict[str, str]] = {} diff --git a/packages/interloper-toolkit/src/interloper_toolkit/models.py b/packages/interloper-toolkit/src/interloper_toolkit/models.py index 2c95b8bc..8c532681 100644 --- a/packages/interloper-toolkit/src/interloper_toolkit/models.py +++ b/packages/interloper-toolkit/src/interloper_toolkit/models.py @@ -180,11 +180,30 @@ class ComponentList(BaseModel): components: list[ComponentSummary] +class BindResult(BaseModel): + """One relation edge created or repointed by ``bind_relation``.""" + + status: Literal["success"] = "success" + src_id: str + name: str + dst_id: str + dst_kind: str + + +class UnbindResult(BaseModel): + """One relation edge removed by ``unbind_relation``.""" + + status: Literal["success"] = "success" + src_id: str + name: str + dst_id: str + + # -- Lineage -------------------------------------------------------------------- -class DependencyEdge(BaseModel): - """A direct dependency edge from the perspective of one asset.""" +class RelationEdge(BaseModel): + """A direct asset-to-asset relation edge from the perspective of one asset.""" asset_id: str param_name: str @@ -197,7 +216,7 @@ class UpstreamResult(BaseModel): status: Literal["success"] = "success" asset_id: str - upstream: list[DependencyEdge] + upstream: list[RelationEdge] class DownstreamResult(BaseModel): @@ -205,7 +224,7 @@ class DownstreamResult(BaseModel): status: Literal["success"] = "success" asset_id: str - downstream: list[DependencyEdge] + downstream: list[RelationEdge] class LineageItem(BaseModel): diff --git a/packages/interloper-toolkit/tests/test_toolkit.py b/packages/interloper-toolkit/tests/test_toolkit.py index 005a5be4..b4fc3b69 100644 --- a/packages/interloper-toolkit/tests/test_toolkit.py +++ b/packages/interloper-toolkit/tests/test_toolkit.py @@ -1,8 +1,10 @@ -"""Tests for the shared read-only toolkit (``interloper_toolkit``). +"""Tests for the shared toolkit (``interloper_toolkit``). A real Store over in-memory SQLite plus a hand-built dumped catalog; the properties under test are the structured ``status`` contract, org scoping, -and the pure logic (lineage traversal, coverage math, schema search). +and the pure logic (lineage traversal, coverage math, schema search), plus +the two write tools (``bind_relation``, ``unbind_relation``) against a +catalog that declares real relations. """ from __future__ import annotations @@ -16,18 +18,49 @@ import interloper as il import pytest from interloper_db import engine as engine_module -from interloper_db.models import Backfill, Component, ComponentRelation, Run +from interloper_db.models import Backfill, Component, ComponentRelation, Quota, Run from interloper_db.store import Store from sqlalchemy import Engine, event from sqlalchemy.pool import StaticPool from sqlmodel import Session -from interloper_toolkit import ToolkitContext, analytics, lineage, scheduling +from interloper_toolkit import ToolkitContext, analytics, collection, lineage, scheduling from interloper_toolkit import catalog as catalog_tools ORG_ID = uuid4() OTHER_ORG_ID = uuid4() + +class DemoConnection(il.Connection): + """Connection the lineage-by-name test sources bind.""" + + +class ShopSource(il.Source): + """Source owning the asset a cross-source relation points at by name.""" + + connection: DemoConnection + + class Orders(il.Asset): + """Order rows another source's asset depends on.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + +class FinanceSource(il.Source): + """Source whose report asset depends on shop_source's orders by name.""" + + connection: DemoConnection + + class Revenue(il.Asset): + """Revenue rows, computed from shop_source's orders.""" + + orders = il.Relation("asset", "shop_source.orders") + + def data(self, context: il.ExecutionContext, orders: il.Upstream) -> list[dict]: + return [] + + CATALOG_DUMP: dict[str, Any] = { "facebook_ads": { "kind": "source", @@ -72,7 +105,7 @@ @pytest.fixture def toolkit_db() -> Iterator[Engine]: - """A fresh in-memory database with component, relation, and run tables. + """A fresh in-memory database with component, relation, run, and quota tables. Yields: The engine bound to that database, disposed once the test finishes. @@ -89,7 +122,7 @@ def _configure_connection(dbapi_connection: Any, _record: Any) -> None: dbapi_connection.execute("PRAGMA foreign_keys=ON") dbapi_connection.create_function("gen_random_uuid", 0, lambda: uuid4().hex) - for model in (Component, ComponentRelation, Backfill, Run): + for model in (Component, ComponentRelation, Backfill, Run, Quota): model.__table__.create(eng) # ty: ignore[unresolved-attribute] try: yield eng @@ -99,8 +132,18 @@ def _configure_connection(dbapi_connection: Any, _record: Any) -> None: @pytest.fixture -def ctx(toolkit_db: Engine) -> ToolkitContext: - store = Store(catalog=il.Catalog(components={})) +def store(toolkit_db: Engine) -> Store: + """A store whose catalog knows the lineage-by-name test sources. + + Returns: + A store reading and writing the fixture database. + + """ + return Store(catalog=il.Catalog.from_assets([ShopSource, FinanceSource])) + + +@pytest.fixture +def ctx(store: Store) -> ToolkitContext: return ToolkitContext(store=store, catalog=CATALOG_DUMP, org_id=ORG_ID) @@ -116,12 +159,8 @@ def _seed_chain(org_id: Any = ORG_ID) -> dict[str, Any]: b = Component(org_id=org_id, kind="asset", key="b", parent_id=source.id) c = Component(org_id=org_id, kind="asset", key="c", parent_id=source.id) deps = [ - ComponentRelation( - src_id=b.id, dst_id=a.id, type="upstream", slot="a", org_id=org_id, src_kind="asset", dst_kind="asset" - ), - ComponentRelation( - src_id=c.id, dst_id=b.id, type="upstream", slot="b", org_id=org_id, src_kind="asset", dst_kind="asset" - ), + ComponentRelation(src_id=b.id, dst_id=a.id, name="a", org_id=org_id, src_kind="asset", dst_kind="asset"), + ComponentRelation(src_id=c.id, dst_id=b.id, name="b", org_id=org_id, src_kind="asset", dst_kind="asset"), ] ids = {"source": source.id, "a": a.id, "b": b.id, "c": c.id} with Session(engine_module.get_engine()) as session: @@ -157,6 +196,68 @@ def test_other_orgs_edges_are_invisible(self, ctx: ToolkitContext): assert result.status == "success" assert result.lineage_count == 0 + def test_get_upstream_reports_relation_name(self, ctx: ToolkitContext, store: Store): + shop = store.components.create(ORG_ID, kind="source", key="shop_source") + finance = store.components.create(ORG_ID, kind="source", key="finance_source") + orders = next(child for child in shop.children if child.key == "orders") + revenue = next(child for child in finance.children if child.key == "revenue") + store.relations.add(revenue.id, name="orders", dst_id=orders.id) + + result = lineage.get_upstream(ctx, str(revenue.id)) + + assert result.status == "success" + assert [(edge.param_name, edge.asset_id) for edge in result.upstream] == [("orders", str(orders.id))] + + def test_lineage_ignores_non_asset_relations(self, ctx: ToolkitContext, store: Store): + shop = store.components.create(ORG_ID, kind="source", key="shop_source") + finance = store.components.create(ORG_ID, kind="source", key="finance_source") + orders = next(child for child in shop.children if child.key == "orders") + revenue = next(child for child in finance.children if child.key == "revenue") + bq = store.components.create(ORG_ID, kind="destination", key="bq") + store.relations.add(revenue.id, name="orders", dst_id=orders.id) + store.relations.add(finance.id, name="destinations", dst_id=bq.id) + + result = lineage.get_full_lineage(ctx, str(revenue.id), direction="upstream") + + assert result.status == "success" + assert result.lineage_count == 1 + assert all(item.asset_key for item in result.lineage) + + +class TestBindRelation: + def test_bind_relation_creates_row(self, ctx: ToolkitContext, store: Store): + source = store.components.create(ORG_ID, kind="source", key="shop_source") + bq = store.components.create(ORG_ID, kind="destination", key="bq") + + result = collection.bind_relation(ctx, str(source.id), "destinations", str(bq.id)) + + assert result.status == "success" + assert (result.name, result.dst_kind) == ("destinations", "destination") + + def test_bind_relation_wrong_kind_is_tool_error(self, ctx: ToolkitContext, store: Store): + source = store.components.create(ORG_ID, kind="source", key="shop_source") + bq = store.components.create(ORG_ID, kind="destination", key="bq") + + result = collection.bind_relation(ctx, str(source.id), "connection", str(bq.id)) + + assert result.status == "error" + assert "does not accept" in result.error + + def test_bind_relation_bad_uuid_is_tool_error(self, ctx: ToolkitContext): + result = collection.bind_relation(ctx, "not-a-uuid", "destinations", "also-not-a-uuid") + + assert result.status == "error" + + def test_unbind_relation_removes_row(self, ctx: ToolkitContext, store: Store): + source = store.components.create(ORG_ID, kind="source", key="shop_source") + bq = store.components.create(ORG_ID, kind="destination", key="bq") + store.relations.add(source.id, name="destinations", dst_id=bq.id) + + result = collection.unbind_relation(ctx, str(source.id), "destinations", str(bq.id)) + + assert result.status == "success" + assert store.relations.list_all(ORG_ID, name="destinations") == [] + class TestCatalog: def test_search_fields_matches_across_sources(self, ctx: ToolkitContext): From 471746446a2e4244f06779626e09622e62e88fa4 Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 03:45:35 +0200 Subject: [PATCH 09/19] feat(agent): bind source connections and destinations by relation name By Digitl --- .../src/interloper_agent/tools/collection.py | 95 +++++++++++++------ .../tests/tools/test_collection.py | 79 +++++++++++++++ 2 files changed, 147 insertions(+), 27 deletions(-) diff --git a/packages/interloper-agent/src/interloper_agent/tools/collection.py b/packages/interloper-agent/src/interloper_agent/tools/collection.py index 4e2c17d9..3212c168 100644 --- a/packages/interloper-agent/src/interloper_agent/tools/collection.py +++ b/packages/interloper-agent/src/interloper_agent/tools/collection.py @@ -501,46 +501,87 @@ def _source_relations( source_key: str, connection_id: str | None, destination_ids: list[str] | None, -) -> tuple[dict[str, list[tuple[UUID, str]]] | None, dict[str, Any] | None]: - """Bind the connection into the definition's resource slot, plus destinations. +) -> tuple[dict[str, list[UUID]] | None, dict[str, Any] | None]: + """Bind the connection into the definition's named relation, plus destinations. + + Finds the relations the definition declares whose ``kind`` includes + ``"connection"``. A given connection binds under whichever one it fits + (an empty ``key`` accepts any connection, otherwise the connection's key + must appear in the relation's key list); every non-optional connection + relation left unbound afterwards is an error naming the relation and the + key it expects. Destinations bind under the fixed ``"destinations"`` name + once each one is confirmed to belong to the organisation. + + Args: + store: The database store, for looking up the connection and + destination rows. + org_id: Organisation UUID the connection and every destination must + belong to. + defn: The source's catalog definition, carrying its declared + ``relations`` (name to relation dict: ``kind``, ``key``, ``many``, + ``optional``, ``on_delete``, ``name``). + source_key: The source definition's catalog key, named in error + messages. + connection_id: UUID of the connection to bind, or ``None`` to leave + every connection relation unbound. + destination_ids: UUIDs of the destinations to attach, or ``None``. Returns: - ``(relations, None)`` or ``(None, error_response)``. + ``(relations, None)``, ``relations`` mapping relation name to the + UUIDs bound under it (ready for ``ComponentStore.create``'s + ``relations`` argument), or ``(None, error_response)``. """ - slots = (((defn.get("relations") or {}).get("resource") or {}).get("slots")) or {} + relations_defn = defn.get("relations") or {} + connection_relations = { + name: relation + for name, relation in relations_defn.items() + if "connection" in (relation["kind"] if isinstance(relation["kind"], list) else [relation["kind"]]) + } + connection = None if connection_id is not None: connection = store.components.get(UUID(connection_id), kind="connection") if connection.org_id != org_id: return None, {"status": "error", "error": f"Connection '{connection_id}' not found"} - resource_bindings: list[tuple[UUID, str]] = [] - for slot_name, spec in slots.items(): - expected = spec.get("key") - if connection is not None and (not expected or connection.key == expected): - resource_bindings.append((connection.id, slot_name)) - connection = None - elif spec.get("required"): + + bindings: dict[str, list[UUID]] = {} + if connection is not None: + name = next( + ( + relation_name + for relation_name, relation in connection_relations.items() + if not relation.get("key") + or connection.key in ([relation["key"]] if isinstance(relation["key"], str) else relation["key"]) + ), + None, + ) + if name is None: + return None, { + "status": "error", + "error": f"Connection '{connection.key}' does not fit any relation of '{source_key}'", + } + bindings[name] = [connection.id] + + for name, relation in connection_relations.items(): + if not relation.get("optional") and name not in bindings: + expected = relation.get("key") or "connection" return None, { "status": "error", "error": ( - f"'{source_key}' requires a '{expected or 'connection'}' in slot '{slot_name}' — " + f"'{source_key}' requires a '{expected}' as '{name}'; " "pick one from the collection or set one up first" ), } - if connection is not None: - return None, { - "status": "error", - "error": f"Connection '{connection.key}' does not fit any slot of '{source_key}'", - } - destination_bindings: list[tuple[UUID, str]] = [] + destination_bindings: list[UUID] = [] for dest_id in destination_ids or []: dest = store.components.get(UUID(dest_id), kind="destination") if dest.org_id != org_id: return None, {"status": "error", "error": f"Destination '{dest_id}' not found"} - destination_bindings.append((dest.id, "")) + destination_bindings.append(dest.id) + bindings["destinations"] = destination_bindings - return {"resource": resource_bindings, "destination": destination_bindings}, None + return bindings, None def _unresolved_requirements(defn: dict[str, Any], row: Any) -> list[str]: @@ -576,8 +617,8 @@ def create_source( name: Display name — default to the label of the chosen account / discriminator option. config: Values for the definition's config schema (e.g. account_id). - connection_id: UUID of the connection to bind — required when the - definition declares a required connection slot. + connection_id: UUID of the connection to bind; required when the + definition declares a required connection relation. asset_keys: Child asset keys to enable; omit to enable all. destination_ids: Destination UUIDs to attach (optional). """ @@ -619,8 +660,8 @@ def create_source( "key": row.key, "name": row.name, "asset_count": len(row.children), - "connection_bound": bool(relations["resource"]), - "destination_count": len(relations["destination"]), + "connection_bound": connection_id is not None, + "destination_count": len(relations["destinations"]), }, "unresolved_requirements": _unresolved_requirements(defn, row), } @@ -765,12 +806,12 @@ def create_job( org_id = get_org_id(tool_context) store = get_store() - targets: list[tuple[UUID, str]] = [] + targets: list[UUID] = [] for source_id in target_source_ids: source = store.components.get(UUID(source_id), kind="source") if source.org_id != org_id: return {"status": "error", "error": f"Source '{source_id}' not found"} - targets.append((source.id, "")) + targets.append(source.id) if not targets: return {"status": "error", "error": "target_source_ids must name at least one source"} @@ -787,7 +828,7 @@ def create_job( "lookback": lookback, "offset": offset, }, - relations={"target": targets}, + relations={"targets": targets}, ) except (ConfigError, CatalogKeyError) as e: return {"status": "error", "error": str(e)} diff --git a/packages/interloper-agent/tests/tools/test_collection.py b/packages/interloper-agent/tests/tools/test_collection.py index 77474101..f8f1e74d 100644 --- a/packages/interloper-agent/tests/tools/test_collection.py +++ b/packages/interloper-agent/tests/tools/test_collection.py @@ -153,3 +153,82 @@ def test_update_component_requires_a_change(store: FakeStore, ctx: ToolContext): result = collection.update_component(str(store.component.id), tool_context=ctx) assert result["status"] == "error" assert store.update_kwargs is None + + +# -- _source_relations -------------------------------------------------------- + + +class _RelationsComponentStore: + """Serves fixed connection/destination rows by id for _source_relations tests.""" + + def __init__(self, rows: dict[Any, Any]): + """Bind the fake facet to the rows it serves. + + Args: + rows: Maps component UUID to the row ``get`` returns for it. + """ + self._rows = rows + + def get(self, component_id: Any, *, kind: str | None = None) -> Any: + return self._rows[component_id] + + +class _RelationsStore: + """Presents the ``components`` facet ``_source_relations`` reaches for.""" + + def __init__(self, rows: dict[Any, Any]): + """Bind the fake store to the rows its components facet serves. + + Args: + rows: Maps component UUID to the row ``components.get`` returns. + """ + self.components = _RelationsComponentStore(rows) + + +def _relation_row(**overrides: Any) -> Any: + defaults: dict[str, Any] = {"id": uuid4(), "org_id": ORG_ID, "key": "facebook_ads_connection"} + return SimpleNamespace(**{**defaults, **overrides}) + + +def test_source_relations_binds_connection_by_name(): + connection = _relation_row(key="facebook_ads_connection") + destination = _relation_row(key="bigquery") + store = _RelationsStore({connection.id: connection, destination.id: destination}) + defn = { + "relations": { + "connection": {"kind": "connection", "key": "facebook_ads_connection", "optional": False}, + "destinations": {"kind": "destination", "many": True, "optional": True}, + } + } + relations, error = collection._source_relations( + store, ORG_ID, defn, "facebook_ads", str(connection.id), [str(destination.id)] + ) + assert error is None + assert relations == {"connection": [connection.id], "destinations": [destination.id]} + + +def test_source_relations_rejects_connection_with_mismatched_key(): + connection = _relation_row(key="bing_ads_connection") + store = _RelationsStore({connection.id: connection}) + defn = {"relations": {"connection": {"kind": "connection", "key": "facebook_ads_connection", "optional": False}}} + relations, error = collection._source_relations(store, ORG_ID, defn, "facebook_ads", str(connection.id), None) + assert relations is None + assert error is not None + assert "does not fit any relation of 'facebook_ads'" in error["error"] + + +def test_source_relations_requires_missing_connection(): + store = _RelationsStore({}) + defn = {"relations": {"connection": {"kind": "connection", "key": "facebook_ads_connection", "optional": False}}} + relations, error = collection._source_relations(store, ORG_ID, defn, "facebook_ads", None, None) + assert relations is None + assert error is not None + assert "requires a 'facebook_ads_connection' as 'connection'" in error["error"] + + +def test_source_relations_allows_optional_connection_unbound(): + store = _RelationsStore({}) + defn = {"relations": {"connection": {"kind": "connection", "key": "facebook_ads_connection", "optional": True}}} + relations, error = collection._source_relations(store, ORG_ID, defn, "facebook_ads", None, None) + assert error is None + assert relations == {"destinations": []} From 49e3efaab6d42adec42bd44bc1b3fa9a02c553a8 Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 03:57:13 +0200 Subject: [PATCH 10/19] refactor(scheduler)!: hooks match on the watches relation; the DAG joins read-only upstreams By Digitl --- .../src/interloper_scheduler/executor.py | 41 ++----- .../src/interloper_scheduler/hooks.py | 2 +- .../interloper-scheduler/tests/test_cron.py | 2 +- .../tests/test_executor.py | 112 +++++++----------- .../interloper-scheduler/tests/test_hooks.py | 22 ++-- packages/interloper-slack/tests/test_hook.py | 4 +- 6 files changed, 68 insertions(+), 115 deletions(-) diff --git a/packages/interloper-scheduler/src/interloper_scheduler/executor.py b/packages/interloper-scheduler/src/interloper_scheduler/executor.py index f7dc972f..19039bd2 100644 --- a/packages/interloper-scheduler/src/interloper_scheduler/executor.py +++ b/packages/interloper-scheduler/src/interloper_scheduler/executor.py @@ -1,11 +1,12 @@ """Run executor: the envelope that assembles a run's operations and drives the runner. The executor owns the run lifecycle — load, mark running, trace, terminal -status, failure event — and the platform side of graph assembly: flattening -the hydrated target workload into its operations, joining upstream -dependencies from the store as non-materializable context, and skipping the -retry lineage's prior successes. The runner executes the operations; their -returned effects (config and state fields) are applied generically to each +status, failure event — and skipping the retry lineage's prior successes. +Flattening the hydrated target workload into its operations and joining +bound upstreams the run itself does not materialize are the framework's own +concern (``Workload.operations()``, ``DAG._include_read_only_upstreams()``), +not the executor's. The runner executes the operations; their returned +effects (config and state fields) are applied generically to each operation's component row after the run. """ @@ -14,7 +15,7 @@ import asyncio import datetime as dt import logging -from typing import Any, cast +from typing import Any from uuid import UUID import interloper as il @@ -117,7 +118,6 @@ def execute(self, run_id: UUID) -> bool: self._store.runs.complete(run_id, success=True) return True - self._resolve_upstream(operations) if retry_of: successes = self._prior_successes(retry_of) for operation in operations: @@ -161,33 +161,6 @@ def _mark_running(session: Session, db_run: Run) -> None: session.add(db_run) session.commit() - def _resolve_upstream(self, operations: list[il.Operation]) -> None: - """Add transitive upstream dependencies to *operations* as non-materializable. - - Platform-side graph assembly: hydrated nodes carry their dependencies - as row ids, so the walk loads each unseen id from the store and - follows the dependencies it declares in turn. Joined upstream nodes - are read from their destinations, never recomputed. - - Args: - operations: The nodes to walk from, extended in place. - """ - visited = {operation.id for operation in operations} - frontier = list(operations) - while frontier: - next_frontier: list[il.Operation] = [] - for operation in frontier: - for upstream_ids in operation.upstreams.values(): - for dependency_id in upstream_ids: - if dependency_id in visited: - continue - visited.add(dependency_id) - upstream = cast(il.Asset, self._store.components.load(UUID(dependency_id))) - upstream.materializable = False - operations.append(upstream) - next_frontier.append(upstream) - frontier = next_frontier - def _prior_successes(self, retry_of: UUID) -> set[UUID]: """Node row ids that already succeeded in the retry lineage. diff --git a/packages/interloper-scheduler/src/interloper_scheduler/hooks.py b/packages/interloper-scheduler/src/interloper_scheduler/hooks.py index a764e804..0e252fb3 100644 --- a/packages/interloper-scheduler/src/interloper_scheduler/hooks.py +++ b/packages/interloper-scheduler/src/interloper_scheduler/hooks.py @@ -188,7 +188,7 @@ def _matching_hooks(self, session: Session, run: Run, target: Component) -> list .join(ComponentRelation, onclause=ComponentRelation.src_id == Component.id) # ty: ignore[invalid-argument-type] .where(Component.kind == "hook") .where(Component.org_id == run.org_id) - .where(ComponentRelation.type == "watch") + .where(ComponentRelation.name == "watches") .where(col(ComponentRelation.dst_id).in_(watched_ids)) .distinct() ).all() diff --git a/packages/interloper-scheduler/tests/test_cron.py b/packages/interloper-scheduler/tests/test_cron.py index 1c410abd..fff43652 100644 --- a/packages/interloper-scheduler/tests/test_cron.py +++ b/packages/interloper-scheduler/tests/test_cron.py @@ -269,7 +269,7 @@ def _job_targeting(store: Store, *source_keys: str, config: dict[str, Any]) -> U key="cron_job", name="J", config=config, - relations={"target": [(tid, "") for tid in targets]}, + relations={"targets": targets}, ) with Session(store.engine) as session: db_job = session.get(Component, row.id) diff --git a/packages/interloper-scheduler/tests/test_executor.py b/packages/interloper-scheduler/tests/test_executor.py index b6692484..61f4deca 100644 --- a/packages/interloper-scheduler/tests/test_executor.py +++ b/packages/interloper-scheduler/tests/test_executor.py @@ -414,91 +414,71 @@ def test_empty_effects_write_nothing(self, hydrated_asset: il.Asset) -> None: assert store.stamped == [] -class TestResolveUpstream: - """Upstream dependencies join the graph as read-only context.""" +class _UpstreamFixture(il.Asset): + """Plain asset fixture standing in for a hydrated upstream.""" - @staticmethod - def _asset(dependencies: dict[str, str] | None = None) -> il.Asset: - @il.asset() - def node() -> list[dict[str, Any]]: - return [{"x": 1}] - - instance = node(id=str(uuid4()), destinations=[il.MemoryDestination()]) - if dependencies: - instance.dependencies = dependencies - return instance + def data(self) -> list[dict[str, Any]]: + """Return one row. - def test_a_dependency_is_loaded_and_made_non_materializable(self) -> None: - # Joined upstream nodes are read from their destinations, never recomputed. - upstream = self._asset() - target = self._asset({"up": upstream.id}) - store = _RecordingStore(None) - store.components = SimpleNamespace(load=lambda _id: upstream) - executor = _executor(store) - operations: list[il.Operation] = [target] + Returns: + One row. + """ + return [{"x": 1}] - executor._resolve_upstream(operations) - assert operations == [target, upstream] - assert upstream.materializable is False +class _DownstreamFixture(il.Asset): + """Asset fixture whose optional relation accepts any asset.""" - def test_the_walk_is_transitive(self) -> None: - grandparent = self._asset() - parent = self._asset({"up": grandparent.id}) - target = self._asset({"up": parent.id}) - by_id = {parent.id: parent, grandparent.id: grandparent} - store = _RecordingStore(None) - store.components = SimpleNamespace(load=lambda component_id: by_id[str(component_id)]) - executor = _executor(store) - operations: list[il.Operation] = [target] + upstream: il.Asset | None = il.Relation("asset", optional=True) - executor._resolve_upstream(operations) + def data(self) -> list[dict[str, Any]]: + """Return one row. - assert [operation.id for operation in operations] == [target.id, parent.id, grandparent.id] + Returns: + One row. + """ + return [{"y": 1}] - def test_a_shared_dependency_is_loaded_once(self) -> None: - shared = self._asset() - first = self._asset({"up": shared.id}) - second = self._asset({"up": shared.id}) - loads: list[str] = [] - store = _RecordingStore(None) - def load(component_id: Any) -> il.Asset: - loads.append(str(component_id)) - return shared +class TestUpstreamJoinsReadOnly: + """A bound upstream the run itself does not materialize joins the DAG read-only. - store.components = SimpleNamespace(load=load) - executor = _executor(store) - operations: list[il.Operation] = [first, second] + The hydrator (Task 4) binds the upstream on the hydrated component + directly; the DAG (phase 1 Task 6) is what joins it as a read-only node. + The executor no longer walks anything itself, so this exercises the real + ``RunExecutor.execute`` path, capturing the assembled DAG through + ``_run_dag`` to inspect what it built. + """ - executor._resolve_upstream(operations) + def test_a_bound_upstream_is_joined_and_made_non_materializable( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + il.MemoryDestination.clear() + upstream = _UpstreamFixture(id=str(uuid4()), destinations=[il.MemoryDestination()]) + target = _DownstreamFixture( + id=str(uuid4()), destinations=[il.MemoryDestination()], upstream=upstream + ) - assert loads == [shared.id] + run = Run(id=uuid4(), component_id=uuid4(), org_id=uuid4(), status="dispatched") + monkeypatch.setattr(executor_module, "Session", lambda _engine: _FakeSession(run)) - def test_a_dependency_already_in_the_graph_is_not_reloaded(self) -> None: - upstream = self._asset() - target = self._asset({"up": upstream.id}) - store = _RecordingStore(None) - store.components = SimpleNamespace( - load=lambda _id: pytest.fail("an in-graph dependency must not be reloaded") - ) - executor = _executor(store) - operations: list[il.Operation] = [target, upstream] + built: list[il.DAG] = [] + real_run_dag = RunExecutor._run_dag - executor._resolve_upstream(operations) + def _capturing_run_dag(self: RunExecutor, dag: il.DAG, *args: Any, **kwargs: Any) -> il.RunResult: + built.append(dag) + return real_run_dag(self, dag, *args, **kwargs) - assert len(operations) == 2 + monkeypatch.setattr(RunExecutor, "_run_dag", _capturing_run_dag) - def test_no_dependencies_is_a_no_op(self) -> None: - target = self._asset() - store = _RecordingStore(None) - store.components = SimpleNamespace(load=lambda _id: pytest.fail("nothing to load")) + store = _RecordingStore(target) executor = _executor(store) - operations: list[il.Operation] = [target] - executor._resolve_upstream(operations) + assert executor.execute(run.id) is True - assert operations == [target] + (dag,) = built + assert dag.operation_map[upstream.id].materializable is False + assert dag.predecessors[target.id] == [upstream.id] class TestRetrySkipsPriorSuccesses: diff --git a/packages/interloper-scheduler/tests/test_hooks.py b/packages/interloper-scheduler/tests/test_hooks.py index 5c989680..79dbee06 100644 --- a/packages/interloper-scheduler/tests/test_hooks.py +++ b/packages/interloper-scheduler/tests/test_hooks.py @@ -129,7 +129,7 @@ def test_trigger_hook_cascades_with_partition(self, store: Store): hook = store.components.create( _ORG, kind="hook", key="trigger_hook", name="Cascade", config={"events": ["run_completed"]}, - relations={"watch": [(leaf.id, "")], "target": [(root.id, "")]}, + relations={"watches": [leaf.id], "targets": [root.id]}, ) run = _terminal_run(store, leaf.id) @@ -155,7 +155,7 @@ def test_claim_prevents_refiring(self, store: Store): store.components.create( _ORG, kind="hook", key="trigger_hook", name="Cascade", config={"events": ["run_completed"]}, - relations={"watch": [(leaf.id, "")], "target": [(root.id, "")]}, + relations={"watches": [leaf.id], "targets": [root.id]}, ) _terminal_run(store, leaf.id) @@ -174,7 +174,7 @@ def test_event_type_mismatch_does_not_fire(self, store: Store): store.components.create( _ORG, kind="hook", key="trigger_hook", name="OnFailureOnly", config={"events": ["run_failed"]}, - relations={"watch": [(leaf.id, "")], "target": [(root.id, "")]}, + relations={"watches": [leaf.id], "targets": [root.id]}, ) _terminal_run(store, leaf.id, status="success") @@ -200,7 +200,7 @@ def fake_post(url: str, **kwargs: Any) -> httpx.Response: store.components.create( _ORG, kind="hook", key="webhook_hook", name="OnAnyAsset", config={"events": ["run_completed"], "url": "https://example.test/n"}, - relations={"watch": [(source.id, "")]}, + relations={"watches": [source.id]}, ) _terminal_run(store, child.id) @@ -220,7 +220,7 @@ def boom(*args: Any, **kwargs: Any) -> None: store.components.create( _ORG, kind="hook", key="webhook_hook", name="Notify", config={"events": ["run_failed"], "url": "https://example.test/x"}, - relations={"watch": [(source.id, "")]}, + relations={"watches": [source.id]}, ) run = _terminal_run(store, source.id, status="failed") @@ -240,7 +240,7 @@ def test_self_targeting_trigger_is_refused(self, store: Store): store.components.create( _ORG, kind="hook", key="trigger_hook", name="Ouroboros", config={"events": ["run_completed"]}, - relations={"watch": [(source.id, "")], "target": [(source.id, "")]}, + relations={"watches": [source.id], "targets": [source.id]}, ) _terminal_run(store, source.id) @@ -259,7 +259,7 @@ def test_metadata_carries_component_identity(self, store: Store, monkeypatch: py store.components.create( _ORG, kind="hook", key="webhook_hook", name="Notify", config={"events": ["run_completed"], "url": "https://example.test/n"}, - relations={"watch": [(source.id, "")]}, + relations={"watches": [source.id]}, ) _terminal_run(store, source.id) @@ -278,7 +278,7 @@ def test_metadata_falls_back_to_key_when_unnamed(self, store: Store, monkeypatch store.components.create( _ORG, kind="hook", key="webhook_hook", name="Notify", config={"events": ["run_completed"], "url": "https://example.test/n"}, - relations={"watch": [(source.id, "")]}, + relations={"watches": [source.id]}, ) # components.create always derives a name, so the nullable column is # the only way the fallback is reachable. @@ -301,7 +301,7 @@ def test_failure_metadata_carries_the_run_error(self, store: Store, monkeypatch: store.components.create( _ORG, kind="hook", key="webhook_hook", name="Notify", config={"events": ["run_failed"], "url": "https://example.test/n"}, - relations={"watch": [(source.id, "")]}, + relations={"watches": [source.id]}, ) run = _terminal_run(store, source.id, status="failed") # The error text lives on the run's event rows, not the run itself. @@ -318,7 +318,7 @@ def test_success_metadata_has_no_error(self, store: Store, monkeypatch: pytest.M store.components.create( _ORG, kind="hook", key="webhook_hook", name="Notify", config={"events": ["run_completed"], "url": "https://example.test/n"}, - relations={"watch": [(source.id, "")]}, + relations={"watches": [source.id]}, ) _terminal_run(store, source.id) @@ -336,7 +336,7 @@ def test_chain_to_unwatched_target_is_allowed(self, store: Store): store.components.create( _ORG, kind="hook", key="trigger_hook", name="Chain", config={"events": ["run_completed"]}, - relations={"watch": [(leaf.id, "")], "target": [(root.id, "")]}, + relations={"watches": [leaf.id], "targets": [root.id]}, ) _terminal_run(store, leaf.id) diff --git a/packages/interloper-slack/tests/test_hook.py b/packages/interloper-slack/tests/test_hook.py index 0eac7621..d84b818d 100644 --- a/packages/interloper-slack/tests/test_hook.py +++ b/packages/interloper-slack/tests/test_hook.py @@ -97,7 +97,7 @@ def test_http_error_propagates(self, slack): class TestDefinition: def test_declares_a_connection_slot(self): - assert SlackHook.resource_types["connection"] is SlackConnection + assert SlackHook.relations["connection"].target is SlackConnection def test_defaults_to_failures_only(self): assert _hook().events == ["run_failed"] @@ -105,4 +105,4 @@ def test_defaults_to_failures_only(self): def test_is_a_catalogued_hook(self): definition = SlackHook.definition() assert (definition.kind, definition.key, definition.name) == ("hook", "slack_hook", "Slack") - assert definition.relations["resource"].slots["connection"].key == "slack_connection" + assert definition.relations["connection"].key == "slack_connection" From 5d9f544d91ae3d4af0b3eb91d31627933639bfcc Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 04:11:24 +0200 Subject: [PATCH 11/19] chore(platform): migration round trip verified; sweep retired names and em-dashes By Digitl --- .../tests/routes/test_components.py | 4 ++-- .../migrations/versions/017_relation_name.py | 16 ++++++++++++++++ .../src/interloper_db/store/components.py | 4 ++-- .../src/interloper_scheduler/executor.py | 2 +- packages/interloper-slack/tests/test_hook.py | 2 +- 5 files changed, 22 insertions(+), 6 deletions(-) diff --git a/packages/interloper-api/tests/routes/test_components.py b/packages/interloper-api/tests/routes/test_components.py index edca70a7..df072bbc 100644 --- a/packages/interloper-api/tests/routes/test_components.py +++ b/packages/interloper-api/tests/routes/test_components.py @@ -526,7 +526,7 @@ def test_no_components_is_an_empty_list(self, crud_client: TestClient) -> None: class TestListRelations: - """``GET /components/relations`` — optionally narrowed by name and kinds.""" + """``GET /components/relations``, optionally narrowed by name and kinds.""" def test_lists_every_relation(self, crud_client: TestClient, crud_store: CrudStore) -> None: source_id, destination_id = uuid4(), uuid4() @@ -967,7 +967,7 @@ def test_bindings_of_none_stay_none(self) -> None: class TestComponentResponseRelations: - """``ComponentResponse.relations`` — keyed by name, dst_kind carried along.""" + """``ComponentResponse.relations``, keyed by name, dst_kind carried along.""" def test_component_response_relations_keyed_by_name(self) -> None: connection_id, destination_id = uuid4(), uuid4() diff --git a/packages/interloper-db/src/interloper_db/migrations/versions/017_relation_name.py b/packages/interloper-db/src/interloper_db/migrations/versions/017_relation_name.py index 7dcd8d39..578f5171 100644 --- a/packages/interloper-db/src/interloper_db/migrations/versions/017_relation_name.py +++ b/packages/interloper-db/src/interloper_db/migrations/versions/017_relation_name.py @@ -12,6 +12,16 @@ final ``DELETE ... WHERE name IS NULL`` is defensive only; production has no rows outside that vocabulary, so it is a no-op there. +The downgrade maps every asset-kind row back to ``type = 'upstream'``, so a +pre-017 ``dependency`` type is never recovered: that rename never shipped, so +no row in the wild carries it. In upgrade(), ``_PLURAL`` entries match by +``type``; in downgrade(), they match by ``name``. + +``upgrade`` is a no-op on a database whose ``component_relations`` table has +no ``type`` column: ``create_all()`` always provisions the table from the +current model, so a fresh database already has the final (``name``) shape +before Alembic runs at all. + Revision ID: 017 Revises: 016 """ @@ -32,6 +42,12 @@ def upgrade() -> None: + bind = op.get_bind() + columns = {column["name"] for column in sa.inspect(bind).get_columns(_TABLE)} + if "type" not in columns: + # Fresh database: create_all() already produced the final (name) shape. + return + op.add_column(_TABLE, sa.Column("name", sa.String(), nullable=True)) op.execute("UPDATE component_relations SET name = slot WHERE type IN ('resource', 'dependency', 'upstream')") for type_, name in _PLURAL.items(): diff --git a/packages/interloper-db/src/interloper_db/store/components.py b/packages/interloper-db/src/interloper_db/store/components.py index f0633555..28e73f64 100644 --- a/packages/interloper-db/src/interloper_db/store/components.py +++ b/packages/interloper-db/src/interloper_db/store/components.py @@ -116,7 +116,7 @@ def create( ``False`` opts into plaintext storage. children: Source kinds only — which child asset keys to enable (``None`` enables all the catalog class declares). - relations: ``{name: [dst_id, …]}`` — replaced per name. + relations: ``{name: [dst_id, …]}``, replaced per name. Returns: The created component row, eager-loaded. @@ -207,7 +207,7 @@ def update( ``False`` opts into plaintext storage. children: Source kinds only — the exact set of child asset keys to keep enabled. - relations: ``{name: [dst_id, …]}`` — replaced per name. + relations: ``{name: [dst_id, …]}``, replaced per name. Returns: The updated component row, eager-loaded. diff --git a/packages/interloper-scheduler/src/interloper_scheduler/executor.py b/packages/interloper-scheduler/src/interloper_scheduler/executor.py index 19039bd2..c94772d1 100644 --- a/packages/interloper-scheduler/src/interloper_scheduler/executor.py +++ b/packages/interloper-scheduler/src/interloper_scheduler/executor.py @@ -1,7 +1,7 @@ """Run executor: the envelope that assembles a run's operations and drives the runner. The executor owns the run lifecycle — load, mark running, trace, terminal -status, failure event — and skipping the retry lineage's prior successes. +status, failure event, and skipping the retry lineage's prior successes. Flattening the hydrated target workload into its operations and joining bound upstreams the run itself does not materialize are the framework's own concern (``Workload.operations()``, ``DAG._include_read_only_upstreams()``), diff --git a/packages/interloper-slack/tests/test_hook.py b/packages/interloper-slack/tests/test_hook.py index d84b818d..65edc99e 100644 --- a/packages/interloper-slack/tests/test_hook.py +++ b/packages/interloper-slack/tests/test_hook.py @@ -96,7 +96,7 @@ def test_http_error_propagates(self, slack): class TestDefinition: - def test_declares_a_connection_slot(self): + def test_declares_a_connection_relation(self): assert SlackHook.relations["connection"].target is SlackConnection def test_defaults_to_failures_only(self): From 90be2f02887e7abedd1d6f74548dbcc91c192be3 Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 04:45:20 +0200 Subject: [PATCH 12/19] feat(agent): expose bind_relation and unbind_relation on the collection agent The toolkit's two write functions were reachable from no surface: the MCP server is deliberately read-only, and the ADK agent never registered them, so a relation could only be rebound through the app. They now sit in the collection agent's tool list beside its other write tools, re-exported from the toolkit package as its whole write surface, with the read-only guard on the MCP server's tool list widened to catch them. The collection instruction and the update_component copy said rebinding a relation happens in the app; both now point at the tools instead. By Digitl --- .../src/interloper_agent/agent.py | 7 +++- .../src/interloper_agent/prompts.py | 10 ++++- .../src/interloper_agent/tools/collection.py | 39 ++++++++++++++++++- packages/interloper-agent/tests/test_agent.py | 21 ++++++++++ packages/interloper-mcp/tests/test_tools.py | 2 +- .../src/interloper_toolkit/__init__.py | 11 ++++-- .../src/interloper_toolkit/collection.py | 7 +++- 7 files changed, 86 insertions(+), 11 deletions(-) diff --git a/packages/interloper-agent/src/interloper_agent/agent.py b/packages/interloper-agent/src/interloper_agent/agent.py index 71305a6c..ba7ebda1 100644 --- a/packages/interloper-agent/src/interloper_agent/agent.py +++ b/packages/interloper-agent/src/interloper_agent/agent.py @@ -118,8 +118,9 @@ def _catalog_tools() -> list: "The organisation's collection of component instances: lists their sources, connections, and " "destinations, checks connection health, sets up new connections via the app's secure form " "(never collecting credentials in chat), creates sources conversationally — resolving " - "provider-backed options like the account to use through an existing connection — and edits " - "existing components (rename, config changes, a source's enabled assets)." + "provider-backed options like the account to use through an existing connection — edits " + "existing components (rename, config changes, a source's enabled assets), and binds or " + "unbinds a component's relations by name (a source's connection, a job's targets)." ), instruction=with_current_time(COLLECTION_INSTRUCTION), tools=[ @@ -131,6 +132,8 @@ def _catalog_tools() -> list: collection.create_source, collection.create_sources, collection.update_component, + collection.bind_relation, + collection.unbind_relation, collection.create_job, interaction.request_user_selection, interaction.request_confirmation, diff --git a/packages/interloper-agent/src/interloper_agent/prompts.py b/packages/interloper-agent/src/interloper_agent/prompts.py index 661abe10..a3d6a467 100644 --- a/packages/interloper-agent/src/interloper_agent/prompts.py +++ b/packages/interloper-agent/src/interloper_agent/prompts.py @@ -110,7 +110,7 @@ 6. Recap with request_confirmation (what and how many; accounts, assets, connection, destination — "None" when none), then create_sources on confirm. Report per-account failures and any unresolved cross-source - requirements (those are wired in the app). + requirements, and offer to bind them once the user names the upstream. 7. Offer a schedule: recap the job (name, cadence in words, targets) and create_job on confirm. @@ -124,6 +124,14 @@ just the delta. Connection credentials are never edited in chat — offer a rename, or point the user to the app (or a fresh connection via the secure form) for credential changes. + +Bind what a component points at with bind_relation, and detach it with +unbind_relation: both take the component's id, the relation name its class +declares (list_components names the component, the catalog specialist the +names it declares), and the target's id. A single-valued name repoints, so +it needs no unbind first; a non-optional one cannot be emptied, only +repointed. Recap the component, the name, and old → new, and act only on +confirmation. """ + PRESENTATION CATALOG_CONSULT_INSTRUCTION = """\ diff --git a/packages/interloper-agent/src/interloper_agent/tools/collection.py b/packages/interloper-agent/src/interloper_agent/tools/collection.py index 3212c168..09b73088 100644 --- a/packages/interloper-agent/src/interloper_agent/tools/collection.py +++ b/packages/interloper-agent/src/interloper_agent/tools/collection.py @@ -67,8 +67,8 @@ def update_component( the stored config is kept; pass null to reset a field to its default. Connection configs hold credentials and are never edited here: the user changes those in the app (renaming a connection is fine). Rebinding - relations (a source's connection, a job's targets) also happens in the - app, not through config. + relations (a source's connection, a job's targets) is not config either: + use bind_relation and unbind_relation. Args: component_id: UUID of the component, from list_components. @@ -144,6 +144,41 @@ def update_component( return {"status": "error", "error": str(e)} +# -- Relations (generic over kinds) ----------------------------------------------- + + +def bind_relation( + component_id: str, + name: str, + dst_id: str, + tool_context: ToolContext | None = None, +) -> dict[str, Any]: + # Thin ADK wrapper: the implementation (and LLM-facing docstring, adopted + # below) lives in the shared toolkit, which the MCP server deliberately + # does not register, since it writes. + return toolkit_collection.bind_relation(toolkit_ctx(tool_context), component_id, name, dst_id).model_dump( + mode="json" + ) + + +bind_relation.__doc__ = toolkit_collection.bind_relation.__doc__ + + +def unbind_relation( + component_id: str, + name: str, + dst_id: str, + tool_context: ToolContext | None = None, +) -> dict[str, Any]: + # Thin ADK wrapper: see bind_relation above. + return toolkit_collection.unbind_relation(toolkit_ctx(tool_context), component_id, name, dst_id).model_dump( + mode="json" + ) + + +unbind_relation.__doc__ = toolkit_collection.unbind_relation.__doc__ + + # -- Connection operations (kind-specific by nature) ------------------------------ diff --git a/packages/interloper-agent/tests/test_agent.py b/packages/interloper-agent/tests/test_agent.py index f866aef5..aa1eefce 100644 --- a/packages/interloper-agent/tests/test_agent.py +++ b/packages/interloper-agent/tests/test_agent.py @@ -3,11 +3,26 @@ import datetime from typing import cast +from google.adk.agents.base_agent import BaseAgent from google.adk.agents.readonly_context import ReadonlyContext from interloper_agent import agent as agent_module +def _tool_names(agent: BaseAgent) -> set[str]: + """The names an agent's registered tools carry. + + Args: + agent: The agent whose tool list is read. + + Returns: + One name per registered tool: a plain function's ``__name__``, or a + tool object's own ``name`` (an ``AgentTool``, say). + """ + tools = getattr(agent, "tools", []) + return {getattr(tool, "__name__", None) or getattr(tool, "name", "") for tool in tools} + + def test_with_current_time_appends_now(): provider = agent_module.with_current_time("BASE") text = provider(cast(ReadonlyContext, None)) @@ -32,3 +47,9 @@ def test_all_agents_carry_the_current_time(): text = instruction(cast(ReadonlyContext, None)) assert isinstance(text, str) assert "Current date and time:" in text + + +def test_the_collection_agent_registers_the_relation_write_tools(): + # The toolkit's only write functions: exposed here, never on the + # deliberately read-only MCP server. + assert {"bind_relation", "unbind_relation"} <= _tool_names(agent_module.collection_agent) diff --git a/packages/interloper-mcp/tests/test_tools.py b/packages/interloper-mcp/tests/test_tools.py index 8519fd47..52ec548c 100644 --- a/packages/interloper-mcp/tests/test_tools.py +++ b/packages/interloper-mcp/tests/test_tools.py @@ -40,7 +40,7 @@ async def test_only_read_only_tools_are_exposed(store: Store, catalog: il.Catalo names = {t.name for t in tools} assert len(names) == 20 - forbidden = {n for n in names if n.startswith(("trigger_", "toggle_", "create_", "request_"))} + forbidden = {n for n in names if n.startswith(("trigger_", "toggle_", "create_", "request_", "bind_", "unbind_"))} assert forbidden == set() assert {"list_jobs", "list_definitions", "get_full_lineage", "freshness_check"} <= names diff --git a/packages/interloper-toolkit/src/interloper_toolkit/__init__.py b/packages/interloper-toolkit/src/interloper_toolkit/__init__.py index eae28b1e..e747a16b 100644 --- a/packages/interloper-toolkit/src/interloper_toolkit/__init__.py +++ b/packages/interloper-toolkit/src/interloper_toolkit/__init__.py @@ -9,12 +9,15 @@ Almost every function here is read-only; the sole exceptions are :func:`interloper_toolkit.collection.bind_relation` and -:func:`interloper_toolkit.collection.unbind_relation`, which write. A -surface that must stay read-only (the MCP server's own registration is one) -never registers those two. +:func:`interloper_toolkit.collection.unbind_relation`, which write and are +re-exported here as this package's whole write surface. A surface that must +stay read-only (the MCP server's own registration is one) never registers +those two; the ADK agent, whose own write tools already live beside them, +does. """ +from interloper_toolkit.collection import bind_relation, unbind_relation from interloper_toolkit.context import ToolkitContext, serialize from interloper_toolkit.models import ToolError -__all__ = ["ToolError", "ToolkitContext", "serialize"] +__all__ = ["ToolError", "ToolkitContext", "bind_relation", "serialize", "unbind_relation"] diff --git a/packages/interloper-toolkit/src/interloper_toolkit/collection.py b/packages/interloper-toolkit/src/interloper_toolkit/collection.py index 74458c44..8945cdde 100644 --- a/packages/interloper-toolkit/src/interloper_toolkit/collection.py +++ b/packages/interloper-toolkit/src/interloper_toolkit/collection.py @@ -84,7 +84,8 @@ def bind_relation(ctx: ToolkitContext, component_id: str, name: str, dst_id: str Works on any kind: a source's connection, a job's watched assets, a destination target, whatever the component's own class declares under that name. A ``many`` name accumulates; a single-valued one repoints, so - rebinding it needs no prior unbind_relation call. + rebinding it needs no prior unbind_relation call. Recap what the binding + changes and get the user's explicit confirmation BEFORE calling this. Args: component_id: UUID of the component the relation originates from. @@ -102,6 +103,10 @@ def bind_relation(ctx: ToolkitContext, component_id: str, name: str, dst_id: str def unbind_relation(ctx: ToolkitContext, component_id: str, name: str, dst_id: str) -> UnbindResult | ToolError: """Detach one component from another under a declared relation name. + A non-optional relation cannot be emptied, only repointed with + bind_relation. Recap what the component loses and get the user's + explicit confirmation BEFORE calling this. + Args: component_id: UUID of the component the relation originates from. name: Relation name the edge is filed under. From 6b04683f91cf18756384c792a9d201bde8037189 Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 04:48:36 +0200 Subject: [PATCH 13/19] fix(db): a drifted child surfaces as ComponentDriftError; single-relation duplicates and drift are tested A source whose class dropped one of its assets kept the child row, and the hydrator read that child's relations against the vocabulary the narrowed class resolves: the rows the dropped asset legitimately holds then read as "relations its class does not declare", failing the whole source with a misleading HydrationError. The child set is now checked against the parent's own declaration before any child's relations are read, so the drift is reported as drift, naming the child and its source. Two hydration guards gain the tests they lacked: the new drift path, and two rows under a single-valued relation (hand-inserted, since the store's own writes repoint such a name instead of accumulating). Also: the components module docstring named a RelationMixin that does not exist (the store composes a RelationStore), remove() now states that it guards no undeclared name on purpose, so drifted rows can be cleared, and the emptiness guard in _sync_relations says why creation may leave a non-optional relation unbound while an update may not. By Digitl --- .../src/interloper_db/store/components.py | 6 ++- .../src/interloper_db/store/hydration.py | 44 ++++++++++++++++--- .../src/interloper_db/store/relations.py | 8 ++++ .../tests/store/test_components.py | 24 ++++++++++ .../tests/store/test_hydration.py | 27 ++++++++++++ 5 files changed, 102 insertions(+), 7 deletions(-) diff --git a/packages/interloper-db/src/interloper_db/store/components.py b/packages/interloper-db/src/interloper_db/store/components.py index 28e73f64..6e07dd08 100644 --- a/packages/interloper-db/src/interloper_db/store/components.py +++ b/packages/interloper-db/src/interloper_db/store/components.py @@ -13,8 +13,10 @@ class declares between its own assets. status cascades through the parent's. - **job**: hydration drift-checks every target before reconstruction. -Relation reads and writes live in the :class:`RelationMixin` layer this -mixin builds on — see :mod:`interloper_db.store.relations`. +Relation reads and writes are not here at all: this store composes a +:class:`~interloper_db.store.relations.RelationStore` and delegates to it, +so the acceptance rules a relation name carries live in one place (see +:mod:`interloper_db.store.relations`). """ from __future__ import annotations diff --git a/packages/interloper-db/src/interloper_db/store/hydration.py b/packages/interloper-db/src/interloper_db/store/hydration.py index 5abcf4d4..b1bbe6be 100644 --- a/packages/interloper-db/src/interloper_db/store/hydration.py +++ b/packages/interloper-db/src/interloper_db/store/hydration.py @@ -39,8 +39,9 @@ from interloper.catalog.base import Catalog from interloper.component import KINDS -from interloper.errors import CatalogKeyError, HydrationError, format_exception +from interloper.errors import CatalogKeyError, ComponentDriftError, HydrationError, format_exception from interloper.serializable import Spec +from interloper.source.base import SourceDefinition from sqlmodel import Session, select from interloper_db.models import Component, ComponentRelation @@ -146,7 +147,11 @@ def _build_init( under that name, a list when the declared relation is ``many`` and a single value otherwise. Children are the one non-relation contribution: they embed as the ``assets`` override map, since the - parent source is the unit of reconstruction. + parent source is the unit of reconstruction. A child the parent's + declaration has dropped is refused as drift by :meth:`_check_declared` + before its own relations are read: those rows are declared by the + class the child no longer belongs to, so reading them would report + the parent's drift as an undeclared relation name on the child. Args: session: Active DB session, used to read relations and children. @@ -196,13 +201,42 @@ class does not declare, or holds more than one row under a children = session.exec( select(Component).where(Component.parent_id == db_component.id).order_by(Component.created_at) # ty: ignore[invalid-argument-type] ).all() - if assets := { - child.key: {"id": str(child.id), **self._build_init(session, child, seen=seen)} for child in children - }: + assets: dict[str, Any] = {} + for child in children: + self._check_declared(db_component, child) + assets[child.key] = {"id": str(child.id), **self._build_init(session, child, seen=seen)} + if assets: init["assets"] = assets return init + def _check_declared(self, db_parent: Component, db_child: Component) -> None: + """Refuse a child row whose key its parent no longer declares. + + The same question :func:`~interloper_db.store.status.asset_status` + asks of an asset row, asked here of the parent's whole child set: the + catalog's own declaration of the parent is authoritative, and a child + outside it has drifted out of the source. A parent that does not + resolve as a source declares nothing to check against, and is left to + :meth:`_resolve_path` to report. + + Args: + db_parent: The owning component row, whose declaration decides. + db_child: The child row whose key is checked. + + Raises: + ComponentDriftError: If the parent's declaration does not name + the child's key. + """ + definition = self._catalog.get(db_parent.key) + if not isinstance(definition, SourceDefinition): + return + if db_child.key not in {asset.key for asset in definition.assets}: + raise ComponentDriftError( + f"Asset '{db_child.key}' ({db_child.id}) is no longer declared by source " + f"'{db_parent.key}' ({db_parent.id}); its catalog key has drifted." + ) + def _relations_by_name(self, session: Session, src_id: UUID | None) -> dict[str, list[ComponentRelation]]: """Group a component's outgoing relations by name, ordered stably. diff --git a/packages/interloper-db/src/interloper_db/store/relations.py b/packages/interloper-db/src/interloper_db/store/relations.py index 36723c54..b57f2a84 100644 --- a/packages/interloper-db/src/interloper_db/store/relations.py +++ b/packages/interloper-db/src/interloper_db/store/relations.py @@ -123,6 +123,11 @@ def remove(self, component_id: UUID, *, name: str, dst_id: UUID) -> None: A missing source propagates as a ``NotFoundError`` from :meth:`_lock`. + Unlike every other write path this takes no undeclared-name guard, + deliberately: a name the class no longer declares is exactly the row + someone needs to clear, and refusing it would leave the component + unloadable with no way out. + Args: component_id: Source component the relation originates from. name: Relation name the edge is filed under. @@ -222,6 +227,9 @@ def _sync_relations(self, session: Session, src: Component, bindings: dict[str, if not relation.many and len(dst_ids) > 1: raise ConfigError(f"'{src.key}'.{name} is single-valued and takes one target at a time") existing = self._rows(session, src.id, name) + # Gated on existing rows: creation may leave a non-optional name + # unbound, which only hydration refuses; an update that clears + # one is a removal. if existing and not dst_ids and not relation.optional: raise ConfigError( f"'{src.key}'.{name} is non-optional and cannot be emptied; " diff --git a/packages/interloper-db/tests/store/test_components.py b/packages/interloper-db/tests/store/test_components.py index 819d3cfa..e27a3b68 100644 --- a/packages/interloper-db/tests/store/test_components.py +++ b/packages/interloper-db/tests/store/test_components.py @@ -110,6 +110,18 @@ def data(self, context: il.ExecutionContext, rows: il.Upstream) -> list[dict]: return [] +class WireUpNarrowedSource(il.Source): + """``wire_up_source`` as a later release declares it: no ``totals``.""" + + key = "wire_up_source" + + class Rows(il.Asset): + """The one asset the narrowed class still declares.""" + + def data(self, context: il.ExecutionContext) -> list[dict]: + return [] + + class WireDownOptionalSource(il.Source): """Downstream source whose asset optionally reads ``wire_up_source.rows``.""" @@ -952,6 +964,18 @@ def test_an_asset_the_source_no_longer_declares_raises(self, component_db: Engin with pytest.raises(ComponentDriftError, match="is no longer declared by source"): store.components.load(orphan_id) + def test_a_child_the_class_dropped_drifts_the_whole_source(self, component_db: Engine): + # The dropped child's own relation rows are declared by the class it + # fell out of, so reading them would report this drift as an + # undeclared relation name on the child. + writer = Store(catalog=il.Catalog.from_assets([WireUpSource])) + source = writer.components.create(_ORG, kind="source", key="wire_up_source") + assert sorted(child.key for child in source.children) == ["rows", "totals"] + reader = Store(catalog=il.Catalog.from_assets([WireUpNarrowedSource])) + + with pytest.raises(ComponentDriftError, match="Asset 'totals' .* is no longer declared by source"): + reader.components.load(source.id) + class TestEnsureChildrenDrift: """A source whose key no longer resolves cannot have its children reconciled.""" diff --git a/packages/interloper-db/tests/store/test_hydration.py b/packages/interloper-db/tests/store/test_hydration.py index 3b941f3b..5e5176db 100644 --- a/packages/interloper-db/tests/store/test_hydration.py +++ b/packages/interloper-db/tests/store/test_hydration.py @@ -249,6 +249,33 @@ def test_a_relation_name_the_class_does_not_declare_is_an_actionable_error( with pytest.raises(HydrationError, match="has 'mystery' relations its class does not declare"): _init(store, shop.id) + def test_two_rows_under_a_single_valued_relation_is_an_actionable_error( + self, store: Store, component_db: Engine + ): + shop = store.components.create(_ORG, kind="source", key="shop") + # Hand-inserted: the store's own writes repoint a single-valued name + # instead of accumulating, so only a rogue writer produces this row. + connections = [ + store.components.create(_ORG, kind="connection", key="shop_connection", config={}, encrypted=False) + for _ in range(2) + ] + with Session(component_db) as session: + for connection in connections: + session.add( + ComponentRelation( + src_id=shop.id, + name="connection", + dst_id=connection.id, + org_id=_ORG, + src_kind="source", + dst_kind="connection", + ) + ) + session.commit() + + with pytest.raises(HydrationError, match="holds 2 rows under single-valued relation 'connection'"): + _init(store, shop.id) + class TestSourceRoundTrip: """Sources with child assets, intra-source deps, and overrides.""" From 580be7318eda3748eec70e150054bc084adc231f Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 04:49:54 +0200 Subject: [PATCH 14/19] test(agent): a source without connection relations rejects a connection The last deferred test of the branch: a definition declaring only destinations, handed a connection_id, reports that the connection fits no relation of the source rather than binding it anywhere. Sweep of what the earlier tasks left: the executor module docstring's remaining em-dash (on the one line the rewrite did not touch), a test name still saying "sibling upstreams" where the store binds sibling relations, and the migration 017 docstring, which listed neither destination among the types _PLURAL maps nor the empty-name rows its defensive DELETE drops. By Digitl --- .../interloper-agent/tests/tools/test_collection.py | 12 ++++++++++++ .../migrations/versions/017_relation_name.py | 11 +++++++---- .../interloper-db/tests/store/test_components.py | 2 +- .../src/interloper_scheduler/executor.py | 2 +- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/packages/interloper-agent/tests/tools/test_collection.py b/packages/interloper-agent/tests/tools/test_collection.py index f8f1e74d..4a90c2ab 100644 --- a/packages/interloper-agent/tests/tools/test_collection.py +++ b/packages/interloper-agent/tests/tools/test_collection.py @@ -226,6 +226,18 @@ def test_source_relations_requires_missing_connection(): assert "requires a 'facebook_ads_connection' as 'connection'" in error["error"] +def test_source_relations_rejects_a_connection_a_source_cannot_hold(): + connection = _relation_row() + store = _RelationsStore({connection.id: connection}) + defn = {"relations": {"destinations": {"kind": "destination", "many": True, "optional": True}}} + relations, error = collection._source_relations( + store, ORG_ID, defn, "static_source", str(connection.id), None + ) + assert relations is None + assert error is not None + assert "does not fit any relation of 'static_source'" in error["error"] + + def test_source_relations_allows_optional_connection_unbound(): store = _RelationsStore({}) defn = {"relations": {"connection": {"kind": "connection", "key": "facebook_ads_connection", "optional": True}}} diff --git a/packages/interloper-db/src/interloper_db/migrations/versions/017_relation_name.py b/packages/interloper-db/src/interloper_db/migrations/versions/017_relation_name.py index 578f5171..138355c8 100644 --- a/packages/interloper-db/src/interloper_db/migrations/versions/017_relation_name.py +++ b/packages/interloper-db/src/interloper_db/migrations/versions/017_relation_name.py @@ -7,10 +7,13 @@ relation is single-valued is a class rule the store enforces, so the partial unique index on resource slots goes. -The four types the old vocabulary ever wrote (``resource``, ``dependency``/ -``upstream``, ``target``, ``watch``) are backfilled into ``name`` below; the -final ``DELETE ... WHERE name IS NULL`` is defensive only; production has no -rows outside that vocabulary, so it is a no-op there. +The types the old vocabulary ever wrote (``resource``, ``dependency``/ +``upstream``, and the three ``_PLURAL`` maps: ``destination``, ``target``, +``watch``) are backfilled into ``name`` below. The final ``DELETE`` is +defensive only: it drops a row left with a NULL ``name`` (a ``type`` outside +that vocabulary) or an empty one (a ``resource`` row whose ``slot`` was +never filled, which the store had no path to write). Production holds +neither, so it is a no-op there. The downgrade maps every asset-kind row back to ``type = 'upstream'``, so a pre-017 ``dependency`` type is never recovered: that rename never shipped, so diff --git a/packages/interloper-db/tests/store/test_components.py b/packages/interloper-db/tests/store/test_components.py index e27a3b68..2ebb0380 100644 --- a/packages/interloper-db/tests/store/test_components.py +++ b/packages/interloper-db/tests/store/test_components.py @@ -374,7 +374,7 @@ def demo_store(self, component_db: Engine) -> Store: """ return Store(catalog=il.Catalog.from_assets([DemoSource])) - def test_create_source_binds_sibling_upstreams(self, store: Store): + def test_create_source_binds_sibling_relations(self, store: Store): source = store.components.create(_ORG, kind="source", key="wire_up_source") rows = store.relations.list_all(_ORG, src_kind="asset", dst_kind="asset") diff --git a/packages/interloper-scheduler/src/interloper_scheduler/executor.py b/packages/interloper-scheduler/src/interloper_scheduler/executor.py index c94772d1..61970e8a 100644 --- a/packages/interloper-scheduler/src/interloper_scheduler/executor.py +++ b/packages/interloper-scheduler/src/interloper_scheduler/executor.py @@ -1,6 +1,6 @@ """Run executor: the envelope that assembles a run's operations and drives the runner. -The executor owns the run lifecycle — load, mark running, trace, terminal +The executor owns the run lifecycle: load, mark running, trace, terminal status, failure event, and skipping the retry lineage's prior successes. Flattening the hydrated target workload into its operations and joining bound upstreams the run itself does not materialize are the framework's own From c116786e7101b8af2e81d0bc76ea647bcab657da Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 04:52:03 +0200 Subject: [PATCH 15/19] style(agent): drop the em-dashes from the collection agent's description The clause the relation tools were appended to used a pair of em-dashes; parentheses carry the same aside. By Digitl --- packages/interloper-agent/src/interloper_agent/agent.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/interloper-agent/src/interloper_agent/agent.py b/packages/interloper-agent/src/interloper_agent/agent.py index ba7ebda1..137fa4d8 100644 --- a/packages/interloper-agent/src/interloper_agent/agent.py +++ b/packages/interloper-agent/src/interloper_agent/agent.py @@ -117,8 +117,8 @@ def _catalog_tools() -> list: description=( "The organisation's collection of component instances: lists their sources, connections, and " "destinations, checks connection health, sets up new connections via the app's secure form " - "(never collecting credentials in chat), creates sources conversationally — resolving " - "provider-backed options like the account to use through an existing connection — edits " + "(never collecting credentials in chat), creates sources conversationally (resolving " + "provider-backed options like the account to use through an existing connection), edits " "existing components (rename, config changes, a source's enabled assets), and binds or " "unbinds a component's relations by name (a source's connection, a job's targets)." ), From 030392df24c10778dfe1bbdfaaf1de0f16326a15 Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 07:17:43 +0200 Subject: [PATCH 16/19] fix(core)!: an optional inferred upstream detaches when its target is deleted Deletion of a relation's target is governed by on_delete alone; optional governs only whether a relation may be left unbound or empty. An optional data() upstream parameter already tolerates a missing leg, so the inferred relation now declares on_delete="detach" and no longer blocks deleting the upstream through the platform's delete guard. A parameter naming a component class keeps the default block: a connection or config a read consumes stays undeletable while it is bound, optional or not. By Digitl --- docs/extending/components.md | 4 +-- docs/guide/dependencies.md | 5 ++- docs/guide/resources.md | 3 ++ .../src/interloper/asset/base.py | 13 ++++++-- .../src/interloper/component/relation.py | 9 +++-- .../interloper-core/tests/asset/test_base.py | 33 +++++++++++++++++++ 6 files changed, 59 insertions(+), 8 deletions(-) diff --git a/docs/extending/components.md b/docs/extending/components.md index 6498139f..c1067a3d 100644 --- a/docs/extending/components.md +++ b/docs/extending/components.md @@ -80,9 +80,9 @@ destinations a source writes to, a job's targets, an upstream asset. | `kind` | The component kind, or kinds, the relation may point at. | | `key` | The keys it narrows to: an exact key, `source.asset`, `*.asset`, a list, or `""` for any key of those kinds. | | `many` | Whether it binds several components at once. | -| `optional` | Whether it may stay unbound. Says nothing about data. | +| `optional` | Whether it may stay unbound, or be emptied once bound. Says nothing about data, and nothing about deletion. | | `default` | A zero-argument factory producing the value an unbound relation resolves to. | -| `on_delete` | What deleting the target does to the referrer: `block` (default, for consumption relations) or `detach` (for orchestration pointers such as a job's targets or a hook's watches). | +| `on_delete` | What deleting a bound target does, and the only field that decides it: `block` (the default) refuses the deletion while this relation holds the target, `detach` lets it through and drops the binding. Orchestration pointers (a job's `targets`, a hook's `watches`) and inferred optional upstreams declare `detach`; consumption relations keep `block`, an optional one included. | Two attributes are derived, not declared: `name` is stamped from the attribute the relation is declared under, and `target` is the class the `il.Relation(cls)` shorthand was written with, diff --git a/docs/guide/dependencies.md b/docs/guide/dependencies.md index a4a416d9..e420e57f 100644 --- a/docs/guide/dependencies.md +++ b/docs/guide/dependencies.md @@ -30,7 +30,10 @@ class Shop(il.Source): The parameter name is the relation name and the bare asset key it expects, so `orders: il.Upstream` declares `il.Relation("asset", "orders")`, a sibling of the same source -instance. A `None` default makes the relation **optional**, meaning its wiring may be absent: +instance. A `None` default makes the relation **optional**, meaning its wiring may be absent, and +declares `on_delete="detach"` with it: a parameter that tolerates a missing leg has no claim on +the upstream, so deleting that upstream is allowed and simply drops the wiring, where a required +upstream refuses the deletion while it is bound. ```py @il.asset(partitioning=il.TimePartitionConfig(column="date")) diff --git a/docs/guide/resources.md b/docs/guide/resources.md index 10b80093..0aff52f9 100644 --- a/docs/guide/resources.md +++ b/docs/guide/resources.md @@ -79,6 +79,9 @@ class WarehouseDestination(il.Destination): `il.Relation(ReportingConfig)` is shorthand for `il.Relation(kind="config", key="reporting_config")` with the class kept as the relation's `target`, which is what makes a fallback possible. `optional=True` allows the relation to stay unbound; `default=` is a zero-argument factory. +Neither says anything about deletion: `on_delete` alone decides that, and a resource relation +keeps its `block` default whether it is optional or not, so a bound connection, config or +destination cannot be deleted while a component consumes it. ## Fallbacks diff --git a/packages/interloper-core/src/interloper/asset/base.py b/packages/interloper-core/src/interloper/asset/base.py index 0e177141..c9969c08 100644 --- a/packages/interloper-core/src/interloper/asset/base.py +++ b/packages/interloper-core/src/interloper/asset/base.py @@ -208,6 +208,14 @@ def _data_hints(cls) -> dict[str, Any]: def _infer_relation(cls, name: str, hint: Any, *, optional: bool) -> Relation: """The relation one ``data()`` parameter declares. + An optional upstream is inferred ``on_delete="detach"``: the parameter + already tolerates the leg being absent, so nothing is owed to it and + deleting the upstream must not be refused on its behalf. A parameter + naming a component class keeps the default ``block`` whether it is + optional or not: this ruling covers asset upstreams only, and a + connection or config a read consumes stays undeletable while it is + bound. + Args: name: The parameter name, which is the relation's name and, for an upstream, the bare asset key it expects. @@ -227,10 +235,11 @@ def _infer_relation(cls, name: str, hint: Any, *, optional: bool) -> Relation: """ target, admits_none = unwrap_optional(hint, {}) optional = optional or admits_none + on_delete = "detach" if optional else "block" if target is Upstream: - return Relation("asset", name, optional=optional) + return Relation("asset", name, optional=optional, on_delete=on_delete) if get_origin(target) is list and get_args(target) == (Upstream,): - return Relation("asset", name, many=True, optional=optional) + return Relation("asset", name, many=True, optional=optional, on_delete=on_delete) if isinstance(target, type) and issubclass(target, Component) and target.kind: return Relation(target, optional=optional) if isinstance(hint, str): diff --git a/packages/interloper-core/src/interloper/component/relation.py b/packages/interloper-core/src/interloper/component/relation.py index 61aa34c9..14cde5cc 100644 --- a/packages/interloper-core/src/interloper/component/relation.py +++ b/packages/interloper-core/src/interloper/component/relation.py @@ -137,9 +137,12 @@ class Relation(BaseModel): several component kinds, ``key`` (when non-empty) narrows to specific keys within those kinds, matched through :meth:`ComponentIdentity.satisfies`. ``many`` marks a relation that binds several components at once; - ``optional`` marks one that may stay unbound. ``default`` and - ``has_fallback`` together describe a relation that can be resolved - without an explicit binding. + ``optional`` marks one that may stay unbound or be left empty, and says + nothing else: ``on_delete`` alone decides what deleting a bound target + does, ``block`` refusing the deletion while this relation holds the + target and ``detach`` letting it through and dropping the binding. + ``default`` and ``has_fallback`` together describe a relation that can be + resolved without an explicit binding. A relation is also its own descriptor: :meth:`Component._collect` installs the stamped copy under the relation's name, so the class attribute reads diff --git a/packages/interloper-core/tests/asset/test_base.py b/packages/interloper-core/tests/asset/test_base.py index 971de281..6141bf1c 100644 --- a/packages/interloper-core/tests/asset/test_base.py +++ b/packages/interloper-core/tests/asset/test_base.py @@ -317,6 +317,39 @@ def data(self, context: il.ExecutionContext, config: Cfg | None = None) -> Any: assert A.relations["config"].optional is True + def test_an_optional_resource_still_blocks_its_deletion(self): + class A(il.Asset): + def data(self, context: il.ExecutionContext, config: Cfg | None = None) -> Any: # pragma: no cover + return [] + + assert A.relations["config"].on_delete == "block" + + def test_an_optional_upstream_detaches_on_delete(self): + class A(il.Asset): + def data(self, context: il.ExecutionContext, x: il.Upstream | None = None) -> Any: # pragma: no cover + return [] + + relation = A.relations["x"] + assert (relation.optional, relation.on_delete) == (True, "detach") + + def test_a_required_upstream_blocks_on_delete(self): + class A(il.Asset): + def data(self, context: il.ExecutionContext, orders: il.Upstream) -> Any: # pragma: no cover + return [] + + relation = A.relations["orders"] + assert (relation.optional, relation.on_delete) == (False, "block") + + def test_an_optional_list_upstream_detaches_on_delete(self): + class A(il.Asset): + def data( + self, context: il.ExecutionContext, legs: list[il.Upstream] | None = None + ) -> Any: # pragma: no cover + return [] + + relation = A.relations["legs"] + assert (relation.many, relation.optional, relation.on_delete) == (True, True, "detach") + def test_optional_annotation_makes_optional(self): # Written as a string on purpose: a lazily-evaluated annotation must # resolve the same way as a real class. From 910d5e7ee59cc51c7e6740d83207de05e1a01848 Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Tue, 8 Sep 2026 07:20:25 +0200 Subject: [PATCH 17/19] fix(db)!: on_delete alone decides whether a referrer blocks a delete The delete guard treated any optional relation as detaching, so a destination in use (declared optional=True, on_delete="block") could be deleted from under the sources writing to it, and every optional consumption relation with it. _relation_detaches now reads on_delete only; an unresolvable name still blocks, keeping the guard fail-closed. optional keeps its own job, which unbind still honors: whether a relation may be left unbound or emptied. By Digitl --- .../src/interloper_db/store/components.py | 15 +++-- .../src/interloper_db/store/relations.py | 13 ++-- .../tests/store/test_components.py | 64 +++++++++++++++---- 3 files changed, 67 insertions(+), 25 deletions(-) diff --git a/packages/interloper-db/src/interloper_db/store/components.py b/packages/interloper-db/src/interloper_db/store/components.py index 6e07dd08..1c641fcc 100644 --- a/packages/interloper-db/src/interloper_db/store/components.py +++ b/packages/interloper-db/src/interloper_db/store/components.py @@ -243,11 +243,12 @@ def update( def delete(self, component_id: UUID) -> None: """Delete a component. Children and out-bound relations cascade via FK. - In-bound relations follow the semantics the referrer's class - declares for the name they are filed under: a consuming relation (a - bound ``connection``, a required upstream) blocks the deletion; one - declared ``on_delete="detach"`` or optional (a job's ``targets``, a - hook's ``watches``) detaches, its row cascading away while the + In-bound relations follow the ``on_delete`` the referrer's class + declares for the name they are filed under, and nothing else: a + consuming relation (a bound ``connection``, a bound ``destination``, + a required upstream) blocks the deletion; one declared + ``on_delete="detach"`` (a job's ``targets``, a hook's ``watches``, an + optional upstream) detaches, its row cascading away while the referrer keeps working with reduced scope. Args: @@ -281,8 +282,8 @@ def _blocking_referrers(self, session: Session, db_component: Component) -> list Deleting a relation destination cascades the edge row, which would leave a *consuming* referrer silently broken at its next run, so those relations refuse the deletion. An edge whose name the referrer - declares ``on_delete="detach"`` or optional is skipped: cascading it - is the intended outcome. Edges internal to the subtree (a source's + declares ``on_delete="detach"`` is skipped: cascading it is the + intended outcome. Edges internal to the subtree (a source's own sibling relations) don't count, and a referrer that is a source-owned asset is reported as its parent source, the unit the user can act on. diff --git a/packages/interloper-db/src/interloper_db/store/relations.py b/packages/interloper-db/src/interloper_db/store/relations.py index b57f2a84..5ba5f458 100644 --- a/packages/interloper-db/src/interloper_db/store/relations.py +++ b/packages/interloper-db/src/interloper_db/store/relations.py @@ -250,11 +250,12 @@ def _sync_relations(self, session: Session, src: Component, bindings: dict[str, def _relation_detaches(self, session: Session, referrer: Component, relation_row: ComponentRelation) -> bool: """Whether an edge detaches (rather than blocks) when its destination is deleted. - Consults the referrer's own vocabulary: a name declared - ``on_delete="detach"`` detaches, as does an optional one, since the - referrer keeps working with nothing bound there. Anything - unresolvable (an undeclared name, a drifted key) blocks, keeping the - guard fail-closed. + Consults the referrer's own vocabulary, where ``on_delete`` is the + sole authority: only a name declared ``on_delete="detach"`` detaches. + Whether the name is ``optional`` is a separate question, about leaving + it unbound or emptying it, and does not license deleting a target the + referrer holds. Anything unresolvable (an undeclared name, a drifted + key) blocks, keeping the guard fail-closed. Args: session: Open session the referrer's vocabulary is resolved @@ -268,7 +269,7 @@ def _relation_detaches(self, session: Session, referrer: Component, relation_row relation = self._vocabulary(session, referrer).get(relation_row.name) if relation is None: return False - return relation.on_delete == "detach" or relation.optional + return relation.on_delete == "detach" def _vocabulary(self, session: Session, row: Component) -> dict[str, il.Relation]: """The relation vocabulary governing a component row. diff --git a/packages/interloper-db/tests/store/test_components.py b/packages/interloper-db/tests/store/test_components.py index 2ebb0380..6ef98a50 100644 --- a/packages/interloper-db/tests/store/test_components.py +++ b/packages/interloper-db/tests/store/test_components.py @@ -70,7 +70,16 @@ def data(self, context: il.ExecutionContext, up: il.Upstream) -> list[dict]: class GuardOptional(il.Asset): - """Asset whose optional ``up`` relation detaches when its upstream goes.""" + """Asset whose ``up`` relation detaches when its upstream goes.""" + + up = il.Relation("asset", "guard_upstream", optional=True, on_delete="detach") + + def data(self, context: il.ExecutionContext, up: il.Upstream | None) -> list[dict]: + return [] + + +class GuardOptionalBlocking(il.Asset): + """Asset whose ``up`` relation is optional yet keeps the default ``block``.""" up = il.Relation("asset", "guard_upstream", optional=True) @@ -123,12 +132,12 @@ def data(self, context: il.ExecutionContext) -> list[dict]: class WireDownOptionalSource(il.Source): - """Downstream source whose asset optionally reads ``wire_up_source.rows``.""" + """Downstream source whose asset reads ``wire_up_source.rows`` detachably.""" class Reader(il.Asset): - """Asset with an optional cross-source upstream.""" + """Asset with a detaching cross-source upstream.""" - rows = il.Relation("asset", "wire_up_source.rows", optional=True) + rows = il.Relation("asset", "wire_up_source.rows", optional=True, on_delete="detach") def data(self, context: il.ExecutionContext) -> list[dict]: return [] @@ -158,6 +167,7 @@ def store(component_db: Engine) -> Store: GuardUpstream, GuardRequired, GuardOptional, + GuardOptionalBlocking, WireUpSource, WireDownSource, WireDownOptionalSource, @@ -277,6 +287,24 @@ def test_bound_connection_blocks_delete_and_names_referrer(self, store: Store, c ] assert "in use by Down" in str(excinfo.value) + def test_bound_destination_blocks_delete_and_names_referrer(self, store: Store, connection: Component): + destination = store.components.create(_ORG, kind="destination", key="dest") + source = store.components.create( + _ORG, + kind="source", + key="wire_down_source", + name="Down", + relations={"connection": [connection.id], "destinations": [destination.id]}, + ) + + # ``destinations`` is optional and many-valued yet keeps the default + # ``block``: a source writing to a destination consumes it, so the + # destination may not be deleted from under it. + with pytest.raises(InUseError) as excinfo: + store.components.delete(destination.id) + assert [r["id"] for r in excinfo.value.referrers] == [str(source.id)] + assert "in use by Down" in str(excinfo.value) + def test_delete_succeeds_after_repointing(self, store: Store, connection: Component): other = store.components.create(_ORG, kind="connection", key="wire_connection", config={}, encrypted=False) source = store.components.create( @@ -340,27 +368,39 @@ def test_intra_subtree_relations_do_not_block(self, store: Store): class TestUpstreamDeleteSemantics: - """A required upstream relation blocks deletion; an optional one detaches.""" + """``on_delete`` alone decides: a ``block`` upstream relation refuses the deletion, a ``detach`` one gives way.""" - def test_delete_blocks_on_required_referrer_and_detaches_optional(self, store: Store): + def test_delete_blocks_on_blocking_referrer_and_detaches_the_detaching_one(self, store: Store): upstream = store.components.create(_ORG, kind="asset", key="guard_upstream", name="Up") - required = store.components.create( + blocking = store.components.create( _ORG, kind="asset", key="guard_required", name="Req", relations={"up": [upstream.id]} ) - optional = store.components.create( + detaching = store.components.create( _ORG, kind="asset", key="guard_optional", name="Opt", relations={"up": [upstream.id]} ) with pytest.raises(InUseError) as excinfo: store.components.delete(upstream.id) - assert [r["id"] for r in excinfo.value.referrers] == [str(required.id)] + assert [r["id"] for r in excinfo.value.referrers] == [str(blocking.id)] - store.components.delete(required.id) + store.components.delete(blocking.id) store.components.delete(upstream.id) - assert store.components.get(optional.id).id == optional.id + assert store.components.get(detaching.id).id == detaching.id assert store.relations.list_all(_ORG, name="up") == [] + def test_an_optional_relation_keeping_the_default_policy_blocks(self, store: Store): + upstream = store.components.create(_ORG, kind="asset", key="guard_upstream", name="Up") + referrer = store.components.create( + _ORG, kind="asset", key="guard_optional_blocking", name="Opt", relations={"up": [upstream.id]} + ) + + # The two knobs are independent: the relation may be left unbound + # (``optional``), which says nothing about deleting what it holds. + with pytest.raises(InUseError) as excinfo: + store.components.delete(upstream.id) + assert [r["id"] for r in excinfo.value.referrers] == [str(referrer.id)] + class TestIntraSourceWiring: """Sibling relation edges are derived idempotently over the full child set.""" @@ -420,7 +460,7 @@ def test_removing_child_with_required_external_upstream_blocked(self, store: Sto assert [r["id"] for r in excinfo.value.referrers] == [str(down.id)] assert store.relations.list_all(_ORG, name="rows") != [] - def test_removing_child_with_optional_external_upstream_detaches(self, store: Store): + def test_removing_child_with_detaching_external_upstream_detaches(self, store: Store): up = store.components.create(_ORG, kind="source", key="wire_up_source") down = store.components.create(_ORG, kind="source", key="wire_down_optional_source") store.relations.add(_child(down, "reader").id, name="rows", dst_id=_child(up, "rows").id) From 14c4b72024a43a3d5dc83268a6aac3b567e8ba60 Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Wed, 9 Sep 2026 11:04:47 +0200 Subject: [PATCH 18/19] test(db): hydration tests read bindings through the relation attributes By Digitl --- packages/interloper-db/tests/store/test_hydration.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/packages/interloper-db/tests/store/test_hydration.py b/packages/interloper-db/tests/store/test_hydration.py index 5e5176db..f852ef25 100644 --- a/packages/interloper-db/tests/store/test_hydration.py +++ b/packages/interloper-db/tests/store/test_hydration.py @@ -312,11 +312,8 @@ def test_load_hydrates_with_stable_ids_and_deps(self, store: Store): rows_by_key = {child.key: str(child.id) for child in db_source.children} assets_by_key = {asset.key: asset for asset in source.assets} assert {key: asset.id for key, asset in assets_by_key.items()} == rows_by_key - assert assets_by_key["e"].bound_ids() == { - "b": [rows_by_key["b"]], - "c": [rows_by_key["c"]], - "d": [rows_by_key["d"]], - } + e = assets_by_key["e"] + assert {name: getattr(e, name).id for name in ("b", "c", "d")} == {name: rows_by_key[name] for name in "bcd"} def test_source_owned_asset_loads_through_its_parent(self, store: Store): db_source = store.components.create(_ORG, kind="source", key="demo_source", name="Demo") @@ -366,7 +363,7 @@ def test_an_owned_asset_referencing_another_source_loads(self, store: Store): revenue = store.components.load(revenue_row.id) assert isinstance(revenue, il.Asset) - assert revenue.bound_ids() == {"orders": [str(shop_orders.id)]} + assert revenue.orders.id == str(shop_orders.id) # ty: ignore[unresolved-attribute] def test_a_shared_upstream_reached_twice_hydrates_once(self, store: Store, monkeypatch: pytest.MonkeyPatch): shop = store.components.create(_ORG, kind="source", key="shop") From af1459e242c4c16d97ed15f8f9bbe6de456c5f32 Mon Sep 17 00:00:00 2001 From: Guillaume Onfroy Date: Wed, 9 Sep 2026 13:09:21 +0200 Subject: [PATCH 19/19] fix(db): the relation store reads kinds, keys and local as properties By Digitl --- packages/interloper-db/src/interloper_db/store/relations.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/interloper-db/src/interloper_db/store/relations.py b/packages/interloper-db/src/interloper_db/store/relations.py index 5ba5f458..84f0492c 100644 --- a/packages/interloper-db/src/interloper_db/store/relations.py +++ b/packages/interloper-db/src/interloper_db/store/relations.py @@ -355,9 +355,9 @@ def _resolve(self, session: Session, src: Component, relation: il.Relation, name if not relation.accepts(dst.kind, self._identity(session, dst), owner=self._identity(session, src)): raise ConfigError( f"'{src.key}'.{name} does not accept {dst.kind} '{dst.key}' " - f"(declared: kind {relation.kinds()}, key {relation.keys() or 'any'})" + f"(declared: kind {relation.kinds}, key {relation.keys or 'any'})" ) - if relation.source_local and src.parent_id is not None and dst.parent_id != src.parent_id: + if relation.local and src.parent_id is not None and dst.parent_id != src.parent_id: raise ConfigError(f"'{src.key}'.{name} names a sibling; '{dst.key}' belongs to another source instance") return dst