Skip to content

feat!: one Relation primitive for every component link - #321

Open
aaaaahaaaaa wants to merge 30 commits into
mainfrom
feat/many-upstreams-core
Open

feat!: one Relation primitive for every component link#321
aaaaahaaaaa wants to merge 30 commits into
mainfrom
feat/many-upstreams-core

Conversation

@aaaaahaaaaa

@aaaaahaaaaa aaaaahaaaaa commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Summary

This PR replaces every link between components (destinations, connections, upstream assets, job
targets, hook watches) with one primitive: Relation. Today those links are five separate
mechanisms with five separate names (relation_types, RelationDefinition, resource_types,
ResourceRef, depends_on, Dependency, RelationSlot), each with its own selection rule,
fallback behaviour and serialisation quirks. Relation collapses all of them into one class,
declared the same way on every kind (an annotation, a typed or bare il.Relation attribute, or the
decorator's relations=), selected the same way (an exact key, source.asset, *.asset, or a list
of keys), and read the same way (bound value if wired, else a fallback resolved at read time, never
at bind time).

The point of unifying this is the north star the whole redesign serves: a campaign matcher asset
that fans in every campaigns asset an organisation has configured, with the set of upstreams
decided by wiring at the instance level, not hardcoded into the class. Assets get one addition on
top of the shared model: data() receives il.Upstream (or list[il.Upstream]) for asset
relations instead of the bound component, since an asset's data is what downstream assets actually
need, not the component itself. This PR is core (interloper-core, interloper-assets
connectors), plus the interloper-app and interloper-db files the earlier dependency ->
upstream rename touched; the platform, matcher and app follow as stacked PRs on top of it.

Decisions

Topic Decision
Primitive One Relation class declares every link, on every kind. It replaces relation_types, RelationDefinition, resource_types, ResourceRef, depends_on, Dependency and RelationSlot.
Selection Always by key: an exact key, source.asset, *.asset, or a list of keys. A class in a declaration is shorthand for its key.
Bare key A bare asset key selects a sibling of the same source instance.
Wiring Always an instance on the component. Ids exist only in specs and in rows.
Fallback Fallbacks are resolved when a relation is read, never bound: an explicit default= first, otherwise the declared class when it is a Resource (settings may come from the environment, so it is constructed at read time and a validation error surfaces then, as today) or when all its fields are defaulted. An unbound non-optional relation with no fallback is a build error.
Optional Means the wiring may be absent. It says nothing about data: a bound upstream whose partition holds nothing yields data=None, for every relation.
Removal on_delete is declared per relation (block or detach). Unbinding is derived: refused when it would empty a non-optional relation, detach otherwise. on_unbind is dropped.
Declaration forms Annotation with a Component class (connection: Conn), a typed il.Relation attribute (destinations: list[Destination] = il.Relation("destination", many=True, optional=True), the annotation is for the type checker), a bare il.Relation attribute, or @il.asset(relations={...}). Explicit beats annotation; a subclass replaces its parent's declaration of the same name. Relation is its own descriptor (class access gives the Relation, instance access the bound value); a TYPE_CHECKING-only __new__ returning Any makes the typed form check under ty.
Injection data() receives the bound instance for resource relations and il.Upstream (single) or list[il.Upstream] (many) for asset relations.
Serialisation A manifest is one component, nested, and is exactly to_spec() output. A target that has a parent is emitted under its parent and referenced as {ref: id} everywhere else. A target without a parent is inline at the first relation that reaches it and referenced afterwards. No flag, no kind knowledge.
Containment Stays parent_id (assets under sources). parent moves onto Component so the serialisation rule is generic. To revisit once this lands (containment as an owned relation).
Rows component_relations(src_id, name, dst_id, ...), primary key (src_id, name, dst_id). type and slot collapse into name.
DAG Edges from bound asset relations. A bound upstream not among the DAG's workloads is included as a read-only node.
Migration Revision 017 is rewritten in place (it never shipped); no 018.
Compatibility None. All consumers ship together; the manifests repo updates in lockstep.

Execution rulings, finalized during implementation

A few points the design left open were settled while building this PR:

  • Declaration forms. Relation is its own descriptor and also carries a TYPE_CHECKING-only
    __new__ returning Any, so the typed attribute form (x: list[Destination] = Relation(...))
    type-checks under ty as if x were assigned the target type directly. Relation.__set__ owns
    rebinding, and Component.__setattr__ routes relation-name assignment to it, since pydantic's own
    __setattr__ only special-cases properties.
  • Fallback. A Resource (BaseSettings) target is always self-filling. Its fallback constructs
    at read time and a ValidationError surfaces then if a required, env-backed field is missing; an
    unbound connection is no longer a build-time error, matching today's behaviour.
  • Construction-time validation. validate_relations at construction time skips an unbound,
    non-optional asset relation whose key is not source-local (cross-source or wildcard keys can only
    be filled by the DAG, not by the source building its own assets). The DAG's own
    _check_relations still validates every live node once resolution has run.
  • Read-only inclusion. A bound upstream not among the DAG's workloads joins as a single,
    non-transitive pass: it is read, not run, so its own upstreams are not pulled in behind it.
  • Removal. Unbinding is derived from cardinality, not a separate on_unbind hook: refused when
    it would empty a non-optional relation, a plain detach otherwise.

What changes for connector authors

Before:

@il.source(resources={"connection": FacebookAdsConnection})
class FacebookAds(il.Source):
    account_id: str = il.InputField(discriminator=True)

    @il.asset(depends_on={"campaigns": il.Dependency(key="campaigns")})
    def campaigns_stats(self, context, connection, campaigns) -> list[dict]: ...

After:

@il.source
class FacebookAds(il.Source):
    connection: FacebookAdsConnection
    account_id: str = il.InputField(discriminator=True)

    def campaigns_stats(self, context: il.ExecutionContext, campaigns: il.Upstream) -> list[dict]: ...

The connection moves from a decorator kwarg to a plain annotation on the source; every asset that
needs it gets it trickled down automatically. The upstream moves from depends_on={...} with an
explicit il.Dependency to a bare il.Upstream-annotated parameter: the parameter's own name is the
bare key, so campaigns on campaigns_stats resolves to the sibling campaigns asset with no
further declaration. Cross-source or wildcard upstreams, and any relation that needs a non-default
key, still go through relations={...} on the decorator, unchanged in spirit from depends_on=.

All 24 connectors in interloper-assets have been moved onto this form.

Stack

This PR is phase 1 (core: interloper-core, interloper-assets connectors). Phase 2
(platform: interloper-db, interloper-api, interloper-toolkit, interloper-agent,
interloper-scheduler) is stacked on top of this branch as its own PR, followed by phase 3
(matcher) and phase 4 (app), each stacked on the one before.

It is not, however, strictly core-only: the branch also carries the earlier dependency ->
upstream rename commits, which touch interloper-app's catalog and component types and
interloper-db (migration 017 in the old type/slot shape, plus the relation store and its
tests). Phases 2 and 4 rewrite both of those, so what lands here is an intermediate state of
files those phases own, not their final shape.

Until phase 2 merges, the platform packages' test suites are red against this core: they still
reference the retired names (RelationDefinition, Dependency, resource_types, .upstreams,
.slots) that phase 1 removes. Alone, this branch leaves 37 platform tests failing
(interloper-api 4, interloper-scheduler 33) and 3 interloper-db test modules
uncollectable
. This is expected and owned by phase 2, not a regression to chase down here.
Because of that, this PR must merge together with, or immediately before, phase 2 so main
is never left on a red platform for longer than the merge itself takes.

Verification

Counts below are after the review fix wave (one write path for bindings, id-based trickle
detection on copies, forbidden unknown Relation kwargs, refused uncollected component
annotations, fetch-provider validation on kind-declared relations).

  • uv run --frozen ruff check: all checks passed. The pre-existing import-order issue in
    interloper-db/src/interloper_db/store/relations.py introduced by this branch's commit
    77598513 (an identity rename) has been fixed with ruff check --fix.
  • uv run --frozen ty check: 18 diagnostics repo-wide, all in phase-2-owned packages and all
    expected (old relation names not yet ported): interloper-db 15, interloper-slack 2,
    interloper-scheduler 1. Zero in interloper-core, interloper-assets, interloper-google-cloud,
    interloper-pandas, interloper-docker, interloper-k8s.
  • uv run --frozen pytest packages/interloper-core packages/interloper-assets packages/interloper-google-cloud packages/interloper-pandas packages/interloper-docker packages/interloper-k8s -q:
    1734 passed.
  • Phase-2 packages, collected/run against this core for the record (expected to fail until phase 2
    lands): 37 failures and 3 uncollectable modules in total.
    • interloper-db: 3 collection errors (AttributeError on retired names in
      test_components.py, test_hydration.py, test_relations.py).
    • interloper-api: 4 failed, 380 passed.
    • interloper-toolkit: 10 passed (no coverage of the retired surface yet).
    • interloper-agent: 11 passed (no coverage of the retired surface yet).
    • interloper-scheduler: 33 failed, 78 passed.
  • Retired-name sweep (resource_types, relation_types, RelationDefinition, Dependency,
    ResourceRef, depends_on, RelationSlot, AssetIdentity, and related old names) over
    interloper-core, interloper-assets, interloper-google-cloud, interloper-pandas,
    interloper-docker, interloper-k8s, docs, plugins, examples yields 12 hits across four
    categories, all deliberate:
    • 1 regression test (tests/component/test_base.py:845): asserts the old names are gone from the public API.
    • 4 docstring and comment hits (tests/resource/test_fields.py:98-99,109 and resource/fields.py:365): explain the current design by contrasting it with the retired one.
    • 5 upgrade skill migration table hits (plugins/interloper/skills/interloper-upgrade/SKILL.md:65,73-75,81): legitimate reference in the "Old" column of the 0.7x migration table.
    • 2 Docker compose syntax hits (examples/telemetry/docker-compose.yml:16,61): false positive on Docker's own depends_on: key.
  • Em-dash sweep over interloper-core/interloper-assets source and the guide/extending/reference/ui
    docs: all em-dashes in files this branch touched were rewritten; the remaining hits are all in
    files this branch never touched (pre-existing house style elsewhere in the codebase, left alone).

Follow-ups

  • Multi-dot declared keys resolve via rpartition; the semantics are unstated and untested.
  • Unbinding an already-empty, non-optional relation is a silent no-op.
  • Relation.__new__ returning Any means ty no longer checks Relation(...) call sites, and a
    non-optional relation annotated with the bare target type reads as always-present to ty even
    though it can be unbound at runtime.
  • tests/source/test_base.py's Finance.Revenue docstring rationale for optional=True is stale;
    tests/dag mixes Matcher and FakeMatcher fixture naming inconsistently.
  • A job-trickled destination serialises inline under its first target rather than under the job
    itself.

By Digitl

@codecov

codecov Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 92.34973% with 14 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...per-scheduler/src/interloper_scheduler/executor.py 11.11% 8 Missing ⚠️
...kages/interloper-core/src/interloper/asset/base.py 96.73% 3 Missing ⚠️
...terloper-toolkit/src/interloper_toolkit/lineage.py 25.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

…field to upstreams

The relation type dependency becomes upstream (a role name, like target and
watch) and Asset.dependencies becomes Asset.upstreams, across core, store,
toolkit, scheduler, app, docs and specs. Migration 017 renames persisted
rows and narrows the per-slot unique index to resources.

By Digitl
… flags

The slot a class declares and the slot the catalog publishes are one class.
The catalog JSON carries optional (inverted from required) and many.

By Digitl
depends_on holds asset keys and il.Dependency values (optional, many) and
replaces requires and optional_requires; upstreams is always a list. Every
reader goes through Asset.declared_upstreams(), and sibling wiring through
Asset.sibling_upstreams() in both the source and the store.

By Digitl
The whole-branch review flagged that validate_upstreams enforced
non-optional presence and identity but never cardinality: a single-valued
slot (many=False) silently accepted several wired upstreams and handed
data() whichever list _read_upstreams happened to read last. Raise
DependencyContractError when a single-valued slot has more than one
wired upstream present in the DAG, and pin the missing-data case for a
non-optional single slot (Upstream(asset, data=None) still flows through
when the upstream never materialized for that partition).

By Digitl
… names

The review found the guide and docstrings over-generalized "no data for
the partition" to None, when only a destination with nothing materialized
for that scope (no table or object at all) produces it; an existing but
empty scope returns whatever the destination gives back for an empty read.
Reword guide/dependencies.md, Upstream.data, and _read_upstreams with that
precision, and sweep stale relation/dependency naming left over from the
upstream-relation rename: relations.py and hydration.py module docstrings,
DependencyContractError's docstring, _destination_read's summary, the
reference/errors.md table (new DAGError/DependencyNotFoundError/AssetError/
DependencyContractError cases), and two clarifying lines on
DAG._resolve_declared about reused instances and empty-list wiring.

By Digitl
…e relations repoint

Attribute assignment wrote _bound directly, so it bound without cascading:
setting a source's destinations left its assets with none and a run wrote
nothing. Both write paths now land in Component._replace_binding, which
checks, deduplicates, refuses to empty a non-optional relation, and calls the
new _rebound hook that Source and Job override to trickle. A second bind on a
single-valued relation now replaces its target instead of demanding an unbind.

By Digitl
…ion kwargs; reject uncollected Component annotations

A deep copy rebuilds a source's own bindings and its assets' separately, so
the copy's assets hold distinct objects carrying the same ids: comparing by
identity read every trickled binding on a copy as the asset's own, and a
chained copy never repointed its assets. Compare target ids instead.

Relation now forbids extra kwargs, so a misspelled flag is a validation error
rather than a silently dropped declaration. And an annotation naming a
component class that pydantic kept as a plain field, or that resolves
nowhere, is refused at class definition instead of leaving the component with
no relation, no trickle and no reference.

By Digitl
… name replaces slot

A provider naming a relation declared by kind and key (no class to read the
method off) crashed with an AttributeError while formatting its own error.
Both that case and an undeclared relation now raise the intended TypeError.
The module's vocabulary follows the relation model: the provider form is
"<name>.<method>", and the prose and locals say name rather than slot.

By Digitl
@aaaaahaaaaa
aaaaahaaaaa force-pushed the feat/many-upstreams-core branch from 00b023b to 9f1c2b8 Compare September 7, 2026 23:40
@aaaaahaaaaa aaaaahaaaaa changed the title feat(core)!: upstream relation, depends_on and il.Dependency, many-valued upstreams and DAG resolution feat!: one Relation primitive for every component link Sep 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant