From b0506cd8760dfce7d6c50b146646c7b0de39d614 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Tue, 3 Nov 2026 23:29:20 +0100 Subject: [PATCH 1/9] feat(coordinator)!: plan and run builds through build_release build_release() in timenet.coordinator.build builds one release into a local store, after each release that its build needs. It finds the backend that claims the dataset. If the caller asks for another version than the card declares, it raises TimeNetVersionUnavailableError before anything runs. Then it plans with plan_parents(), adds the build of the requested release, and runs each step in order. A copy step uses copy_release(). If the store holds the release intact, the step copies only each payload source that the store lacks or holds damaged. A build step calls build_prepared() of the backend for that exact release. Each step sends StepStarted and StepFinished. Only the requested release gets force. build_release() returns the Plan that ran. The BuilderBackend protocol gets two methods. dataset_ids() lists the IDs of the datasets that a backend builds. build_prepared(release, store, *, force, pinned_by, options) builds exactly one release into a local store that holds each parent of the release, and each payload source of such a parent. BuildOptions holds keep_cache and the progress callback. find_builder() returns the first backend whose dataset_ids() holds the ID. knows(), declared_version() and build() stay for now. ConnectorBuilder lists its dataset IDs with known_ids() of timenet_connectors.discovery. known_ids() reads the cards on disk and imports no connector. A card that cannot be read raises TimeNetInvalidCardError, which names the file. A dataset ID matches only the ID that a card declares, so the module spelling, such as timenet/hello_world, no longer resolves. The build command checks the ID against known_ids() and shows the usage error with the known IDs. TimeNetVersionUnavailableError is a TimeNetDatasetNotFoundError and a TimeFValidationError. It carries the requested release and the declared version. plan_parents() takes open_access_only. With it, a local store gets the same access checks as a registry that is not local, also for the card and the lineage of a requested release that the store already holds. If the output is not local or open_access_only is set, the planner checks the access of the requested card first, also for a dataset without parents. BREAKING CHANGE: a BuilderBackend must implement dataset_ids() and build_prepared(). find_builder(), timenet-build build and timenet_connectors.build() no longer accept the module spelling of a dataset ID. Part of TNET-157. --- .../src/timenet_connectors/builder/backend.py | 13 +- .../src/timenet_connectors/builder/cli.py | 11 +- .../src/timenet_connectors/discovery.py | 27 +- .../tests/test_build_backend.py | 36 ++ .../tests/test_build_cli.py | 9 + .../tests/test_build_plan.py | 42 +-- packages/timenet/src/timenet/builders.py | 84 ++++- .../timenet/src/timenet/coordinator/build.py | 193 ++++++++++ .../timenet/src/timenet/coordinator/plan.py | 65 +++- packages/timenet/src/timenet/errors.py | 28 ++ .../timenet/src/timenet/testing/__init__.py | 5 +- .../timenet/src/timenet/testing/backends.py | 38 ++ .../timenet/tests/coordinator/test_build.py | 333 ++++++++++++++++++ packages/timenet/tests/test_builders.py | 53 +-- packages/timenet/tests/test_errors.py | 15 +- 15 files changed, 865 insertions(+), 87 deletions(-) create mode 100644 packages/timenet/src/timenet/coordinator/build.py create mode 100644 packages/timenet/src/timenet/testing/backends.py create mode 100644 packages/timenet/tests/coordinator/test_build.py diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py index bd3c9d7ab..01473e5d2 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py @@ -8,12 +8,23 @@ from timenet.types import DatasetMetadata from timenet.types.references import DatasetRef from timenet_connectors.builder.cli import build_one -from timenet_connectors.discovery import connector_dir, has_connector +from timenet_connectors.discovery import connector_dir, has_connector, known_ids class ConnectorBuilder: """Builds a dataset by running its connector in an environment built from its requirements.""" + def dataset_ids(self) -> list[str]: # ruff: ignore[no-self-use] (BuilderBackend protocol method) + """List the dataset IDs of the connector cards in this package, without importing a connector. + + Returns: + The dataset IDs, sorted. + + Raises: + TimeNetInvalidCardError: If a card cannot be read or is not a mapping. + """ # ruff: ignore[docstring-extraneous-exception] - raised by known_ids + return known_ids() + def knows(self, dataset_id: str) -> bool: # ruff: ignore[no-self-use] (BuilderBackend protocol method) """Report whether a connector package exists for the id, without importing it. diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index c5260dc6f..7286534f7 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -49,7 +49,7 @@ from timenet.types.references import DatasetRef from timenet.writer.progress import ProgressStage, WriteProgressEvent from timenet_connectors.builder.env import run_isolated -from timenet_connectors.discovery import connector_dir, resolve +from timenet_connectors.discovery import known_ids, resolve from timenet_connectors.download import DownloadProgress, ProgressCallback, progress_sink @@ -147,11 +147,10 @@ def build( TimeNetAccessError: If the output is remote and gets a release without access declared ``open``. """ - try: - # connector_dir imports no connector. So an unknown id fails before any environment exists. - connector_dir(dataset_id) - except LookupError as exc: - raise typer.BadParameter(str(exc)) from exc + # known_ids imports no connector. So an unknown id fails before any environment exists. + known = known_ids() + if dataset_id not in known: + raise typer.BadParameter(f"no connector for {dataset_id!r}; known: {', '.join(known) or '(none)'}") forwarded = out if out is not None else settings().registry if forwarded is None: forwarded = str(settings().registry_path) diff --git a/packages/timenet-connectors/src/timenet_connectors/discovery.py b/packages/timenet-connectors/src/timenet_connectors/discovery.py index 09dd5a445..7da09abf7 100644 --- a/packages/timenet-connectors/src/timenet_connectors/discovery.py +++ b/packages/timenet-connectors/src/timenet_connectors/discovery.py @@ -15,6 +15,7 @@ import yaml from timenet.connectors import BaseConnector +from timenet.errors import TimeNetInvalidCardError import timenet_connectors.datasets as _datasets @@ -47,7 +48,7 @@ def connector_dir(dataset_id: str) -> Path: path = Path(next(iter(spec.submodule_search_locations))) if (path / "dataset.yaml").is_file(): return path - raise LookupError(f"no connector for {dataset_id!r}; known: {', '.join(_known_ids()) or '(none)'}") + raise LookupError(f"no connector for {dataset_id!r}; known: {', '.join(known_ids()) or '(none)'}") def has_connector(dataset_id: str) -> bool: @@ -57,7 +58,7 @@ def has_connector(dataset_id: str) -> bool: answer here, not an exception. Args: - dataset_id: A flat (``hello_world``) or ``org/name`` id. + dataset_id: The dataset ID, as ``org/name``. Returns: Whether a connector package exists for the id. @@ -73,7 +74,7 @@ def requirements_for(dataset_id: str) -> Path | None: """Return the connector's ``requirements.txt``, or ``None`` if it declares none. Args: - dataset_id: A flat (``hello_world``) or ``org/name`` id. + dataset_id: The dataset ID, as ``org/name``. Returns: The path to the connector's ``requirements.txt``, or ``None``. @@ -85,21 +86,31 @@ def requirements_for(dataset_id: str) -> Path | None: return path if path.is_file() else None -def _known_ids() -> list[str]: +def known_ids() -> list[str]: """List the dataset ids declared by the connector cards on disk, importing nothing. - :func:`available` answers the same question by importing every connector. The unknown-id error - path cannot afford that: those dependencies live in each connector's own environment, not this - one. + :func:`available` answers the same question by importing every connector. The builder backend + and the unknown-id error path cannot afford that: those dependencies live in each connector's + own environment, not this one. A dataset id matches exactly as its card declares it. Returns: The declared dataset ids, sorted. + + Raises: + TimeNetInvalidCardError: If a card cannot be read or is not a mapping. """ ids: set[str] = set() for root in _datasets.__path__: # A flat id sits at ``datasets//``, an ``org/name`` id one level deeper. for card in (*Path(root).glob("*/dataset.yaml"), *Path(root).glob("*/*/dataset.yaml")): - declared = (yaml.safe_load(card.read_text(encoding="utf-8")) or {}).get("dataset_id") + # A card that cannot be read fails each lookup, as DatasetMetadata.from_yaml fails for it. + try: + raw = yaml.safe_load(card.read_text(encoding="utf-8")) + except (OSError, yaml.YAMLError) as error: + raise TimeNetInvalidCardError(f"could not read dataset card {card}: {error}") from error + if not isinstance(raw, dict): + raise TimeNetInvalidCardError(f"dataset card {card} must be a YAML mapping, got {type(raw).__name__}") + declared = raw.get("dataset_id") if declared: ids.add(declared) return sorted(ids) diff --git a/packages/timenet-connectors/tests/test_build_backend.py b/packages/timenet-connectors/tests/test_build_backend.py index 63ec0616e..a63fdaa13 100644 --- a/packages/timenet-connectors/tests/test_build_backend.py +++ b/packages/timenet-connectors/tests/test_build_backend.py @@ -1,8 +1,10 @@ from pathlib import Path +import re import sys import pytest +from timenet.builders import find_card from timenet.errors import TimeNetInvalidCardError from timenet.manifest import Manifest, PayloadSource from timenet.manifest.files import FilePart @@ -10,8 +12,10 @@ from timenet.registry import open_local_version from timenet.types.references import DatasetRef from timenet.values_backends import ValuesBackend +from timenet_connectors import datasets as datasets_module from timenet_connectors.builder import backend as backend_module, cli as cli_module from timenet_connectors.builder.backend import ConnectorBuilder +from timenet_connectors.discovery import known_ids def _unexpected_isolation(*args, **kwargs): @@ -31,6 +35,17 @@ def test_knows_rejects_an_unknown_id_without_importing_a_connector(): assert leaf not in sys.modules +def test_connector_builder_lists_its_dataset_ids_without_importing_a_connector(monkeypatch): + leaf = "timenet_connectors.datasets.physionet.ecg_qa_cot" + monkeypatch.delitem(sys.modules, leaf, raising=False) + + dataset_ids = list(ConnectorBuilder().dataset_ids()) + + assert {"timenet/hello-world", "physionet/ecg-qa-cot"} <= set(dataset_ids) + assert "nope/nothing" not in dataset_ids + assert leaf not in sys.modules + + def test_a_card_that_cannot_be_read_raises_and_only_a_missing_connector_gives_none(tmp_path, monkeypatch): assert ConnectorBuilder().card("nope/nothing") is None assert ConnectorBuilder().declared_version("nope/nothing") is None @@ -43,6 +58,27 @@ def test_a_card_that_cannot_be_read_raises_and_only_a_missing_connector_gives_no ConnectorBuilder().declared_version("test/broken") +@pytest.mark.parametrize( + ("text", "message"), + [ + ("dataset_id: [unclosed\n", "could not read dataset card {}"), + ("- a list\n", "dataset card {} must be a YAML mapping"), + ], +) +def test_a_card_that_cannot_be_read_fails_each_lookup_with_the_card_error(tmp_path, monkeypatch, text, message): + for name, card in [("good", "dataset_id: test/good\n"), ("broken", text)]: + (tmp_path / "test" / name).mkdir(parents=True) + (tmp_path / "test" / name / "dataset.yaml").write_text(card) + monkeypatch.setattr(datasets_module, "__path__", [str(tmp_path)]) + expected = re.escape(message.format(tmp_path / "test" / "broken" / "dataset.yaml")) + + with pytest.raises(TimeNetInvalidCardError, match=expected): + known_ids() + # The installed entry point loads the builder backend of this package, which lists its dataset IDs. + with pytest.raises(TimeNetInvalidCardError, match=expected): + find_card("test/good") + + def test_build_runs_in_an_isolated_environment_by_default(tmp_path, monkeypatch): monkeypatch.delenv("TIMENET_ISOLATION", raising=False) calls = [] diff --git a/packages/timenet-connectors/tests/test_build_cli.py b/packages/timenet-connectors/tests/test_build_cli.py index 495e6b0e7..c104a4ce9 100644 --- a/packages/timenet-connectors/tests/test_build_cli.py +++ b/packages/timenet-connectors/tests/test_build_cli.py @@ -106,6 +106,15 @@ def test_build_rejects_an_unknown_id_without_importing_anything(monkeypatch, tmp assert connectors == [] +def test_build_rejects_an_id_that_no_card_declares_and_names_the_known_ids(tmp_path): + # The module path of the connector accepts this spelling, but a dataset ID must match its card. + result = runner.invoke(app, ["build", "timenet/hello_world", "--out", str(tmp_path)]) + + assert result.exit_code == 2 + assert "no connector for 'timenet/hello_world'" in result.output + assert "timenet/hello-world" in result.output + + def test_build_forwards_quiet_to_the_isolated_child(monkeypatch, tmp_path): captured = {} # The console is a process-wide singleton; monkeypatch restores it for the tests that follow. diff --git a/packages/timenet-connectors/tests/test_build_plan.py b/packages/timenet-connectors/tests/test_build_plan.py index 8b0deaa37..b93ff5e6e 100644 --- a/packages/timenet-connectors/tests/test_build_plan.py +++ b/packages/timenet-connectors/tests/test_build_plan.py @@ -22,7 +22,7 @@ from timenet.reader import TimeFReader from timenet.registry import LocalRegistry, S3Registry, WritableRegistry from timenet.registry._paths import release_dir -from timenet.testing import make_dataset +from timenet.testing import install_backends, make_dataset from timenet.types import Access, TimeSeriesSpec, ureg from timenet.types.access import AccessRequirement from timenet.types.parent_dataset_card import ParentDatasetCard @@ -48,6 +48,9 @@ class _CardBackend: def __init__(self, cards): self.cards = cards + def dataset_ids(self): + return list(self.cards) + def knows(self, dataset_id): return dataset_id in self.cards @@ -67,6 +70,9 @@ class _InRepoCards(ConnectorBuilder): def __init__(self, cards): self.cards = cards + def dataset_ids(self): + return list(self.cards) + def knows(self, dataset_id): return dataset_id in self.cards @@ -74,14 +80,6 @@ def card(self, dataset_id): return self.cards.get(dataset_id) -class _Entry: - def __init__(self, backend): - self._backend = backend - - def load(self): - return lambda: self._backend - - def _cards(**parents: dict[str, DatasetRef]) -> dict: """Return connector cards by dataset ID; each keyword is ``={alias: ref}`` for ``test/``.""" cards = {GRAND.dataset_id: make_dataset().metadata} @@ -104,7 +102,7 @@ def _install_cards(monkeypatch, **parents: dict[str, DatasetRef]) -> dict: The cards by dataset ID, which a test may change. """ cards = _cards(**parents) - monkeypatch.setattr(builders, "entry_points", lambda group: [_Entry(_InRepoCards(cards))]) + install_backends(monkeypatch, _InRepoCards(cards)) return cards @@ -238,8 +236,7 @@ def test_the_plan_reads_each_card_from_the_backend_that_claims_the_dataset(tmp_p del cards[GRAND.dataset_id] # The in-repo backend claims GRAND (the hello-world connector) but none of the test datasets, # which only the second backend knows. - entries = [_Entry(ConnectorBuilder()), _Entry(_CardBackend(cards))] - monkeypatch.setattr(builders, "entry_points", lambda group: entries) + install_backends(monkeypatch, ConnectorBuilder(), _CardBackend(cards)) steps = plan_parents(CHILD.dataset_id, LocalRegistry(tmp_path), build_parents=True) @@ -252,10 +249,9 @@ def test_the_plan_reads_each_card_from_the_backend_that_claims_the_dataset(tmp_p def test_a_parent_whose_card_cannot_be_read_fails_the_plan_with_the_card_error(tmp_path, monkeypatch): (tmp_path / "dataset.yaml").write_text("dataset_id: [unclosed\n") # The in-repo backend claims PARENT, whose card cannot be read. The second backend serves CHILD. - monkeypatch.setattr(backend_module, "has_connector", lambda dataset_id: dataset_id == PARENT.dataset_id) + monkeypatch.setattr(backend_module, "known_ids", lambda: [PARENT.dataset_id]) monkeypatch.setattr(backend_module, "connector_dir", lambda dataset_id: tmp_path) - entries = [_Entry(ConnectorBuilder()), _Entry(_CardBackend(_cards(child={"parent": PARENT})))] - monkeypatch.setattr(builders, "entry_points", lambda group: entries) + install_backends(monkeypatch, ConnectorBuilder(), _CardBackend(_cards(child={"parent": PARENT}))) with pytest.raises(TimeNetInvalidCardError, match="could not read dataset card") as caught: plan_parents(CHILD.dataset_id, LocalRegistry(tmp_path / "output"), build_parents=True) @@ -468,8 +464,8 @@ def run_isolated(dataset_id, root, *, force=False, **_): def test_build_prepares_parents_before_the_isolated_child(tmp_path, monkeypatch): - _install_cards(monkeypatch, child={"parent": PARENT, "grand": GRAND}, parent={}) - monkeypatch.setattr(cli_module, "connector_dir", lambda dataset_id: tmp_path) + cards = _install_cards(monkeypatch, child={"parent": PARENT, "grand": GRAND}, parent={}) + monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) monkeypatch.delenv("TIMENET_ISOLATION", raising=False) origin = LocalRegistry(tmp_path / "origin") origin.put_dataset(make_dataset()) @@ -604,7 +600,7 @@ def test_a_fetch_of_a_release_that_is_not_open_fails_before_any_download(tmp_pat def test_a_pinned_parent_build_hands_its_pins_to_the_isolated_child(tmp_path, monkeypatch): - _install_cards(monkeypatch, child={"parent": PARENT}, parent={"parent": GRAND}) + cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={"parent": GRAND}) output = LocalRegistry(tmp_path / "output") _parent_without_its_source(output) pins = output.get_manifest(PARENT.dataset_id).payload_sources[SourceKey(str(GRAND))] @@ -627,7 +623,7 @@ def child(command, env): monkeypatch.delenv("TIMENET_ISOLATION", raising=False) monkeypatch.setattr(env_module, "requirements_for", lambda dataset_id: None) monkeypatch.setattr(env_module, "_run_build", child) - monkeypatch.setattr(cli_module, "connector_dir", lambda dataset_id: tmp_path) + monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) monkeypatch.setattr(cli_module, "resolve", lambda dataset_id: _Grand) monkeypatch.setattr(cli_module, "run_pipeline", run_pipeline) @@ -669,7 +665,7 @@ def test_a_parent_that_another_backend_claims_builds_there_with_its_pins(tmp_pat calls, isolated = [], [] # The in-repo backend comes first but claims none of the test datasets. backends = [ConnectorBuilder(), _CopyBackend(_cards(child={"parent": PARENT}, parent={}, stored={}), saved, calls)] - monkeypatch.setattr(builders, "entry_points", lambda group: [_Entry(backend) for backend in backends]) + install_backends(monkeypatch, *backends) monkeypatch.delenv("TIMENET_ISOLATION", raising=False) monkeypatch.setattr(cli_module, "run_isolated", _isolated_builds(isolated)) if output_kind == "remote": @@ -708,7 +704,7 @@ def test_a_subclass_of_the_in_repo_backend_builds_a_parent_through_its_own_build cards = _cards(child={"parent": PARENT}, parent={}) own, isolated = [], [] backend = _OwnBuild(cards, own) if overrides else _InRepoCards(cards) - monkeypatch.setattr(builders, "entry_points", lambda group: [_Entry(backend)]) + install_backends(monkeypatch, backend) monkeypatch.delenv("TIMENET_ISOLATION", raising=False) monkeypatch.setattr(cli_module, "run_isolated", _isolated_builds(isolated)) output = LocalRegistry(tmp_path / "output") @@ -741,7 +737,7 @@ def test_a_proxy_backend_builds_a_parent_through_the_build_of_its_instance(tmp_p # The class of the proxy has no build(). The instance forwards build() to a backend that records it. backend = _Proxy(_OwnBuild(_cards(child={"parent": PARENT}, parent={}), own)) assert not hasattr(_Proxy, "build") - monkeypatch.setattr(builders, "entry_points", lambda group: [_Entry(backend)]) + install_backends(monkeypatch, backend) monkeypatch.delenv("TIMENET_ISOLATION", raising=False) monkeypatch.setattr(cli_module, "run_isolated", _isolated_builds(isolated)) output = LocalRegistry(tmp_path / "output") @@ -758,7 +754,7 @@ def test_a_proxy_backend_builds_a_parent_through_the_build_of_its_instance(tmp_p @pytest.mark.parametrize("case", ["missing", "damaged", "not-open"]) def test_the_cli_hint_names_its_flags(tmp_path, monkeypatch, case): cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) - monkeypatch.setattr(cli_module, "connector_dir", lambda dataset_id: tmp_path) + monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) out = LocalRegistry(tmp_path / "out") if case == "damaged": out.put_dataset(TimeFDataset(metadata=cards[PARENT.dataset_id])) diff --git a/packages/timenet/src/timenet/builders.py b/packages/timenet/src/timenet/builders.py index bcede4529..4daefdeae 100644 --- a/packages/timenet/src/timenet/builders.py +++ b/packages/timenet/src/timenet/builders.py @@ -6,25 +6,60 @@ package plug in the same way the first-party one does. """ -from collections.abc import Mapping +from collections.abc import Iterable, Mapping +from dataclasses import dataclass from importlib.metadata import entry_points from pathlib import Path from typing import TYPE_CHECKING, Protocol +from timenet.progress import ProgressCallback from timenet.types import DatasetMetadata from timenet.types.references import DatasetRef if TYPE_CHECKING: from timenet.manifest import PayloadSource - from timenet.registry import BaseRegistry + from timenet.registry import BaseRegistry, LocalRegistry ENTRY_POINT_GROUP = "timenet.builders" +@dataclass(frozen=True) +class BuildOptions: + """Options of a build that do not change the files of the release. + + Attributes: + keep_cache: Keep the raw download cache after the build. + progress: Receives the progress events of the build, or ``None``. + """ + + keep_cache: bool = False + progress: ProgressCallback | None = None + + class BuilderBackend(Protocol): - """Something that can build a dataset from its connector.""" + """Something that can build a dataset release from its connector. + + The build coordinator plans the build first. It puts each parent of a release, and each payload + source of such a parent, into the local store before it calls :meth:`build_prepared` for that + release. So :meth:`build_prepared` builds exactly one release, and it does not plan, copy or + build other releases. + """ + + def dataset_ids(self) -> Iterable[str]: + """Return the IDs of the datasets that this backend can build. + + Implementations must answer without importing a connector: this runs before any connector + environment exists. + + Returns: + The dataset IDs. + + Raises: + TimeNetInvalidCardError: If the card of a connector cannot be read. + """ + ... def knows(self, dataset_id: str) -> bool: """Report whether this backend has a connector for the id. @@ -105,19 +140,56 @@ def build( # ruff: ignore[too-many-arguments] """ ... + def build_prepared( + self, + release: DatasetRef, + store: "LocalRegistry", + *, + force: bool, + pinned_by: "Mapping[DatasetRef, PayloadSource]", + options: BuildOptions, + ) -> None: + """Build exactly this release into the store. + + Each parent of the release, and each payload source of such a parent, is already in + ``store`` when this method runs. If ``pinned_by`` is not empty, other releases already + read files of this release. The build then publishes the release only if it writes each + pinned file again with the same path, size and checksum. Otherwise it publishes nothing + and raises an error. + + Args: + release: The exact dataset ID and version to build. + store: The local registry to build into. + force: Build the release again, even if ``store`` already holds it. + pinned_by: The releases that already read files of this release, with their pins. + options: The options of the build that do not change its files. + + Raises: + TimeNetDatasetNotFoundError: If this backend does not know the dataset ID of ``release``. + TimeFValidationError: If the connector declares another version than ``release``. + TimeFFormatError: If the build does not reproduce a pinned file. Nothing is published + then. + TimeNetBuildError: If the build runs but fails. + """ + ... + def find_builder(dataset_id: str) -> BuilderBackend | None: - """Return the first registered backend that claims this dataset id. + """Return the first registered backend whose dataset IDs hold this dataset ID. Args: dataset_id: The dataset id. Returns: The backend, or ``None`` when no installed package claims the id. - """ + + Raises: + TimeNetInvalidCardError: If a backend cannot list its dataset IDs, because the card of a + connector cannot be read. + """ # ruff: ignore[docstring-extraneous-exception] - raised by the backend for entry in entry_points(group=ENTRY_POINT_GROUP): backend = entry.load()() - if backend.knows(dataset_id): + if dataset_id in backend.dataset_ids(): return backend return None diff --git a/packages/timenet/src/timenet/coordinator/build.py b/packages/timenet/src/timenet/coordinator/build.py new file mode 100644 index 000000000..7b612aecc --- /dev/null +++ b/packages/timenet/src/timenet/coordinator/build.py @@ -0,0 +1,193 @@ +"""Build a release and each release that its build needs, through one coordinator. + +The coordinator does three things in order. First, it finds the installed builder backend that +claims the dataset, and it checks a requested version against the card of the connector. Then it +plans: :func:`~timenet.coordinator.plan.plan_parents` finds each parent and payload source that the +store does not hold, and the coordinator adds the build of the requested release. Then it runs each +step of the plan in order. A copy goes through :func:`~timenet.registry.copy_release`. A build +goes through ``build_prepared()`` of the backend that builds exactly that release. So every backend +gets the same call, and a backend builds only one release, with its parents already in the store. +""" + +from collections.abc import Mapping +from typing import cast + +from timenet.builders import BuildOptions, exact_builder, find_card +from timenet.coordinator.plan import Plan, Step, plan_parents +from timenet.errors import TimeNetDatasetNotFoundError, TimeNetVersionUnavailableError +from timenet.manifest import Manifest, PayloadSource +from timenet.progress import ProgressCallback, StepFinished, StepStarted +from timenet.registry import copy_release +from timenet.registry._paths import release_dir +from timenet.registry.base import BaseRegistry +from timenet.registry.dependencies import prepare_local_copy +from timenet.registry.local import LocalRegistry +from timenet.registry.shared import complete_copy +from timenet.types.references import DatasetRef, split_ref + + +def build_release( # ruff: ignore[too-many-arguments] + dataset: str, + *, + store: LocalRegistry, + fetch_from: BaseRegistry | None = None, + build_parents: bool = False, + force: bool = False, + pinned_by: Mapping[DatasetRef, PayloadSource] | None = None, + open_access_only: bool = False, + options: BuildOptions | None = None, +) -> Plan: + """Build a release into the local store, after each release that its build needs. + + The connector that claims the dataset builds only the version that its card declares. If + ``dataset`` names another version, this function raises an error before anything runs. Then it + plans. Each parent of the release, and each payload source of such a parent, that ``store`` does + not hold is copied from ``fetch_from``, or else built by the backend that builds that exact + release if ``build_parents`` is set. A missing parent that the plan cannot get fails before any + step runs. If ``store`` already holds the requested release and ``force`` is not set, the plan + gets only its missing payload sources, and nothing builds the release. + + Then each step runs in order. A copy step copies the release and its payload sources with + :func:`~timenet.registry.copy_release`, which sends the progress events of each release + that it copies. If ``store`` holds an intact copy of the release, the step copies only each + payload source that ``store`` lacks or holds damaged, and then makes sure that the release + matches its pins. A build step sends :class:`~timenet.progress.StepStarted`, calls + ``build_prepared()`` of the backend, makes sure that ``store`` holds the release with its payload + sources, and then sends :class:`~timenet.progress.StepFinished`. Only the build of the requested + release gets ``force``. + + Args: + dataset: The dataset ID, as ``org/name``, or one exact release, as ``org/name@1.2.0``. + store: The local registry to build into. + fetch_from: The registry to copy missing parents from. If it is ``None``, nothing is copied. + build_parents: Build missing parents with the backends that build them. + force: Build the requested release again, even if ``store`` holds it, and plan its parents. + pinned_by: The releases that already read files of the requested release, with their pins. + The build of the requested release publishes it only if it reproduces each pinned file. + open_access_only: Refuse each release of the plan that does not declare ``access: open``, + the requested release included, before any step runs. A caller sets it when it copies + the result to a registry that holds only open releases. + options: The options of each build, such as the progress callback. + + Returns: + The plan that ran: its steps in the order in which they ran. + + Raises: + TimeNetDatasetNotFoundError: If no installed backend builds the dataset, or a missing parent + has no installed backend. + TimeNetVersionUnavailableError: If ``dataset`` names another version than the card of the + connector declares. Nothing runs then. + TimeNetVersionConflictError: If a copy step finds that ``store`` holds a release that it + copies, or a payload source of it, with other files. That step copies nothing then. + TimeFValidationError: If ``dataset`` is not a valid dataset reference, if the parents form a + cycle, or if a backend declares another version than a parent pins. + TimeNetMissingParentError: If a parent or a payload source is missing and ``build_parents`` + is not set. + TimeNetAccessError: If ``open_access_only`` is set and a release of the plan does not + declare ``access: open``. Also if ``fetch_from`` is not local and the plan copies such a + release from it. + TimeNetInvalidCardError: If the card of an installed connector cannot be read. + TimeNetFileError: If ``store`` holds a damaged copy of a parent that the plan cannot copy + again. + TimeFFormatError: If a build does not reproduce the files that other releases pin. + TimeNetBuildError: If a build runs but fails. + """ # ruff: ignore[docstring-extraneous-exception] - raised by the plan, the copy and the backends + options = options or BuildOptions() + dataset_id, version = split_ref(dataset) + card = find_card(dataset_id) + if card is None: + raise TimeNetDatasetNotFoundError(f"no installed connector builds {dataset_id!r}") + if version is not None: + wanted = DatasetRef.parse(dataset) + if wanted.version != card.dataset_version: + raise TimeNetVersionUnavailableError(wanted, card.dataset_version) + requested = DatasetRef(dataset_id=card.dataset_id, version=card.dataset_version) + # Passes fetch_from and build_parents to the planner unchanged: they choose how it gets a missing parent. + steps = plan_parents( + dataset_id, + store, + fetch_from=fetch_from, + build_parents=build_parents, + force_build=force, + open_access_only=open_access_only, + ) + if force or not store.exists(requested.dataset_id, str(requested.version)): + planned = {step.release for step in steps} + depends_on = tuple(parent for parent in card.parents.values() if parent in planned) + steps.append(Step(requested, "build", depends_on=depends_on, pinned_by=pinned_by or {})) + for step in steps: + if step.action == "copy": + # The plan copies only from fetch_from. + _copy(step.release, cast("BaseRegistry", fetch_from), store, progress=options.progress) + else: + _build(step, store, force=force and step.release == requested, options=options) + return Plan(tuple(steps)) + + +def _copy( + release: DatasetRef, fetch_from: BaseRegistry, store: LocalRegistry, *, progress: ProgressCallback | None +) -> None: + """Run one copy step. + + The plan also copies a release that ``store`` holds intact, if it must get a payload source of + that release. Then ``fetch_from`` can lack the release itself. So for such a release, this + function copies only each payload source that ``store`` lacks or holds damaged. + + Args: + release: The release of the copy step. + fetch_from: The registry to copy from. + store: The local registry to copy to. + progress: Receives the progress events of each release that is copied, or ``None``. + """ + held = _intact_manifest(store, release) + if held is None: + copy_release(release, fetch_from, store, progress=progress) + return + for pinned in held.payload_sources.values(): + if _intact_manifest(store, pinned.release) is None: + copy_release(pinned.release, fetch_from, store, progress=progress) + # The copied payload sources must hold the files that the release pins. + prepare_local_copy(store.root, release) + + +def _intact_manifest(store: LocalRegistry, reference: DatasetRef) -> Manifest | None: + """Return the manifest of a release that the store holds with every file at its recorded size. + + Args: + store: The local registry. + reference: The release. + + Returns: + The manifest, or ``None`` if ``store`` lacks the release or a file of it, or holds a file of + another size. + """ + try: + manifest = store.get_manifest(reference.dataset_id, str(reference.version)) + except TimeNetDatasetNotFoundError: + return None + return manifest if complete_copy(release_dir(store.root, reference), manifest) else None + + +def _build(step: Step, store: LocalRegistry, *, force: bool, options: BuildOptions) -> None: + """Run one build step with the backend that builds exactly its release. + + Args: + step: The build step. + store: The local registry to build into. + force: Build the release again, even if ``store`` holds it. + options: The options of the build. + + Raises: + TimeNetDatasetNotFoundError: If no installed backend builds the exact release, or if the + backend does not put the release or a payload source of it into ``store``. + """ + builder = exact_builder(step.release) + if builder is None: + raise TimeNetDatasetNotFoundError(f"no installed connector builds {step.release}") + if options.progress is not None: + options.progress(StepStarted(step.release, "build")) + builder.build_prepared(step.release, store, force=force, pinned_by=step.pinned_by, options=options) + # After a successful build, the local registry must hold the exact release with its payload sources. + prepare_local_copy(store.root, step.release) + if options.progress is not None: + options.progress(StepFinished(step.release, "build")) diff --git a/packages/timenet/src/timenet/coordinator/plan.py b/packages/timenet/src/timenet/coordinator/plan.py index fef1a385a..81c916b60 100644 --- a/packages/timenet/src/timenet/coordinator/plan.py +++ b/packages/timenet/src/timenet/coordinator/plan.py @@ -47,6 +47,13 @@ class Step: """ +@dataclass(frozen=True) +class Plan: + """The steps that get each release that a build needs, in the order in which they run.""" + + steps: tuple[Step, ...] + + @dataclass(frozen=True) class _Release: """A required release, its dependencies, and its planned action.""" @@ -69,6 +76,8 @@ class _Graph: output: WritableRegistry fetch_from: BaseRegistry | None build_parents: bool + open_only: bool + """True if each release that the walk visits must declare ``access: open``.""" searched: str """The registries that the plan looks in, for the message of a missing release.""" releases: dict[DatasetRef, _Release] = field(default_factory=dict) @@ -94,13 +103,13 @@ def visit(self, reference: DatasetRef, needed_by: DatasetRef, pinned: PayloadSou # release again with its payload sources. if found is not None and (found.opened or not opened): return - manifest, action = _locate(self.output, self.fetch_from, reference) + manifest, action = _locate(self.output, self.fetch_from, reference, open_only=self.open_only) if manifest is None: card = _card_to_build( reference, needed_by, - self.output, build_parents=self.build_parents, + open_only=self.open_only, pinned=not opened, searched=self.searched, ) @@ -185,13 +194,14 @@ def steps(self, requested: DatasetRef) -> list[Step]: ] -def plan_parents( +def plan_parents( # ruff: ignore[too-many-arguments] dataset_id: str, output: WritableRegistry, *, fetch_from: BaseRegistry | None = None, build_parents: bool = False, force_build: bool = False, + open_access_only: bool = False, ) -> list[Step]: """Plan the parent releases needed to build a dataset. @@ -203,7 +213,11 @@ def plan_parents( output: The destination registry. fetch_from: The registry to copy missing parents from, or ``None`` to copy nothing. build_parents: Build missing parents with their installed connectors. - force_build: Plan a rebuild's parents even if ``output`` holds the requested release. + force_build: Build the requested release again, even if the output holds it, and plan its + parents. + open_access_only: Check the access of each release as for an output that is not local, even + if the output is local. A caller sets it when it copies the result to a registry that + holds only open releases. Returns: The steps to copy or build parents, with dependencies first. @@ -214,8 +228,13 @@ def plan_parents( TimeFValidationError: If parents form a cycle, a connector version differs, or copying targets a remote output. TimeNetMissingParentError: If a required release is missing and ``build_parents`` is false. TimeNetDatasetNotFoundError: If a parent is missing and no installed connector builds it. - TimeNetFileError: If a local parent copy is damaged and ``fetch_from`` has no replacement. - TimeNetAccessError: If a release or its lineage lacks open access required by a registry. + TimeNetFileError: If a local output holds a damaged copy of a parent, and the plan cannot + copy the parent again. + TimeNetAccessError: If the output is not local, or ``open_access_only`` is set, and the + requested card, or a card in the lineage of the requested release that the output + holds, does not declare ``access: open``, or the plan visits a release whose card, or a + card in its lineage, does not. Also if ``fetch_from`` is not local and the plan would + copy such a release from it. """ # ruff: ignore[docstring-extraneous-exception] - raised by the helpers of the walk if fetch_from is not None and not isinstance(output, LocalRegistry): # A registry that is not local accepts a child only if it already holds the parents. A copy @@ -226,6 +245,9 @@ def plan_parents( card = find_card(dataset_id) if card is None: raise LookupError(f"no installed connector declares a card for {dataset_id!r}") + open_only = open_access_only or output.requires_open_access + if open_only: + require_open_access(card) # For a dataset without parents, the plan does not read the output, so the build makes no extra request. if not card.parents: return [] @@ -233,10 +255,11 @@ def plan_parents( # If the output already holds the requested release, a reader needs only its payload sources to # open it. Its declared parents matter only to a rebuild. held = None if force_build else _manifest(output, root) - if held is None and output.requires_open_access: - require_open_access(card) + if held is not None and open_only: + # The installed card has no lineage, so the check above does not see an ancestor that is not open. + require_open_access(held.metadata, held.lineage) searched = str(output) if fetch_from is None else f"{output} or {fetch_from}" - graph = _Graph(output, fetch_from, build_parents=build_parents, searched=searched) + graph = _Graph(output, fetch_from, build_parents=build_parents, open_only=open_only, searched=searched) # The graph holds the requested release before the walk starts. So a release that depends on it # closes a cycle, and the walk does not look it up again. if held is None: @@ -316,20 +339,24 @@ def _manifest(registry: BaseRegistry, reference: DatasetRef) -> Manifest | None: def _locate( - output: WritableRegistry, fetch_from: BaseRegistry | None, reference: DatasetRef + output: WritableRegistry, fetch_from: BaseRegistry | None, reference: DatasetRef, *, open_only: bool ) -> tuple[Manifest | None, Literal["copy"] | None]: # The damage is raised from a variable, the access error by require_open_access. # ruff: ignore[docstring-extraneous-exception] """Find a release in ``output`` or ``fetch_from``. - A local output copy is valid only if its files exist and match their recorded sizes. + In a local output, a copy with a missing file or a file of another size is damaged. If + ``open_only`` is set, the release must declare ``access: open``. Returns: ``(manifest, None)`` from ``output``, ``(manifest, "copy")`` from ``fetch_from``, or ``(None, None)`` if absent. Raises: - TimeNetFileError: If a local copy is damaged and ``fetch_from`` has no replacement. - TimeNetAccessError: If a release or its lineage lacks open access required by a registry. + TimeNetFileError: If a local output holds a damaged copy, and ``fetch_from`` does not hold + the release. + TimeNetAccessError: If the card of the release, or a card in its lineage, does not declare + ``access: open``, and ``open_only`` is set or the ``fetch_from`` registry to copy it + from is not local. """ manifest = _manifest(output, reference) damage = None @@ -344,14 +371,14 @@ def _locate( except TimeNetFileError as error: manifest, damage = None, error if manifest is not None: - if output.requires_open_access: + if open_only: require_open_access(manifest.metadata, manifest.lineage) return manifest, None fetched = None if fetch_from is None else _manifest(fetch_from, reference) if fetched is not None: # A registry that requires open access serves only releases whose cards declare access: open. # So the plan refuses another release before any download. - if fetch_from is not None and fetch_from.requires_open_access: + if open_only or (fetch_from is not None and fetch_from.requires_open_access): require_open_access(fetched.metadata, fetched.lineage) return fetched, "copy" if damage is not None: @@ -362,9 +389,9 @@ def _locate( def _card_to_build( # ruff: ignore[too-many-arguments] reference: DatasetRef, needed_by: DatasetRef, - output: WritableRegistry, *, build_parents: bool, + open_only: bool, pinned: bool, searched: str, ) -> DatasetMetadata: @@ -374,13 +401,13 @@ def _card_to_build( # ruff: ignore[too-many-arguments] The installed connector's card for the exact release in ``reference``. Raises: - TimeNetMissingParentError: If ``build_parents`` is false. - TimeNetAccessError: If the card lacks open access and the output requires it. + TimeNetMissingParentError: If ``build_parents`` is not set. + TimeNetAccessError: If ``open_only`` is set and the card does not declare ``access: open``. """ # ruff: ignore[docstring-extraneous-exception] - the access error is raised by require_open_access if not build_parents: raise TimeNetMissingParentError(reference, needed_by, searched, payload_source=pinned) card = _exact_card(reference) - if output.requires_open_access: + if open_only: require_open_access(card) return card diff --git a/packages/timenet/src/timenet/errors.py b/packages/timenet/src/timenet/errors.py index f87b91f26..a0e16ab79 100644 --- a/packages/timenet/src/timenet/errors.py +++ b/packages/timenet/src/timenet/errors.py @@ -19,6 +19,7 @@ if TYPE_CHECKING: from timenet.types.access import Access, AccessRequirement from timenet.types.references import DatasetRef + from timenet.types.version import Version class TimeNetError(Exception): @@ -161,6 +162,33 @@ class TimeFNullValuesError(TimeFValidationError): """Values contain nulls where the requested form, such as a NumPy array, cannot hold them.""" +class TimeNetVersionUnavailableError(TimeNetDatasetNotFoundError, TimeFValidationError): + """The installed connector of a dataset builds another version than the requested one. + + A connector builds only the version that its card declares. So the requested version does not + exist for the caller that wants to read it, and it is invalid input for the caller that wants to + build it. + + Attributes: + release: The requested release. + declared: The version that the connector builds. + hint: What the caller can do instead, added by the front end, or ``None``. + """ + + def __init__(self, release: "DatasetRef", declared: "Version") -> None: + super().__init__(release, declared) + self.release = release + self.declared = declared + self.hint: str | None = None + + def __str__(self) -> str: + message = ( + f"connector for {self.release.dataset_id!r} builds version {self.declared}, " + f"not the requested {self.release.version}." + ) + return message if self.hint is None else f"{message} {self.hint}" + + class TimeNetInvalidCardError(TimeNetError, ValueError): """A dataset card YAML is malformed or fails schema validation (raised while building).""" diff --git a/packages/timenet/src/timenet/testing/__init__.py b/packages/timenet/src/timenet/testing/__init__.py index c1a152f1f..a366a7d78 100644 --- a/packages/timenet/src/timenet/testing/__init__.py +++ b/packages/timenet/src/timenet/testing/__init__.py @@ -1,9 +1,10 @@ -"""Deterministic builders and assertions shared across TimeNet tests. +"""Deterministic builders, fakes, and assertions shared across TimeNet tests. This subpackage ships as real code under ``src`` (not test-directory helpers), so tests in both workspace packages can import them under pytest's ``importlib`` mode. """ +from timenet.testing.backends import install_backends from timenet.testing.builders import ( CountingLoader, assert_datasets_equal, @@ -12,4 +13,4 @@ ) -__all__ = ["CountingLoader", "assert_datasets_equal", "make_dataset", "sine_loader"] +__all__ = ["CountingLoader", "assert_datasets_equal", "install_backends", "make_dataset", "sine_loader"] diff --git a/packages/timenet/src/timenet/testing/backends.py b/packages/timenet/src/timenet/testing/backends.py new file mode 100644 index 000000000..71f4ece70 --- /dev/null +++ b/packages/timenet/src/timenet/testing/backends.py @@ -0,0 +1,38 @@ +"""A fake set of installed builder backends.""" + +from collections.abc import Callable +from dataclasses import dataclass +from typing import TYPE_CHECKING + +from timenet import builders + + +if TYPE_CHECKING: + import pytest + + +@dataclass(frozen=True) +class _FakeEntryPoint: + """An entry point that no installed distribution declares.""" + + backend: object + dist: None = None + + def load(self) -> Callable[[], object]: + """Return the factory of the backend.""" + return lambda: self.backend + + +def install_backends(monkeypatch: "pytest.MonkeyPatch", *backends: object) -> list[object]: + """Make the given builder backends the only installed ones, for the rest of the test. + + Returns: + The installed backends. Append a backend to this list to install it too. + """ + installed = list(backends) + + def entry_points(*, group: str) -> list[_FakeEntryPoint]: + return [_FakeEntryPoint(backend) for backend in installed] if group == builders.ENTRY_POINT_GROUP else [] + + monkeypatch.setattr(builders, "entry_points", entry_points) + return installed diff --git a/packages/timenet/tests/coordinator/test_build.py b/packages/timenet/tests/coordinator/test_build.py new file mode 100644 index 000000000..cdb3ef9ab --- /dev/null +++ b/packages/timenet/tests/coordinator/test_build.py @@ -0,0 +1,333 @@ +import shutil +import subprocess +import sys + +import pytest + +from timenet.builders import BuildOptions +from timenet.coordinator.build import build_release +from timenet.coordinator.plan import Step +from timenet.dataset import TimeFDataset +from timenet.errors import TimeNetAccessError, TimeNetDatasetNotFoundError, TimeNetVersionUnavailableError +from timenet.progress import StepFinished, StepStarted +from timenet.reader import TimeFReader +from timenet.registry import LocalRegistry +from timenet.registry._paths import release_dir +from timenet.registry.shared import complete_copy +from timenet.testing import install_backends, make_dataset +from timenet.types import Access +from timenet.types.reference_keys import ParentAlias, SourceKey +from timenet.types.references import DatasetRef + + +REF_AB = DatasetRef.parse("a/b@1.0.0") +PARENT_REF = DatasetRef(dataset_id=make_dataset().metadata.dataset_id, version=make_dataset().metadata.dataset_version) +CHILD_REF = DatasetRef.parse("a/child@1.0.0") +GRANDCHILD_REF = DatasetRef.parse("a/grandchild@1.0.0") +_GATED = {"access": Access.CREDENTIALED, "access_url": "https://example.org/dua"} + + +def _card(reference, **update): + """Return the card of the demo dataset under another release.""" + return make_dataset().metadata.model_copy( + update={"dataset_id": reference.dataset_id, "dataset_version": reference.version, **update} + ) + + +def _child_card(**update): + return _card(CHILD_REF, parents={ParentAlias("parent"): PARENT_REF}, **update) + + +def _grandchild_card(): + return _card(GRANDCHILD_REF, parents={ParentAlias("child"): CHILD_REF}) + + +def _demo(card): + """Return the demo dataset under ``card``.""" + dataset = make_dataset() + dataset._metadata = card + return dataset + + +class _Backend: + """A builder backend of another package. It builds the demo dataset under each card that it holds.""" + + def __init__(self, *cards): + self.cards = {card.dataset_id: card for card in cards} + self.calls = [] + + def dataset_ids(self): + return list(self.cards) + + def card(self, dataset_id): + return self.cards.get(dataset_id) + + def build_prepared(self, release, store, *, force, pinned_by, options): + self.calls.append((release, force, dict(pinned_by))) + store.put_dataset(_demo(self.cards[release.dataset_id]), force=force, pinned_by=pinned_by or None) + + +@pytest.fixture +def backends(monkeypatch): + """Return the list of the installed builder backends, which a test fills.""" + return install_backends(monkeypatch) + + +@pytest.fixture +def store(tmp_path): + return LocalRegistry(tmp_path / "store") + + +@pytest.fixture +def fake(backends): + backend = _Backend(_card(REF_AB)) + backends.append(backend) + return backend + + +@pytest.fixture +def fake_parent(backends): + backend = _Backend(make_dataset().metadata) + backends.append(backend) + return backend + + +@pytest.fixture +def fake_child(backends): + backend = _Backend(_child_card()) + backends.append(backend) + return backend + + +def _store_child_without_parent(registry): + """Store the child, which reads a Record of the parent, without the parent. + + Returns: + The pins of the child on the files of the parent. + """ + parent = make_dataset() + registry.put_dataset(parent) + child = TimeFDataset(metadata=_child_card()) + with TimeFReader(registry.open_version(PARENT_REF.dataset_id, str(PARENT_REF.version))) as reader: + child.import_from(reader, alias=ParentAlias("parent"), records=[parent.records[0].id]) + registry.put_dataset(child) + shutil.rmtree(registry.root / PARENT_REF.dataset_id) + return registry.get_manifest(CHILD_REF.dataset_id).payload_sources[SourceKey(str(PARENT_REF))] + + +@pytest.fixture +def pins(store): + """Store the child without the parent, and return the pins of the child on the files of the parent.""" + return _store_child_without_parent(store) + + +def test_build_release_builds_into_the_store_and_returns_the_plan(store, fake): + plan = build_release("a/b", store=store) + assert [(s.action, s.release) for s in plan.steps] == [("build", REF_AB)] + assert store.exists("a/b", "1.0.0") + assert fake.calls == [(REF_AB, False, {})] + + +def test_build_release_calls_the_claiming_backend_with_the_pins(store, fake_parent, fake_child, pins): + plan = build_release(CHILD_REF.dataset_id, store=store, build_parents=True) + # (release, force, pinned_by): the backend of another package gets the pins of the child. + assert fake_parent.calls == [(PARENT_REF, False, {CHILD_REF: pins})] + # The store holds the child, so the plan builds only its missing payload source. + assert fake_child.calls == [] + assert plan.steps == (Step(PARENT_REF, "build", pinned_by={CHILD_REF: pins}),) + assert store.exists(PARENT_REF.dataset_id, str(PARENT_REF.version)) + + +def test_build_release_passes_the_pins_of_the_caller_to_the_backend(store, fake_parent, pins): + plan = build_release(PARENT_REF.dataset_id, store=store, pinned_by={CHILD_REF: pins}) + assert fake_parent.calls == [(PARENT_REF, False, {CHILD_REF: pins})] + assert plan.steps == (Step(PARENT_REF, "build", pinned_by={CHILD_REF: pins}),) + assert store.exists(PARENT_REF.dataset_id, str(PARENT_REF.version)) + + +def test_build_release_rejects_a_version_the_connector_does_not_declare(store, fake): + with pytest.raises(TimeNetVersionUnavailableError) as caught: + build_release("a/b@2.0.0", store=store) + assert (caught.value.release, caught.value.declared) == (DatasetRef.parse("a/b@2.0.0"), REF_AB.version) + assert str(caught.value) == "connector for 'a/b' builds version 1.0.0, not the requested 2.0.0." + assert fake.calls == [] + + +def test_build_release_accepts_the_version_that_the_connector_declares(store, fake): + build_release("a/b@1.0.0", store=store) + assert fake.calls == [(REF_AB, False, {})] + + +def test_build_release_reports_each_step(store, fake): + events = [] + build_release("a/b", store=store, options=BuildOptions(progress=events.append)) + assert events == [StepStarted(REF_AB, "build"), StepFinished(REF_AB, "build")] + + +def test_build_release_copies_a_missing_parent_and_reports_each_step_once(tmp_path, store, fake_parent, fake_child): + origin = LocalRegistry(tmp_path / "origin") + origin.put_dataset(make_dataset()) + events = [] + + plan = build_release( + CHILD_REF.dataset_id, store=store, fetch_from=origin, options=BuildOptions(progress=events.append) + ) + + assert plan.steps == (Step(PARENT_REF, "copy"), Step(CHILD_REF, "build", depends_on=(PARENT_REF,))) + assert events == [ + StepStarted(PARENT_REF, "copy"), + StepFinished(PARENT_REF, "copy"), + StepStarted(CHILD_REF, "build"), + StepFinished(CHILD_REF, "build"), + ] + assert fake_parent.calls == [] + assert store.exists(PARENT_REF.dataset_id, str(PARENT_REF.version)) + + +@pytest.mark.usefixtures("fake_child") +@pytest.mark.parametrize("payload_source", ["missing", "damaged"]) +def test_build_release_reuses_a_held_release_and_copies_only_its_payload_source( + tmp_path, store, fake_parent, pins, payload_source +): + # The store holds the child, but not an intact copy of its payload source. The registry to copy + # from holds only the payload source. + origin = LocalRegistry(tmp_path / "origin") + origin.put_dataset(make_dataset()) + if payload_source == "damaged": + store.put_dataset(make_dataset()) + (release_dir(store.root, PARENT_REF) / "control.duckdb").write_bytes(b"cut short") + events = [] + + plan = build_release( + CHILD_REF.dataset_id, store=store, fetch_from=origin, options=BuildOptions(progress=events.append) + ) + + assert plan.steps == (Step(CHILD_REF, "copy"),) + assert events == [StepStarted(PARENT_REF, "copy"), StepFinished(PARENT_REF, "copy")] + assert fake_parent.calls == [] + assert complete_copy(release_dir(store.root, PARENT_REF), origin.get_manifest(PARENT_REF.dataset_id)) + + +def test_build_release_copies_a_release_whose_payload_source_only_the_store_holds( + tmp_path, store, fake_parent, fake_child, backends +): + # The registry to copy from holds the child, but not the parent whose files the child reads. + origin = LocalRegistry(tmp_path / "origin") + _store_child_without_parent(origin) + store.put_dataset(make_dataset()) + fake_grandchild = _Backend(_grandchild_card()) + backends.append(fake_grandchild) + + plan = build_release(GRANDCHILD_REF.dataset_id, store=store, fetch_from=origin) + + assert plan.steps == (Step(CHILD_REF, "copy"), Step(GRANDCHILD_REF, "build", depends_on=(CHILD_REF,))) + assert fake_parent.calls == fake_child.calls == [] + assert fake_grandchild.calls == [(GRANDCHILD_REF, False, {})] + assert store.get_manifest(CHILD_REF.dataset_id) == origin.get_manifest(CHILD_REF.dataset_id) + + +def test_build_release_builds_a_payload_source_of_a_release_that_it_copies( + tmp_path, store, fake_parent, fake_child, backends +): + # The registry to copy from holds the child, but not the parent whose files the child reads. + origin = LocalRegistry(tmp_path / "origin") + pins = _store_child_without_parent(origin) + fake_grandchild = _Backend(_grandchild_card()) + backends.append(fake_grandchild) + + plan = build_release(GRANDCHILD_REF.dataset_id, store=store, fetch_from=origin, build_parents=True) + + assert plan.steps == ( + Step(PARENT_REF, "build", pinned_by={CHILD_REF: pins}), + Step(CHILD_REF, "copy", depends_on=(PARENT_REF,)), + Step(GRANDCHILD_REF, "build", depends_on=(CHILD_REF,)), + ) + assert fake_parent.calls == [(PARENT_REF, False, {CHILD_REF: pins})] + assert fake_child.calls == [] + assert fake_grandchild.calls == [(GRANDCHILD_REF, False, {})] + assert store.get_manifest(CHILD_REF.dataset_id) == origin.get_manifest(CHILD_REF.dataset_id) + + +def test_build_release_forces_only_the_requested_release(store, fake_parent, fake_child): + store.put_dataset(_demo(_child_card())) + + plan = build_release(CHILD_REF.dataset_id, store=store, build_parents=True, force=True) + + assert [(s.action, s.release) for s in plan.steps] == [("build", PARENT_REF), ("build", CHILD_REF)] + assert fake_parent.calls == [(PARENT_REF, False, {})] + assert fake_child.calls == [(CHILD_REF, True, {})] + + +def test_build_release_skips_a_release_the_store_holds(store, fake): + store.put_dataset(_demo(_card(REF_AB))) + plan = build_release("a/b", store=store) + assert plan.steps == () + assert fake.calls == [] + + +def test_build_release_with_open_access_only_refuses_a_credentialed_card(store, backends): + fake_credentialed = _Backend(_card(REF_AB, **_GATED)) + backends.append(fake_credentialed) + + with pytest.raises(TimeNetAccessError) as caught: + build_release("a/b", store=store, open_access_only=True) + + assert caught.value.dataset == REF_AB + assert fake_credentialed.calls == [] + # Without the flag, the local store takes a release that is not open. + build_release("a/b", store=store) + assert fake_credentialed.calls == [(REF_AB, False, {})] + + +@pytest.mark.parametrize("held", [False, True]) +def test_build_release_with_open_access_only_refuses_a_credentialed_parent(store, backends, held): + gated = _Backend(make_dataset().metadata.model_copy(update=_GATED)) + child = _Backend(_child_card()) + backends.extend([gated, child]) + if held: + store.put_dataset(_demo(gated.cards[PARENT_REF.dataset_id])) + + with pytest.raises(TimeNetAccessError) as caught: + build_release(CHILD_REF.dataset_id, store=store, build_parents=True, open_access_only=True) + + assert caught.value.dataset == PARENT_REF + assert gated.calls == child.calls == [] + + +def test_build_release_with_open_access_only_refuses_a_held_release_with_a_credentialed_ancestor(store, backends): + # The card of the child is open. The child keeps a Record of the open release a/b, which the store + # lacks. It reads nothing from the credentialed parent, which is only in its lineage. + child_card = _card(CHILD_REF, parents={ParentAlias("parent"): PARENT_REF, ParentAlias("open"): REF_AB}) + other, child = _Backend(_card(REF_AB)), _Backend(child_card) + backends.extend([other, child]) + store.put_dataset(_demo(make_dataset().metadata.model_copy(update=_GATED))) + store.put_dataset(_demo(_card(REF_AB))) + dataset = TimeFDataset(metadata=child_card) + with ( + TimeFReader(store.open_version(PARENT_REF.dataset_id, str(PARENT_REF.version))) as gated_reader, + TimeFReader(store.open_version(REF_AB.dataset_id, str(REF_AB.version))) as open_reader, + ): + dataset.import_from(gated_reader, alias=ParentAlias("parent")) + dataset.import_from(open_reader, alias=ParentAlias("open"), records=[make_dataset().records[0].id]) + store.put_dataset(dataset) + shutil.rmtree(store.root / PARENT_REF.dataset_id) + shutil.rmtree(release_dir(store.root, REF_AB)) + held = store.get_manifest(CHILD_REF.dataset_id) + assert [card.source for card in held.lineage] == [PARENT_REF, REF_AB] + assert list(held.payload_sources) == [SourceKey(str(REF_AB))] + + with pytest.raises(TimeNetAccessError) as caught: + build_release(CHILD_REF.dataset_id, store=store, build_parents=True, open_access_only=True) + + assert caught.value.dataset == CHILD_REF + assert other.calls == child.calls == [] + + +def test_build_release_without_a_backend_raises_not_found(store, backends): + with pytest.raises(TimeNetDatasetNotFoundError, match="no installed connector builds 'a/b'"): + build_release("a/b", store=store) + + +def test_the_build_coordinator_imports_nothing_from_timenet_connectors(): + code = "import sys, timenet.coordinator.build; assert 'timenet_connectors' not in sys.modules" + subprocess.run([sys.executable, "-c", code], check=True) diff --git a/packages/timenet/tests/test_builders.py b/packages/timenet/tests/test_builders.py index 06cd0c53a..012a9a322 100644 --- a/packages/timenet/tests/test_builders.py +++ b/packages/timenet/tests/test_builders.py @@ -1,41 +1,43 @@ -from pathlib import Path - from timenet import builders -from timenet.builders import find_builder, find_card -from timenet.testing import make_dataset +from timenet.builders import exact_builder, find_builder, find_card +from timenet.testing import install_backends, make_dataset +from timenet.types.references import DatasetRef class _Backend: - def __init__(self, known, card=None): + def __init__(self, *known, card=None): self._known = known self._card = card - def knows(self, dataset_id): - return dataset_id == self._known + def dataset_ids(self): + return iter(self._known) def card(self, dataset_id): return self._card - def build(self, dataset_id, root, *, force=False): - return Path(root) / dataset_id + def build_prepared(self, release, store, *, force, pinned_by, options): + raise AssertionError(f"the lookup does not build {release}") -class _Entry: - def __init__(self, backend): - self._backend = backend +def test_find_builder_returns_none_when_nothing_claims_the_id(monkeypatch): + install_backends(monkeypatch, _Backend("a/b")) + assert find_builder("x/y") is None - def load(self): - return lambda: self._backend +def test_find_builder_returns_the_first_backend_whose_dataset_ids_hold_the_id(monkeypatch): + wanted = _Backend("a/c", "x/y") + install_backends(monkeypatch, _Backend("a/b"), wanted, _Backend("x/y")) + assert find_builder("x/y") is wanted -def test_find_builder_returns_none_when_nothing_claims_the_id(monkeypatch): - monkeypatch.setattr(builders, "entry_points", lambda group: [_Entry(_Backend("a/b"))]) - assert find_builder("x/y") is None +def test_find_builder_asks_dataset_ids(monkeypatch): + class _KnowsEverything(_Backend): + def knows(self, dataset_id): + return True -def test_find_builder_returns_the_backend_that_claims_the_id(monkeypatch): + # The first backend says yes to every ID through knows(), but its dataset IDs do not hold this one. wanted = _Backend("x/y") - monkeypatch.setattr(builders, "entry_points", lambda group: [_Entry(_Backend("a/b")), _Entry(wanted)]) + install_backends(monkeypatch, _KnowsEverything("a/b"), wanted) assert find_builder("x/y") is wanted @@ -46,7 +48,16 @@ def test_find_builder_returns_none_with_no_registered_backends(monkeypatch): def test_find_card_returns_the_card_of_the_backend_that_claims_the_id(monkeypatch): card = make_dataset().metadata - backends = [_Backend("a/b"), _Backend(card.dataset_id, card)] - monkeypatch.setattr(builders, "entry_points", lambda group: [_Entry(backend) for backend in backends]) + backends = [_Backend("a/b"), _Backend(card.dataset_id, card=card)] + install_backends(monkeypatch, *backends) assert find_card(card.dataset_id) is card assert find_card("x/y") is None + + +def test_exact_builder_returns_the_backend_only_for_the_version_of_its_card(monkeypatch): + card = make_dataset().metadata + backend = _Backend(card.dataset_id, card=card) + install_backends(monkeypatch, backend) + assert exact_builder(DatasetRef(dataset_id=card.dataset_id, version=card.dataset_version)) is backend + assert exact_builder(DatasetRef.parse(f"{card.dataset_id}@2.0.0")) is None + assert exact_builder(DatasetRef.parse("x/y@1.0.0")) is None diff --git a/packages/timenet/tests/test_errors.py b/packages/timenet/tests/test_errors.py index 075de37d1..136b9bf9f 100644 --- a/packages/timenet/tests/test_errors.py +++ b/packages/timenet/tests/test_errors.py @@ -16,8 +16,9 @@ TimeNetMissingParentError, TimeNetRegistryError, TimeNetVersionConflictError, + TimeNetVersionUnavailableError, ) -from timenet.types import Access +from timenet.types import Access, Version from timenet.types.access import AccessRequirement from timenet.types.references import DatasetRef @@ -132,6 +133,11 @@ def test_catchable_as_base(): "registry here already holds org/a@1.0.0, a payload source of org/b@2.0.0, with another card than " "registry there. A published version cannot change, so a changed release needs a new version.", ), + ( + TimeNetVersionUnavailableError(_B, Version(1, 0, 0)), + {"release": _B, "declared": Version(1, 0, 0)}, + "connector for 'org/b' builds version 1.0.0, not the requested 2.0.0.", + ), ], ) def test_an_error_with_facts_ends_with_the_hint_of_its_front_end(error, facts, message): @@ -142,3 +148,10 @@ def test_an_error_with_facts_ends_with_the_hint_of_its_front_end(error, facts, m assert {name: getattr(restored, name) for name in facts} == facts # The message ends with a period, and the hint follows as its own sentence. assert str(restored) == f"{message} Do this instead." + + +def test_a_version_that_the_connector_does_not_build_is_not_found_and_invalid(): + error = TimeNetVersionUnavailableError(_B, Version(1, 0, 0)) + # The client reports a missing version, and a build reports invalid input. Both catch it. + assert isinstance(error, TimeNetDatasetNotFoundError) + assert isinstance(error, TimeFValidationError) From 297f36b0b5ab7669b1346128e03b996723e3759f Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 06:36:52 +0100 Subject: [PATCH 2/9] fix(build): add BuildOptions.quiet and name the fix in the version error BuildOptions gets quiet. It asks the backend to print nothing of its own. An isolated child otherwise prints its build lines and downloads to the inherited stderr. Before, a backend could only guess this from a missing progress callback, so an SDK build with no callback silenced the child too. The protocol docstring of build_prepared() names quiet and the progress callback, and names TimeNetVersionUnavailableError for another version. TimeNetVersionUnavailableError now says what to do: ask for the version that the connector builds, or leave out the version. The hint attribute stays for front ends. Part of TNET-157. --- packages/timenet/src/timenet/builders.py | 10 ++++++++-- packages/timenet/src/timenet/errors.py | 3 ++- packages/timenet/tests/coordinator/test_build.py | 5 ++++- packages/timenet/tests/test_builders.py | 8 +++++++- packages/timenet/tests/test_errors.py | 3 ++- 5 files changed, 23 insertions(+), 6 deletions(-) diff --git a/packages/timenet/src/timenet/builders.py b/packages/timenet/src/timenet/builders.py index 4daefdeae..7e77fb465 100644 --- a/packages/timenet/src/timenet/builders.py +++ b/packages/timenet/src/timenet/builders.py @@ -32,10 +32,13 @@ class BuildOptions: Attributes: keep_cache: Keep the raw download cache after the build. progress: Receives the progress events of the build, or ``None``. + quiet: Ask the backend to print nothing of its own. An isolated child otherwise prints its + build lines and downloads to the inherited stderr. """ keep_cache: bool = False progress: ProgressCallback | None = None + quiet: bool = False class BuilderBackend(Protocol): @@ -162,11 +165,14 @@ def build_prepared( store: The local registry to build into. force: Build the release again, even if ``store`` already holds it. pinned_by: The releases that already read files of this release, with their pins. - options: The options of the build that do not change its files. + options: The options of the build that do not change its files. The backend sends the + progress events of the build to ``options.progress``. If ``options.quiet`` is set, + the backend prints nothing of its own, also not in an isolated child. Raises: TimeNetDatasetNotFoundError: If this backend does not know the dataset ID of ``release``. - TimeFValidationError: If the connector declares another version than ``release``. + TimeNetVersionUnavailableError: If the connector declares another version than + ``release``. It is also a ``TimeFValidationError``. TimeFFormatError: If the build does not reproduce a pinned file. Nothing is published then. TimeNetBuildError: If the build runs but fails. diff --git a/packages/timenet/src/timenet/errors.py b/packages/timenet/src/timenet/errors.py index a0e16ab79..f1ecb7994 100644 --- a/packages/timenet/src/timenet/errors.py +++ b/packages/timenet/src/timenet/errors.py @@ -184,7 +184,8 @@ def __init__(self, release: "DatasetRef", declared: "Version") -> None: def __str__(self) -> str: message = ( f"connector for {self.release.dataset_id!r} builds version {self.declared}, " - f"not the requested {self.release.version}." + f"not the requested {self.release.version}. " + f"Ask for version {self.declared}, or leave out the version." ) return message if self.hint is None else f"{message} {self.hint}" diff --git a/packages/timenet/tests/coordinator/test_build.py b/packages/timenet/tests/coordinator/test_build.py index cdb3ef9ab..9d0a67157 100644 --- a/packages/timenet/tests/coordinator/test_build.py +++ b/packages/timenet/tests/coordinator/test_build.py @@ -149,7 +149,10 @@ def test_build_release_rejects_a_version_the_connector_does_not_declare(store, f with pytest.raises(TimeNetVersionUnavailableError) as caught: build_release("a/b@2.0.0", store=store) assert (caught.value.release, caught.value.declared) == (DatasetRef.parse("a/b@2.0.0"), REF_AB.version) - assert str(caught.value) == "connector for 'a/b' builds version 1.0.0, not the requested 2.0.0." + assert str(caught.value) == ( + "connector for 'a/b' builds version 1.0.0, not the requested 2.0.0. " + "Ask for version 1.0.0, or leave out the version." + ) assert fake.calls == [] diff --git a/packages/timenet/tests/test_builders.py b/packages/timenet/tests/test_builders.py index 012a9a322..4c99d7e75 100644 --- a/packages/timenet/tests/test_builders.py +++ b/packages/timenet/tests/test_builders.py @@ -1,5 +1,5 @@ from timenet import builders -from timenet.builders import exact_builder, find_builder, find_card +from timenet.builders import BuildOptions, exact_builder, find_builder, find_card from timenet.testing import install_backends, make_dataset from timenet.types.references import DatasetRef @@ -61,3 +61,9 @@ def test_exact_builder_returns_the_backend_only_for_the_version_of_its_card(monk assert exact_builder(DatasetRef(dataset_id=card.dataset_id, version=card.dataset_version)) is backend assert exact_builder(DatasetRef.parse(f"{card.dataset_id}@2.0.0")) is None assert exact_builder(DatasetRef.parse("x/y@1.0.0")) is None + + +def test_build_options_let_the_backend_print_by_default(): + # The CLI sets quiet for --quiet. The SDK leaves it unset, so an isolated child prints its lines. + assert BuildOptions().quiet is False + assert BuildOptions(quiet=True).quiet is True diff --git a/packages/timenet/tests/test_errors.py b/packages/timenet/tests/test_errors.py index 136b9bf9f..e5b2fbcff 100644 --- a/packages/timenet/tests/test_errors.py +++ b/packages/timenet/tests/test_errors.py @@ -136,7 +136,8 @@ def test_catchable_as_base(): ( TimeNetVersionUnavailableError(_B, Version(1, 0, 0)), {"release": _B, "declared": Version(1, 0, 0)}, - "connector for 'org/b' builds version 1.0.0, not the requested 2.0.0.", + "connector for 'org/b' builds version 1.0.0, not the requested 2.0.0. " + "Ask for version 1.0.0, or leave out the version.", ), ], ) From 0f9dad9d47b206b74e6417c2416b01ffd3fdba34 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 13:44:01 +0100 Subject: [PATCH 3/9] test(build): say what the card backend of another package stands for The docstring now says this backend is not ConnectorBuilder and the plan never builds through it. Part of TNET-157. --- packages/timenet-connectors/tests/test_build_plan.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/timenet-connectors/tests/test_build_plan.py b/packages/timenet-connectors/tests/test_build_plan.py index b93ff5e6e..dbcc89edf 100644 --- a/packages/timenet-connectors/tests/test_build_plan.py +++ b/packages/timenet-connectors/tests/test_build_plan.py @@ -43,7 +43,7 @@ class _CardBackend: - """A builder backend that serves connector cards by dataset ID.""" + """A backend of another package, not ConnectorBuilder: it serves cards by dataset ID, and the plan never builds through it.""" def __init__(self, cards): self.cards = cards From 357155642d445677be4c6a9ecdbbf507e29730ad Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 13:56:10 +0100 Subject: [PATCH 4/9] docs(coordinator): say that the coordinator is internal Say that the coordinator is internal to TimeNet and point users to the build command and the client. Part of TNET-157. --- packages/timenet/src/timenet/coordinator/__init__.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/packages/timenet/src/timenet/coordinator/__init__.py b/packages/timenet/src/timenet/coordinator/__init__.py index 7e1691695..e71c81d3c 100644 --- a/packages/timenet/src/timenet/coordinator/__init__.py +++ b/packages/timenet/src/timenet/coordinator/__init__.py @@ -1 +1,6 @@ -"""Coordinate dataset builds and reads.""" +"""Plan and run builds and reads. + +The functions of this package, for example ``build_release()``, are internal to TimeNet's own +packages. They make no promise of a stable interface. Users build a dataset with the +``timenet-build`` command and read one with ``TimeNet``. +""" From 52f5f65a4afda653b7b6acf32d8b906c55e8dfa6 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 13:59:56 +0100 Subject: [PATCH 5/9] refactor(build): use dataset_ids() in the build command and trim the protocol docstrings The build command now asks ConnectorBuilder().dataset_ids() for the known dataset IDs, as any other caller of the builder protocol does. So known_ids() is now private and is called _known_ids(). The BuilderBackend docstring now states the contract for an implementer, and the card() docstring and the summary of the coordinator build module say less about the planner. Part of TNET-157. --- .../src/timenet_connectors/builder/backend.py | 6 +++--- .../src/timenet_connectors/builder/cli.py | 8 +++++--- .../src/timenet_connectors/discovery.py | 4 ++-- packages/timenet-connectors/tests/test_build_backend.py | 4 ++-- packages/timenet-connectors/tests/test_build_plan.py | 8 ++++---- packages/timenet/src/timenet/builders.py | 6 ++---- packages/timenet/src/timenet/coordinator/build.py | 2 +- 7 files changed, 19 insertions(+), 19 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py index 01473e5d2..f01a81ca1 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py @@ -8,7 +8,7 @@ from timenet.types import DatasetMetadata from timenet.types.references import DatasetRef from timenet_connectors.builder.cli import build_one -from timenet_connectors.discovery import connector_dir, has_connector, known_ids +from timenet_connectors.discovery import _known_ids, connector_dir, has_connector class ConnectorBuilder: @@ -22,8 +22,8 @@ def dataset_ids(self) -> list[str]: # ruff: ignore[no-self-use] (BuilderBackend Raises: TimeNetInvalidCardError: If a card cannot be read or is not a mapping. - """ # ruff: ignore[docstring-extraneous-exception] - raised by known_ids - return known_ids() + """ # ruff: ignore[docstring-extraneous-exception] - raised by _known_ids + return _known_ids() def knows(self, dataset_id: str) -> bool: # ruff: ignore[no-self-use] (BuilderBackend protocol method) """Report whether a connector package exists for the id, without importing it. diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index 7286534f7..afed40f7d 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -49,7 +49,7 @@ from timenet.types.references import DatasetRef from timenet.writer.progress import ProgressStage, WriteProgressEvent from timenet_connectors.builder.env import run_isolated -from timenet_connectors.discovery import known_ids, resolve +from timenet_connectors.discovery import resolve from timenet_connectors.download import DownloadProgress, ProgressCallback, progress_sink @@ -147,8 +147,10 @@ def build( TimeNetAccessError: If the output is remote and gets a release without access declared ``open``. """ - # known_ids imports no connector. So an unknown id fails before any environment exists. - known = known_ids() + from timenet_connectors.builder.backend import ConnectorBuilder # ruff: ignore[import-outside-top-level] + + # dataset_ids() imports no connector. So an unknown id fails before any environment exists. + known = list(ConnectorBuilder().dataset_ids()) if dataset_id not in known: raise typer.BadParameter(f"no connector for {dataset_id!r}; known: {', '.join(known) or '(none)'}") forwarded = out if out is not None else settings().registry diff --git a/packages/timenet-connectors/src/timenet_connectors/discovery.py b/packages/timenet-connectors/src/timenet_connectors/discovery.py index 7da09abf7..5f2d933c5 100644 --- a/packages/timenet-connectors/src/timenet_connectors/discovery.py +++ b/packages/timenet-connectors/src/timenet_connectors/discovery.py @@ -48,7 +48,7 @@ def connector_dir(dataset_id: str) -> Path: path = Path(next(iter(spec.submodule_search_locations))) if (path / "dataset.yaml").is_file(): return path - raise LookupError(f"no connector for {dataset_id!r}; known: {', '.join(known_ids()) or '(none)'}") + raise LookupError(f"no connector for {dataset_id!r}; known: {', '.join(_known_ids()) or '(none)'}") def has_connector(dataset_id: str) -> bool: @@ -86,7 +86,7 @@ def requirements_for(dataset_id: str) -> Path | None: return path if path.is_file() else None -def known_ids() -> list[str]: +def _known_ids() -> list[str]: """List the dataset ids declared by the connector cards on disk, importing nothing. :func:`available` answers the same question by importing every connector. The builder backend diff --git a/packages/timenet-connectors/tests/test_build_backend.py b/packages/timenet-connectors/tests/test_build_backend.py index a63fdaa13..db99cfd08 100644 --- a/packages/timenet-connectors/tests/test_build_backend.py +++ b/packages/timenet-connectors/tests/test_build_backend.py @@ -15,7 +15,7 @@ from timenet_connectors import datasets as datasets_module from timenet_connectors.builder import backend as backend_module, cli as cli_module from timenet_connectors.builder.backend import ConnectorBuilder -from timenet_connectors.discovery import known_ids +from timenet_connectors.discovery import _known_ids def _unexpected_isolation(*args, **kwargs): @@ -73,7 +73,7 @@ def test_a_card_that_cannot_be_read_fails_each_lookup_with_the_card_error(tmp_pa expected = re.escape(message.format(tmp_path / "test" / "broken" / "dataset.yaml")) with pytest.raises(TimeNetInvalidCardError, match=expected): - known_ids() + _known_ids() # The installed entry point loads the builder backend of this package, which lists its dataset IDs. with pytest.raises(TimeNetInvalidCardError, match=expected): find_card("test/good") diff --git a/packages/timenet-connectors/tests/test_build_plan.py b/packages/timenet-connectors/tests/test_build_plan.py index dbcc89edf..ffee95f68 100644 --- a/packages/timenet-connectors/tests/test_build_plan.py +++ b/packages/timenet-connectors/tests/test_build_plan.py @@ -249,7 +249,7 @@ def test_the_plan_reads_each_card_from_the_backend_that_claims_the_dataset(tmp_p def test_a_parent_whose_card_cannot_be_read_fails_the_plan_with_the_card_error(tmp_path, monkeypatch): (tmp_path / "dataset.yaml").write_text("dataset_id: [unclosed\n") # The in-repo backend claims PARENT, whose card cannot be read. The second backend serves CHILD. - monkeypatch.setattr(backend_module, "known_ids", lambda: [PARENT.dataset_id]) + monkeypatch.setattr(backend_module, "_known_ids", lambda: [PARENT.dataset_id]) monkeypatch.setattr(backend_module, "connector_dir", lambda dataset_id: tmp_path) install_backends(monkeypatch, ConnectorBuilder(), _CardBackend(_cards(child={"parent": PARENT}))) @@ -465,7 +465,7 @@ def run_isolated(dataset_id, root, *, force=False, **_): def test_build_prepares_parents_before_the_isolated_child(tmp_path, monkeypatch): cards = _install_cards(monkeypatch, child={"parent": PARENT, "grand": GRAND}, parent={}) - monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) + monkeypatch.setattr(backend_module, "_known_ids", lambda: list(cards)) monkeypatch.delenv("TIMENET_ISOLATION", raising=False) origin = LocalRegistry(tmp_path / "origin") origin.put_dataset(make_dataset()) @@ -623,7 +623,7 @@ def child(command, env): monkeypatch.delenv("TIMENET_ISOLATION", raising=False) monkeypatch.setattr(env_module, "requirements_for", lambda dataset_id: None) monkeypatch.setattr(env_module, "_run_build", child) - monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) + monkeypatch.setattr(backend_module, "_known_ids", lambda: list(cards)) monkeypatch.setattr(cli_module, "resolve", lambda dataset_id: _Grand) monkeypatch.setattr(cli_module, "run_pipeline", run_pipeline) @@ -754,7 +754,7 @@ def test_a_proxy_backend_builds_a_parent_through_the_build_of_its_instance(tmp_p @pytest.mark.parametrize("case", ["missing", "damaged", "not-open"]) def test_the_cli_hint_names_its_flags(tmp_path, monkeypatch, case): cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) - monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) + monkeypatch.setattr(backend_module, "_known_ids", lambda: list(cards)) out = LocalRegistry(tmp_path / "out") if case == "damaged": out.put_dataset(TimeFDataset(metadata=cards[PARENT.dataset_id])) diff --git a/packages/timenet/src/timenet/builders.py b/packages/timenet/src/timenet/builders.py index 7e77fb465..f82ec7343 100644 --- a/packages/timenet/src/timenet/builders.py +++ b/packages/timenet/src/timenet/builders.py @@ -44,10 +44,8 @@ class BuildOptions: class BuilderBackend(Protocol): """Something that can build a dataset release from its connector. - The build coordinator plans the build first. It puts each parent of a release, and each payload - source of such a parent, into the local store before it calls :meth:`build_prepared` for that - release. So :meth:`build_prepared` builds exactly one release, and it does not plan, copy or - build other releases. + :meth:`build_prepared` builds exactly one release, into a store that already holds the parents + of that release and their payload sources. It does not plan, copy or build other releases. """ def dataset_ids(self) -> Iterable[str]: diff --git a/packages/timenet/src/timenet/coordinator/build.py b/packages/timenet/src/timenet/coordinator/build.py index 7b612aecc..fd4cbf5ce 100644 --- a/packages/timenet/src/timenet/coordinator/build.py +++ b/packages/timenet/src/timenet/coordinator/build.py @@ -1,4 +1,4 @@ -"""Build a release and each release that its build needs, through one coordinator. +"""Build a release and each parent release that it depends on, through one coordinator. The coordinator does three things in order. First, it finds the installed builder backend that claims the dataset, and it checks a requested version against the card of the connector. Then it From b92463508952323e6c598bcfcbdf704f6f201369 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 18:01:13 +0100 Subject: [PATCH 6/9] docs: shorten the docstrings and docs of the release build Say only what a caller of the release build needs to know. Part of TNET-157. --- .../src/timenet_connectors/discovery.py | 4 +- packages/timenet/src/timenet/builders.py | 31 ++++------ .../src/timenet/coordinator/__init__.py | 5 +- .../timenet/src/timenet/coordinator/build.py | 57 ++++++------------- .../timenet/src/timenet/coordinator/plan.py | 14 ++--- packages/timenet/src/timenet/errors.py | 4 +- 6 files changed, 38 insertions(+), 77 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/discovery.py b/packages/timenet-connectors/src/timenet_connectors/discovery.py index 5f2d933c5..6fa5c4049 100644 --- a/packages/timenet-connectors/src/timenet_connectors/discovery.py +++ b/packages/timenet-connectors/src/timenet_connectors/discovery.py @@ -89,9 +89,7 @@ def requirements_for(dataset_id: str) -> Path | None: def _known_ids() -> list[str]: """List the dataset ids declared by the connector cards on disk, importing nothing. - :func:`available` answers the same question by importing every connector. The builder backend - and the unknown-id error path cannot afford that: those dependencies live in each connector's - own environment, not this one. A dataset id matches exactly as its card declares it. + A dataset id matches exactly as its card declares it. Returns: The declared dataset ids, sorted. diff --git a/packages/timenet/src/timenet/builders.py b/packages/timenet/src/timenet/builders.py index f82ec7343..369cc01fe 100644 --- a/packages/timenet/src/timenet/builders.py +++ b/packages/timenet/src/timenet/builders.py @@ -32,8 +32,7 @@ class BuildOptions: Attributes: keep_cache: Keep the raw download cache after the build. progress: Receives the progress events of the build, or ``None``. - quiet: Ask the backend to print nothing of its own. An isolated child otherwise prints its - build lines and downloads to the inherited stderr. + quiet: Ask the backend to print nothing of its own, also from an isolated child process. """ keep_cache: bool = False @@ -42,17 +41,12 @@ class BuildOptions: class BuilderBackend(Protocol): - """Something that can build a dataset release from its connector. - - :meth:`build_prepared` builds exactly one release, into a store that already holds the parents - of that release and their payload sources. It does not plan, copy or build other releases. - """ + """Something that can build a dataset release from its connector.""" def dataset_ids(self) -> Iterable[str]: """Return the IDs of the datasets that this backend can build. - Implementations must answer without importing a connector: this runs before any connector - environment exists. + Implementations must answer without importing a connector. Returns: The dataset IDs. @@ -152,26 +146,22 @@ def build_prepared( ) -> None: """Build exactly this release into the store. - Each parent of the release, and each payload source of such a parent, is already in - ``store`` when this method runs. If ``pinned_by`` is not empty, other releases already - read files of this release. The build then publishes the release only if it writes each - pinned file again with the same path, size and checksum. Otherwise it publishes nothing - and raises an error. + The store already holds each parent of the release with its payload sources. If + ``pinned_by`` is not empty, publish the release only if the build writes each pinned file + again with the same path, size and checksum. Args: release: The exact dataset ID and version to build. store: The local registry to build into. force: Build the release again, even if ``store`` already holds it. pinned_by: The releases that already read files of this release, with their pins. - options: The options of the build that do not change its files. The backend sends the - progress events of the build to ``options.progress``. If ``options.quiet`` is set, - the backend prints nothing of its own, also not in an isolated child. + options: The options of the build. Send its progress events to ``options.progress``. Raises: TimeNetDatasetNotFoundError: If this backend does not know the dataset ID of ``release``. TimeNetVersionUnavailableError: If the connector declares another version than - ``release``. It is also a ``TimeFValidationError``. - TimeFFormatError: If the build does not reproduce a pinned file. Nothing is published + ``release``. + TimeFFormatError: If the build does not write a pinned file again. Nothing is published then. TimeNetBuildError: If the build runs but fails. """ @@ -188,8 +178,7 @@ def find_builder(dataset_id: str) -> BuilderBackend | None: The backend, or ``None`` when no installed package claims the id. Raises: - TimeNetInvalidCardError: If a backend cannot list its dataset IDs, because the card of a - connector cannot be read. + TimeNetInvalidCardError: If the card of a connector cannot be read. """ # ruff: ignore[docstring-extraneous-exception] - raised by the backend for entry in entry_points(group=ENTRY_POINT_GROUP): backend = entry.load()() diff --git a/packages/timenet/src/timenet/coordinator/__init__.py b/packages/timenet/src/timenet/coordinator/__init__.py index e71c81d3c..5f62b44bd 100644 --- a/packages/timenet/src/timenet/coordinator/__init__.py +++ b/packages/timenet/src/timenet/coordinator/__init__.py @@ -1,6 +1,5 @@ """Plan and run builds and reads. -The functions of this package, for example ``build_release()``, are internal to TimeNet's own -packages. They make no promise of a stable interface. Users build a dataset with the -``timenet-build`` command and read one with ``TimeNet``. +This package is internal to TimeNet and has no stable interface. To build a dataset, use the +``timenet-build`` command. To read one, use ``TimeNet``. """ diff --git a/packages/timenet/src/timenet/coordinator/build.py b/packages/timenet/src/timenet/coordinator/build.py index fd4cbf5ce..bb517b493 100644 --- a/packages/timenet/src/timenet/coordinator/build.py +++ b/packages/timenet/src/timenet/coordinator/build.py @@ -1,12 +1,7 @@ -"""Build a release and each parent release that it depends on, through one coordinator. - -The coordinator does three things in order. First, it finds the installed builder backend that -claims the dataset, and it checks a requested version against the card of the connector. Then it -plans: :func:`~timenet.coordinator.plan.plan_parents` finds each parent and payload source that the -store does not hold, and the coordinator adds the build of the requested release. Then it runs each -step of the plan in order. A copy goes through :func:`~timenet.registry.copy_release`. A build -goes through ``build_prepared()`` of the backend that builds exactly that release. So every backend -gets the same call, and a backend builds only one release, with its parents already in the store. +"""Build a release after each parent release that it needs. + +A copy step uses :func:`~timenet.registry.copy_release`. A build step uses ``build_prepared()`` of +the backend that builds exactly that release. """ from collections.abc import Mapping @@ -39,57 +34,42 @@ def build_release( # ruff: ignore[too-many-arguments] ) -> Plan: """Build a release into the local store, after each release that its build needs. - The connector that claims the dataset builds only the version that its card declares. If - ``dataset`` names another version, this function raises an error before anything runs. Then it - plans. Each parent of the release, and each payload source of such a parent, that ``store`` does - not hold is copied from ``fetch_from``, or else built by the backend that builds that exact - release if ``build_parents`` is set. A missing parent that the plan cannot get fails before any - step runs. If ``store`` already holds the requested release and ``force`` is not set, the plan - gets only its missing payload sources, and nothing builds the release. - - Then each step runs in order. A copy step copies the release and its payload sources with - :func:`~timenet.registry.copy_release`, which sends the progress events of each release - that it copies. If ``store`` holds an intact copy of the release, the step copies only each - payload source that ``store`` lacks or holds damaged, and then makes sure that the release - matches its pins. A build step sends :class:`~timenet.progress.StepStarted`, calls - ``build_prepared()`` of the backend, makes sure that ``store`` holds the release with its payload - sources, and then sends :class:`~timenet.progress.StepFinished`. Only the build of the requested - release gets ``force``. + The function copies each missing parent, and each missing payload source of a parent, from + ``fetch_from``, or builds it if ``build_parents`` is set. If ``store`` already holds the + requested release, the function builds it again only if ``force`` is set. Args: dataset: The dataset ID, as ``org/name``, or one exact release, as ``org/name@1.2.0``. store: The local registry to build into. - fetch_from: The registry to copy missing parents from. If it is ``None``, nothing is copied. + fetch_from: The registry to copy missing parents from, or ``None`` to copy nothing. build_parents: Build missing parents with the backends that build them. force: Build the requested release again, even if ``store`` holds it, and plan its parents. pinned_by: The releases that already read files of the requested release, with their pins. - The build of the requested release publishes it only if it reproduces each pinned file. + The build publishes the release only if it writes each pinned file again. open_access_only: Refuse each release of the plan that does not declare ``access: open``, - the requested release included, before any step runs. A caller sets it when it copies - the result to a registry that holds only open releases. + the requested release included, before any step runs. options: The options of each build, such as the progress callback. Returns: - The plan that ran: its steps in the order in which they ran. + The plan that ran, with its steps in the order in which they ran. Raises: - TimeNetDatasetNotFoundError: If no installed backend builds the dataset, or a missing parent - has no installed backend. + TimeNetDatasetNotFoundError: If no installed backend builds the dataset or a missing parent. TimeNetVersionUnavailableError: If ``dataset`` names another version than the card of the connector declares. Nothing runs then. - TimeNetVersionConflictError: If a copy step finds that ``store`` holds a release that it - copies, or a payload source of it, with other files. That step copies nothing then. + TimeNetVersionConflictError: If ``store`` holds a release that a copy step copies, or a + payload source of it, with other files. That step copies nothing then. TimeFValidationError: If ``dataset`` is not a valid dataset reference, if the parents form a cycle, or if a backend declares another version than a parent pins. TimeNetMissingParentError: If a parent or a payload source is missing and ``build_parents`` is not set. TimeNetAccessError: If ``open_access_only`` is set and a release of the plan does not - declare ``access: open``. Also if ``fetch_from`` is not local and the plan copies such a + declare ``access: open``, or if ``fetch_from`` is not local and the plan copies such a release from it. TimeNetInvalidCardError: If the card of an installed connector cannot be read. TimeNetFileError: If ``store`` holds a damaged copy of a parent that the plan cannot copy again. - TimeFFormatError: If a build does not reproduce the files that other releases pin. + TimeFFormatError: If a build does not write again the files that other releases pin. TimeNetBuildError: If a build runs but fails. """ # ruff: ignore[docstring-extraneous-exception] - raised by the plan, the copy and the backends options = options or BuildOptions() @@ -129,9 +109,8 @@ def _copy( ) -> None: """Run one copy step. - The plan also copies a release that ``store`` holds intact, if it must get a payload source of - that release. Then ``fetch_from`` can lack the release itself. So for such a release, this - function copies only each payload source that ``store`` lacks or holds damaged. + If ``store`` holds an intact copy of the release, the function copies only each payload source + that ``store`` lacks or holds damaged. Args: release: The release of the copy step. diff --git a/packages/timenet/src/timenet/coordinator/plan.py b/packages/timenet/src/timenet/coordinator/plan.py index 81c916b60..d4e3d575d 100644 --- a/packages/timenet/src/timenet/coordinator/plan.py +++ b/packages/timenet/src/timenet/coordinator/plan.py @@ -215,9 +215,8 @@ def plan_parents( # ruff: ignore[too-many-arguments] build_parents: Build missing parents with their installed connectors. force_build: Build the requested release again, even if the output holds it, and plan its parents. - open_access_only: Check the access of each release as for an output that is not local, even - if the output is local. A caller sets it when it copies the result to a registry that - holds only open releases. + open_access_only: Require ``access: open`` of each release, as for an output that is not + local. Returns: The steps to copy or build parents, with dependencies first. @@ -230,11 +229,10 @@ def plan_parents( # ruff: ignore[too-many-arguments] TimeNetDatasetNotFoundError: If a parent is missing and no installed connector builds it. TimeNetFileError: If a local output holds a damaged copy of a parent, and the plan cannot copy the parent again. - TimeNetAccessError: If the output is not local, or ``open_access_only`` is set, and the - requested card, or a card in the lineage of the requested release that the output - holds, does not declare ``access: open``, or the plan visits a release whose card, or a - card in its lineage, does not. Also if ``fetch_from`` is not local and the plan would - copy such a release from it. + TimeNetAccessError: If the output is not local or ``open_access_only`` is set, and the + requested release or a release that the plan visits does not declare ``access: open`` + in its card or lineage. Also if ``fetch_from`` is not local and the plan would copy + such a release from it. """ # ruff: ignore[docstring-extraneous-exception] - raised by the helpers of the walk if fetch_from is not None and not isinstance(output, LocalRegistry): # A registry that is not local accepts a child only if it already holds the parents. A copy diff --git a/packages/timenet/src/timenet/errors.py b/packages/timenet/src/timenet/errors.py index f1ecb7994..4c4a125c9 100644 --- a/packages/timenet/src/timenet/errors.py +++ b/packages/timenet/src/timenet/errors.py @@ -165,9 +165,7 @@ class TimeFNullValuesError(TimeFValidationError): class TimeNetVersionUnavailableError(TimeNetDatasetNotFoundError, TimeFValidationError): """The installed connector of a dataset builds another version than the requested one. - A connector builds only the version that its card declares. So the requested version does not - exist for the caller that wants to read it, and it is invalid input for the caller that wants to - build it. + A connector builds only the version that its card declares. Attributes: release: The requested release. From 6387605feb4136816401a99ead1ec04ebef0bf03 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 22:43:04 +0100 Subject: [PATCH 7/9] refactor(build): claim a dataset through one lookup, parse a release once find_builder(), find_card() and exact_builder() now share one private lookup. A registered backend without dataset_ids() fails with a TimeFValidationError that names its entry point, not an AttributeError. build_release() parses the requested reference once. Before any step runs, it makes sure that a plan with a copy step has a registry to copy from. Its docstring names each error in one line. known_ids() in timenet_connectors.discovery is public again. The build command calls it, and ConnectorBuilder.dataset_ids() wraps it. A card without a dataset_id now fails the list with TimeNetInvalidCardError, as a card that is not a mapping does. The ruff rule against an import of timenet_connectors replaces the subprocess test of the build coordinator. Tests that only checked a dataclass default, a step report that another test covers, or that a lookup ignores knows() go away. The check that the version error is a not-found error and a validation error moves into the parametrized error test. Two client tests now install their builder through the entry points, because find_card() no longer goes through find_builder(). Part of TNET-157. --- .../src/timenet_connectors/builder/backend.py | 8 +-- .../src/timenet_connectors/builder/cli.py | 8 ++- .../src/timenet_connectors/discovery.py | 17 +++---- .../tests/test_build_backend.py | 5 +- .../tests/test_build_plan.py | 8 +-- packages/timenet/src/timenet/builders.py | 50 +++++++++++++++---- .../timenet/src/timenet/coordinator/build.py | 34 +++++-------- .../timenet/tests/coordinator/test_build.py | 13 ----- packages/timenet/tests/test_builders.py | 26 +++++----- packages/timenet/tests/test_client.py | 11 ++-- packages/timenet/tests/test_errors.py | 26 ++++++---- 11 files changed, 111 insertions(+), 95 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py index f01a81ca1..ad2d06d88 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py @@ -8,7 +8,7 @@ from timenet.types import DatasetMetadata from timenet.types.references import DatasetRef from timenet_connectors.builder.cli import build_one -from timenet_connectors.discovery import _known_ids, connector_dir, has_connector +from timenet_connectors.discovery import connector_dir, has_connector, known_ids class ConnectorBuilder: @@ -21,9 +21,9 @@ def dataset_ids(self) -> list[str]: # ruff: ignore[no-self-use] (BuilderBackend The dataset IDs, sorted. Raises: - TimeNetInvalidCardError: If a card cannot be read or is not a mapping. - """ # ruff: ignore[docstring-extraneous-exception] - raised by _known_ids - return _known_ids() + TimeNetInvalidCardError: If a card cannot be read or declares no ``dataset_id``. + """ # ruff: ignore[docstring-extraneous-exception] - raised by known_ids + return known_ids() def knows(self, dataset_id: str) -> bool: # ruff: ignore[no-self-use] (BuilderBackend protocol method) """Report whether a connector package exists for the id, without importing it. diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index afed40f7d..fcccfd244 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -49,7 +49,7 @@ from timenet.types.references import DatasetRef from timenet.writer.progress import ProgressStage, WriteProgressEvent from timenet_connectors.builder.env import run_isolated -from timenet_connectors.discovery import resolve +from timenet_connectors.discovery import known_ids, resolve from timenet_connectors.download import DownloadProgress, ProgressCallback, progress_sink @@ -147,10 +147,8 @@ def build( TimeNetAccessError: If the output is remote and gets a release without access declared ``open``. """ - from timenet_connectors.builder.backend import ConnectorBuilder # ruff: ignore[import-outside-top-level] - - # dataset_ids() imports no connector. So an unknown id fails before any environment exists. - known = list(ConnectorBuilder().dataset_ids()) + # known_ids() imports no connector. So an unknown id fails before any environment exists. + known = known_ids() if dataset_id not in known: raise typer.BadParameter(f"no connector for {dataset_id!r}; known: {', '.join(known) or '(none)'}") forwarded = out if out is not None else settings().registry diff --git a/packages/timenet-connectors/src/timenet_connectors/discovery.py b/packages/timenet-connectors/src/timenet_connectors/discovery.py index 6fa5c4049..02dbf161a 100644 --- a/packages/timenet-connectors/src/timenet_connectors/discovery.py +++ b/packages/timenet-connectors/src/timenet_connectors/discovery.py @@ -48,7 +48,7 @@ def connector_dir(dataset_id: str) -> Path: path = Path(next(iter(spec.submodule_search_locations))) if (path / "dataset.yaml").is_file(): return path - raise LookupError(f"no connector for {dataset_id!r}; known: {', '.join(_known_ids()) or '(none)'}") + raise LookupError(f"no connector for {dataset_id!r}; known: {', '.join(known_ids()) or '(none)'}") def has_connector(dataset_id: str) -> bool: @@ -86,16 +86,14 @@ def requirements_for(dataset_id: str) -> Path | None: return path if path.is_file() else None -def _known_ids() -> list[str]: - """List the dataset ids declared by the connector cards on disk, importing nothing. - - A dataset id matches exactly as its card declares it. +def known_ids() -> list[str]: + """List the dataset IDs that the connector cards declare, without importing a connector. Returns: - The declared dataset ids, sorted. + The declared dataset IDs, sorted. Raises: - TimeNetInvalidCardError: If a card cannot be read or is not a mapping. + TimeNetInvalidCardError: If a card cannot be read or declares no ``dataset_id``. """ ids: set[str] = set() for root in _datasets.__path__: @@ -109,8 +107,9 @@ def _known_ids() -> list[str]: if not isinstance(raw, dict): raise TimeNetInvalidCardError(f"dataset card {card} must be a YAML mapping, got {type(raw).__name__}") declared = raw.get("dataset_id") - if declared: - ids.add(declared) + if not declared: + raise TimeNetInvalidCardError(f"dataset card {card} must declare a dataset_id") + ids.add(declared) return sorted(ids) diff --git a/packages/timenet-connectors/tests/test_build_backend.py b/packages/timenet-connectors/tests/test_build_backend.py index db99cfd08..d7d08ec3a 100644 --- a/packages/timenet-connectors/tests/test_build_backend.py +++ b/packages/timenet-connectors/tests/test_build_backend.py @@ -15,7 +15,7 @@ from timenet_connectors import datasets as datasets_module from timenet_connectors.builder import backend as backend_module, cli as cli_module from timenet_connectors.builder.backend import ConnectorBuilder -from timenet_connectors.discovery import _known_ids +from timenet_connectors.discovery import known_ids def _unexpected_isolation(*args, **kwargs): @@ -63,6 +63,7 @@ def test_a_card_that_cannot_be_read_raises_and_only_a_missing_connector_gives_no [ ("dataset_id: [unclosed\n", "could not read dataset card {}"), ("- a list\n", "dataset card {} must be a YAML mapping"), + ("title: no id\n", "dataset card {} must declare a dataset_id"), ], ) def test_a_card_that_cannot_be_read_fails_each_lookup_with_the_card_error(tmp_path, monkeypatch, text, message): @@ -73,7 +74,7 @@ def test_a_card_that_cannot_be_read_fails_each_lookup_with_the_card_error(tmp_pa expected = re.escape(message.format(tmp_path / "test" / "broken" / "dataset.yaml")) with pytest.raises(TimeNetInvalidCardError, match=expected): - _known_ids() + known_ids() # The installed entry point loads the builder backend of this package, which lists its dataset IDs. with pytest.raises(TimeNetInvalidCardError, match=expected): find_card("test/good") diff --git a/packages/timenet-connectors/tests/test_build_plan.py b/packages/timenet-connectors/tests/test_build_plan.py index ffee95f68..dbcc89edf 100644 --- a/packages/timenet-connectors/tests/test_build_plan.py +++ b/packages/timenet-connectors/tests/test_build_plan.py @@ -249,7 +249,7 @@ def test_the_plan_reads_each_card_from_the_backend_that_claims_the_dataset(tmp_p def test_a_parent_whose_card_cannot_be_read_fails_the_plan_with_the_card_error(tmp_path, monkeypatch): (tmp_path / "dataset.yaml").write_text("dataset_id: [unclosed\n") # The in-repo backend claims PARENT, whose card cannot be read. The second backend serves CHILD. - monkeypatch.setattr(backend_module, "_known_ids", lambda: [PARENT.dataset_id]) + monkeypatch.setattr(backend_module, "known_ids", lambda: [PARENT.dataset_id]) monkeypatch.setattr(backend_module, "connector_dir", lambda dataset_id: tmp_path) install_backends(monkeypatch, ConnectorBuilder(), _CardBackend(_cards(child={"parent": PARENT}))) @@ -465,7 +465,7 @@ def run_isolated(dataset_id, root, *, force=False, **_): def test_build_prepares_parents_before_the_isolated_child(tmp_path, monkeypatch): cards = _install_cards(monkeypatch, child={"parent": PARENT, "grand": GRAND}, parent={}) - monkeypatch.setattr(backend_module, "_known_ids", lambda: list(cards)) + monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) monkeypatch.delenv("TIMENET_ISOLATION", raising=False) origin = LocalRegistry(tmp_path / "origin") origin.put_dataset(make_dataset()) @@ -623,7 +623,7 @@ def child(command, env): monkeypatch.delenv("TIMENET_ISOLATION", raising=False) monkeypatch.setattr(env_module, "requirements_for", lambda dataset_id: None) monkeypatch.setattr(env_module, "_run_build", child) - monkeypatch.setattr(backend_module, "_known_ids", lambda: list(cards)) + monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) monkeypatch.setattr(cli_module, "resolve", lambda dataset_id: _Grand) monkeypatch.setattr(cli_module, "run_pipeline", run_pipeline) @@ -754,7 +754,7 @@ def test_a_proxy_backend_builds_a_parent_through_the_build_of_its_instance(tmp_p @pytest.mark.parametrize("case", ["missing", "damaged", "not-open"]) def test_the_cli_hint_names_its_flags(tmp_path, monkeypatch, case): cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) - monkeypatch.setattr(backend_module, "_known_ids", lambda: list(cards)) + monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) out = LocalRegistry(tmp_path / "out") if case == "damaged": out.put_dataset(TimeFDataset(metadata=cards[PARENT.dataset_id])) diff --git a/packages/timenet/src/timenet/builders.py b/packages/timenet/src/timenet/builders.py index 369cc01fe..2cb174b3d 100644 --- a/packages/timenet/src/timenet/builders.py +++ b/packages/timenet/src/timenet/builders.py @@ -8,10 +8,11 @@ from collections.abc import Iterable, Mapping from dataclasses import dataclass -from importlib.metadata import entry_points +from importlib.metadata import EntryPoint, entry_points from pathlib import Path from typing import TYPE_CHECKING, Protocol +from timenet.errors import TimeFValidationError from timenet.progress import ProgressCallback from timenet.types import DatasetMetadata from timenet.types.references import DatasetRef @@ -178,12 +179,35 @@ def find_builder(dataset_id: str) -> BuilderBackend | None: The backend, or ``None`` when no installed package claims the id. Raises: + TimeFValidationError: If a registered backend does not implement ``dataset_ids()``. TimeNetInvalidCardError: If the card of a connector cannot be read. - """ # ruff: ignore[docstring-extraneous-exception] - raised by the backend + """ # ruff: ignore[docstring-extraneous-exception] - raised by _claim and the backend + claim = _claim(dataset_id) + return None if claim is None else claim[1] + + +def _claim(dataset_id: str) -> tuple[EntryPoint, BuilderBackend] | None: + """Return the first registered entry point whose backend claims a dataset ID, with that backend. + + Args: + dataset_id: The dataset ID. + + Returns: + The entry point and its backend, or ``None`` if no backend claims the dataset. + + Raises: + TimeFValidationError: If a registered backend does not implement ``dataset_ids()``. + TimeNetInvalidCardError: If the card of a connector cannot be read. + """ # ruff: ignore[docstring-extraneous-exception] - the card error is raised by the backend for entry in entry_points(group=ENTRY_POINT_GROUP): backend = entry.load()() - if dataset_id in backend.dataset_ids(): - return backend + dataset_ids = getattr(backend, "dataset_ids", None) + if not callable(dataset_ids): + raise TimeFValidationError( + f"the builder backend of entry point {entry.name!r} ({entry.value}) must implement dataset_ids()" + ) + if dataset_id in dataset_ids(): + return entry, backend return None @@ -197,10 +221,11 @@ def find_card(dataset_id: str) -> DatasetMetadata | None: The card, or ``None`` when no installed package claims the dataset. Raises: + TimeFValidationError: If a registered backend does not implement ``dataset_ids()``. TimeNetInvalidCardError: If the card of the connector cannot be read. - """ # ruff: ignore[docstring-extraneous-exception] - raised by the backend - builder = find_builder(dataset_id) - return None if builder is None else builder.card(dataset_id) + """ # ruff: ignore[docstring-extraneous-exception] - raised by _claim and the backend + claim = _claim(dataset_id) + return None if claim is None else claim[1].card(dataset_id) def exact_builder(reference: DatasetRef) -> BuilderBackend | None: @@ -217,8 +242,11 @@ def exact_builder(reference: DatasetRef) -> BuilderBackend | None: another version. Raises: + TimeFValidationError: If a registered backend does not implement ``dataset_ids()``. TimeNetInvalidCardError: If the card of the connector cannot be read. - """ # ruff: ignore[docstring-extraneous-exception] - raised by the backend - builder = find_builder(reference.dataset_id) - card = None if builder is None else builder.card(reference.dataset_id) - return builder if card is not None and card.dataset_version == reference.version else None + """ # ruff: ignore[docstring-extraneous-exception] - raised by _claim and the backend + claim = _claim(reference.dataset_id) + if claim is None: + return None + card = claim[1].card(reference.dataset_id) + return claim[1] if card is not None and card.dataset_version == reference.version else None diff --git a/packages/timenet/src/timenet/coordinator/build.py b/packages/timenet/src/timenet/coordinator/build.py index bb517b493..b7a8a9a99 100644 --- a/packages/timenet/src/timenet/coordinator/build.py +++ b/packages/timenet/src/timenet/coordinator/build.py @@ -5,11 +5,10 @@ """ from collections.abc import Mapping -from typing import cast from timenet.builders import BuildOptions, exact_builder, find_card from timenet.coordinator.plan import Plan, Step, plan_parents -from timenet.errors import TimeNetDatasetNotFoundError, TimeNetVersionUnavailableError +from timenet.errors import TimeFValidationError, TimeNetDatasetNotFoundError, TimeNetVersionUnavailableError from timenet.manifest import Manifest, PayloadSource from timenet.progress import ProgressCallback, StepFinished, StepStarted from timenet.registry import copy_release @@ -55,20 +54,13 @@ def build_release( # ruff: ignore[too-many-arguments] Raises: TimeNetDatasetNotFoundError: If no installed backend builds the dataset or a missing parent. - TimeNetVersionUnavailableError: If ``dataset`` names another version than the card of the - connector declares. Nothing runs then. - TimeNetVersionConflictError: If ``store`` holds a release that a copy step copies, or a - payload source of it, with other files. That step copies nothing then. - TimeFValidationError: If ``dataset`` is not a valid dataset reference, if the parents form a - cycle, or if a backend declares another version than a parent pins. - TimeNetMissingParentError: If a parent or a payload source is missing and ``build_parents`` - is not set. - TimeNetAccessError: If ``open_access_only`` is set and a release of the plan does not - declare ``access: open``, or if ``fetch_from`` is not local and the plan copies such a - release from it. + TimeNetVersionUnavailableError: If ``dataset`` names another version than the card declares. + TimeNetVersionConflictError: If ``store`` holds a copied release with other files. + TimeFValidationError: If ``dataset`` is not a valid reference, or the parents form a cycle. + TimeNetMissingParentError: If a parent is missing and ``build_parents`` is not set. + TimeNetAccessError: If a release of the plan is not open where open access is required. TimeNetInvalidCardError: If the card of an installed connector cannot be read. - TimeNetFileError: If ``store`` holds a damaged copy of a parent that the plan cannot copy - again. + TimeNetFileError: If ``store`` holds a damaged parent that the plan cannot copy again. TimeFFormatError: If a build does not write again the files that other releases pin. TimeNetBuildError: If a build runs but fails. """ # ruff: ignore[docstring-extraneous-exception] - raised by the plan, the copy and the backends @@ -78,11 +70,10 @@ def build_release( # ruff: ignore[too-many-arguments] if card is None: raise TimeNetDatasetNotFoundError(f"no installed connector builds {dataset_id!r}") if version is not None: - wanted = DatasetRef.parse(dataset) + wanted = DatasetRef(dataset_id=dataset_id, version=version) if wanted.version != card.dataset_version: raise TimeNetVersionUnavailableError(wanted, card.dataset_version) requested = DatasetRef(dataset_id=card.dataset_id, version=card.dataset_version) - # Passes fetch_from and build_parents to the planner unchanged: they choose how it gets a missing parent. steps = plan_parents( dataset_id, store, @@ -95,12 +86,13 @@ def build_release( # ruff: ignore[too-many-arguments] planned = {step.release for step in steps} depends_on = tuple(parent for parent in card.parents.values() if parent in planned) steps.append(Step(requested, "build", depends_on=depends_on, pinned_by=pinned_by or {})) + if fetch_from is None and any(step.action == "copy" for step in steps): + raise TimeFValidationError(f"the plan of {requested} copies a release, but has no registry to copy from") for step in steps: - if step.action == "copy": - # The plan copies only from fetch_from. - _copy(step.release, cast("BaseRegistry", fetch_from), store, progress=options.progress) - else: + if step.action == "build": _build(step, store, force=force and step.release == requested, options=options) + elif fetch_from is not None: + _copy(step.release, fetch_from, store, progress=options.progress) return Plan(tuple(steps)) diff --git a/packages/timenet/tests/coordinator/test_build.py b/packages/timenet/tests/coordinator/test_build.py index 9d0a67157..69b4fde41 100644 --- a/packages/timenet/tests/coordinator/test_build.py +++ b/packages/timenet/tests/coordinator/test_build.py @@ -1,6 +1,4 @@ import shutil -import subprocess -import sys import pytest @@ -161,12 +159,6 @@ def test_build_release_accepts_the_version_that_the_connector_declares(store, fa assert fake.calls == [(REF_AB, False, {})] -def test_build_release_reports_each_step(store, fake): - events = [] - build_release("a/b", store=store, options=BuildOptions(progress=events.append)) - assert events == [StepStarted(REF_AB, "build"), StepFinished(REF_AB, "build")] - - def test_build_release_copies_a_missing_parent_and_reports_each_step_once(tmp_path, store, fake_parent, fake_child): origin = LocalRegistry(tmp_path / "origin") origin.put_dataset(make_dataset()) @@ -329,8 +321,3 @@ def test_build_release_with_open_access_only_refuses_a_held_release_with_a_crede def test_build_release_without_a_backend_raises_not_found(store, backends): with pytest.raises(TimeNetDatasetNotFoundError, match="no installed connector builds 'a/b'"): build_release("a/b", store=store) - - -def test_the_build_coordinator_imports_nothing_from_timenet_connectors(): - code = "import sys, timenet.coordinator.build; assert 'timenet_connectors' not in sys.modules" - subprocess.run([sys.executable, "-c", code], check=True) diff --git a/packages/timenet/tests/test_builders.py b/packages/timenet/tests/test_builders.py index 4c99d7e75..2b5a0843d 100644 --- a/packages/timenet/tests/test_builders.py +++ b/packages/timenet/tests/test_builders.py @@ -1,5 +1,10 @@ +from types import SimpleNamespace + +import pytest + from timenet import builders -from timenet.builders import BuildOptions, exact_builder, find_builder, find_card +from timenet.builders import exact_builder, find_builder, find_card +from timenet.errors import TimeFValidationError from timenet.testing import install_backends, make_dataset from timenet.types.references import DatasetRef @@ -30,15 +35,16 @@ def test_find_builder_returns_the_first_backend_whose_dataset_ids_hold_the_id(mo assert find_builder("x/y") is wanted -def test_find_builder_asks_dataset_ids(monkeypatch): - class _KnowsEverything(_Backend): +def test_find_builder_names_the_entry_point_of_a_backend_without_dataset_ids(monkeypatch): + class _NoIds: def knows(self, dataset_id): return True - # The first backend says yes to every ID through knows(), but its dataset IDs do not hold this one. - wanted = _Backend("x/y") - install_backends(monkeypatch, _KnowsEverything("a/b"), wanted) - assert find_builder("x/y") is wanted + # install_backends() gives entry points without a name, so this test makes its own. + entry = SimpleNamespace(name="old", value="old_package:Builder", load=lambda: _NoIds) + monkeypatch.setattr(builders, "entry_points", lambda group: [entry]) + with pytest.raises(TimeFValidationError, match=r"'old' \(old_package:Builder\) must implement dataset_ids\(\)"): + find_builder("x/y") def test_find_builder_returns_none_with_no_registered_backends(monkeypatch): @@ -61,9 +67,3 @@ def test_exact_builder_returns_the_backend_only_for_the_version_of_its_card(monk assert exact_builder(DatasetRef(dataset_id=card.dataset_id, version=card.dataset_version)) is backend assert exact_builder(DatasetRef.parse(f"{card.dataset_id}@2.0.0")) is None assert exact_builder(DatasetRef.parse("x/y@1.0.0")) is None - - -def test_build_options_let_the_backend_print_by_default(): - # The CLI sets quiet for --quiet. The SDK leaves it unset, so an isolated child prints its lines. - assert BuildOptions().quiet is False - assert BuildOptions(quiet=True).quiet is True diff --git a/packages/timenet/tests/test_client.py b/packages/timenet/tests/test_client.py index 0ad9fc02f..1ddcf3a21 100644 --- a/packages/timenet/tests/test_client.py +++ b/packages/timenet/tests/test_client.py @@ -6,7 +6,7 @@ from pydantic import ValidationError import pytest -from timenet import builders as builders_module, client as client_module +from timenet import client as client_module from timenet.client import TimeNet from timenet.connectors import BaseConnector from timenet.dataset import Record, RegularAxis, Signal, Source, TimeFDataset @@ -24,7 +24,7 @@ from timenet.manifest.files import FilePart from timenet.reader import Loading, LoadPlan, TimeFReader from timenet.registry import TIMENET_REGISTRY_URL, LocalRegistry, RemoteRegistry, open_local_version -from timenet.testing import assert_datasets_equal, make_dataset +from timenet.testing import assert_datasets_equal, install_backends, make_dataset from timenet.types import Access, DatasetMetadata, Domain, License, Version from timenet.types.access import AccessRequirement from timenet.types.parent_dataset_card import ParentDatasetCard @@ -67,6 +67,9 @@ class _Builder: def __init__(self, calls): self.calls = calls + def dataset_ids(self): + return ("test/child", "timenet/hello-world") + def knows(self, dataset_id): return True @@ -187,7 +190,7 @@ def test_a_missing_payload_source_builds_with_its_own_connector_against_the_pins calls = [] monkeypatch.setattr(_Builder, "VERSION", "2.0.0" if case == "other-version" else "1.0.0") monkeypatch.setattr(_Builder, "BACKEND", "zarr" if case == "differs" else "parquet") - monkeypatch.setattr(builders_module, "find_builder", lambda dataset_id: _Builder(calls)) + install_backends(monkeypatch, _Builder(calls)) with pytest.raises(TimeNetMissingParentError, match=r"auto_build=True\.$"): TimeNet(registry).open("test/child", auto_build=False) @@ -226,7 +229,7 @@ def test_a_damaged_release_is_repaired_from_fetch_from_and_never_built_over(tmp_ size = control.stat().st_size control.write_bytes(b"cut short") calls = [] - monkeypatch.setattr(builders_module, "find_builder", lambda dataset_id: _Builder(calls)) + install_backends(monkeypatch, _Builder(calls)) monkeypatch.setattr(client_module, "find_builder", lambda dataset_id: _Builder(calls)) with pytest.raises(TimeNetFileError) as caught: diff --git a/packages/timenet/tests/test_errors.py b/packages/timenet/tests/test_errors.py index e5b2fbcff..4e3d72114 100644 --- a/packages/timenet/tests/test_errors.py +++ b/packages/timenet/tests/test_errors.py @@ -65,55 +65,64 @@ def test_catchable_as_base(): @pytest.mark.parametrize( - ("error", "facts", "message"), + ("error", "facts", "message", "caught_as"), [ ( TimeNetMissingParentError(_A, _B, "here", payload_source=True), {"missing": _A, "needed_by": _B, "payload_source": True}, "payload source org/a@1.0.0 of org/b@2.0.0 is not in here.", + (), ), ( TimeNetMissingParentError(_A, _B, "here", payload_source=False), {"payload_source": False}, "parent release org/a@1.0.0 of org/b@2.0.0 is not in here.", + (), ), ( TimeNetFileError(_A, "org/a@1.0.0", "control.duckdb", "1,024 bytes", None), {"release": _A, "path": "control.duckdb", "expected": "1,024 bytes", "actual": None}, "org/a@1.0.0: file 'control.duckdb' is missing, expected 1,024 bytes.", + (), ), ( TimeNetFileError(_A, "org/a@1.0.0", "control.duckdb", "1,024 bytes", "9 bytes"), {"expected": "1,024 bytes", "actual": "9 bytes"}, "org/a@1.0.0: file 'control.duckdb' has 9 bytes, expected 1,024 bytes.", + (), ), ( TimeNetFileError(_A, "download of org/a@1.0.0", "values.parquet", "sha256:aa", "sha256:bb"), {"owner": "download of org/a@1.0.0", "expected": "sha256:aa", "actual": "sha256:bb"}, "download of org/a@1.0.0: file 'values.parquet' has sha256:bb, expected sha256:aa.", + (), ), ( TimeNetAccessError(_A, Access.CREDENTIALED, (_PAGE_A,)), {"dataset": _A, "access": Access.CREDENTIALED, "requirements": (_PAGE_A,)}, "org/a@1.0.0 is credentialed. Get access at https://a.example for org/a@1.0.0.", + (), ), ( TimeNetAccessError(_C, Access.RESTRICTED, (_PAGE_A, _PAGE_B)), {"dataset": _C, "access": Access.RESTRICTED, "requirements": (_PAGE_A, _PAGE_B)}, "org/c@3.0.0 is restricted. " "Get access at https://a.example for org/a@1.0.0 and at https://b.example for org/b@2.0.0.", + (), ), ( TimeNetAccessError(_C, Access.RESTRICTED, (_PAGE_C, _PAGE_A, _PAGE_B)), {"requirements": (_PAGE_C, _PAGE_A, _PAGE_B)}, "org/c@3.0.0 is restricted. Get access at https://c.example for org/c@3.0.0, " "at https://a.example for org/a@1.0.0 and at https://b.example for org/b@2.0.0.", + (), ), ( # A card that a test or a tool builds in code can lack the page. TimeNetAccessError(_A, Access.CREDENTIALED, ()), {"requirements": ()}, "org/a@1.0.0 is credentialed.", + (), ), ( TimeNetVersionConflictError(_A, "registry here", "registry there"), @@ -126,33 +135,32 @@ def test_catchable_as_base(): }, "registry here already holds org/a@1.0.0 with other files than registry there. " "A published version cannot change, so a changed release needs a new version.", + (), ), ( TimeNetVersionConflictError(_A, "registry here", "registry there", _B, "card"), {"release": _A, "needed_by": _B, "difference": "card"}, "registry here already holds org/a@1.0.0, a payload source of org/b@2.0.0, with another card than " "registry there. A published version cannot change, so a changed release needs a new version.", + (), ), ( TimeNetVersionUnavailableError(_B, Version(1, 0, 0)), {"release": _B, "declared": Version(1, 0, 0)}, "connector for 'org/b' builds version 1.0.0, not the requested 2.0.0. " "Ask for version 1.0.0, or leave out the version.", + # The client reports a missing version, and a build reports invalid input. + (TimeNetDatasetNotFoundError, TimeFValidationError), ), ], ) -def test_an_error_with_facts_ends_with_the_hint_of_its_front_end(error, facts, message): +def test_an_error_with_facts_ends_with_the_hint_of_its_front_end(error, facts, message, caught_as): assert str(error) == message error.hint = "Do this instead." # A worker process may pickle the error; its facts and its hint come back. restored = pickle.loads(pickle.dumps(error)) assert {name: getattr(restored, name) for name in facts} == facts + # A front end can catch the error as each of these classes. + assert all(isinstance(restored, kind) for kind in caught_as) # The message ends with a period, and the hint follows as its own sentence. assert str(restored) == f"{message} Do this instead." - - -def test_a_version_that_the_connector_does_not_build_is_not_found_and_invalid(): - error = TimeNetVersionUnavailableError(_B, Version(1, 0, 0)) - # The client reports a missing version, and a build reports invalid input. Both catch it. - assert isinstance(error, TimeNetDatasetNotFoundError) - assert isinstance(error, TimeFValidationError) From 00b8a168997a8f28844386b038fcd824e80e852c Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Thu, 5 Nov 2026 00:13:37 +0100 Subject: [PATCH 8/9] docs: shorten the docstrings of the release build Shorter docstrings for the code that this change adds or changes. Part of TNET-157. --- .../src/timenet_connectors/builder/backend.py | 8 +-- .../src/timenet_connectors/builder/cli.py | 2 +- .../src/timenet_connectors/discovery.py | 19 +++-- packages/timenet/src/timenet/builders.py | 69 +++++++++--------- packages/timenet/src/timenet/client.py | 10 +-- .../src/timenet/coordinator/__init__.py | 6 +- .../timenet/src/timenet/coordinator/build.py | 72 ++++++++----------- .../timenet/src/timenet/coordinator/plan.py | 65 ++++++++--------- packages/timenet/src/timenet/errors.py | 6 +- 9 files changed, 113 insertions(+), 144 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py index ad2d06d88..c37faa5cd 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py @@ -12,16 +12,16 @@ class ConnectorBuilder: - """Builds a dataset by running its connector in an environment built from its requirements.""" + """Build a dataset with its connector, in an environment built from the connector's requirements.""" def dataset_ids(self) -> list[str]: # ruff: ignore[no-self-use] (BuilderBackend protocol method) - """List the dataset IDs of the connector cards in this package, without importing a connector. + """List this package's dataset IDs without importing connectors. Returns: - The dataset IDs, sorted. + The sorted dataset IDs. Raises: - TimeNetInvalidCardError: If a card cannot be read or declares no ``dataset_id``. + TimeNetInvalidCardError: If a card is unreadable, is not a YAML mapping, or lacks ``dataset_id``. """ # ruff: ignore[docstring-extraneous-exception] - raised by known_ids return known_ids() diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index fcccfd244..d2fd18bf6 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -203,7 +203,7 @@ def build_one( # ruff: ignore[too-many-arguments] dataset_id: The dataset ID. output: A local directory or a ``timenet://``, ``http(s)://``, or ``s3://`` registry URL. fetch_from: The registry, or its URI, to copy missing parent releases from. - build_parents: Build missing parent releases from their installed connectors. + build_parents: Build missing parents with installed connectors. force: Rebuild an existing version. keep_cache: Keep the raw download cache after building. isolation: Use a separate environment if true, this interpreter if false, or ``TIMENET_ISOLATION`` if ``None``. diff --git a/packages/timenet-connectors/src/timenet_connectors/discovery.py b/packages/timenet-connectors/src/timenet_connectors/discovery.py index 02dbf161a..313333934 100644 --- a/packages/timenet-connectors/src/timenet_connectors/discovery.py +++ b/packages/timenet-connectors/src/timenet_connectors/discovery.py @@ -52,16 +52,13 @@ def connector_dir(dataset_id: str) -> Path: def has_connector(dataset_id: str) -> bool: - """Report whether a connector package exists for an id, without importing it. - - Ask this instead of catching :func:`connector_dir`'s ``LookupError``: a miss is an ordinary - answer here, not an exception. + """Check for a dataset's connector package and card without importing the connector. Args: dataset_id: The dataset ID, as ``org/name``. Returns: - Whether a connector package exists for the id. + Whether a connector package with a ``dataset.yaml`` card exists. """ try: connector_dir(dataset_id) @@ -71,29 +68,29 @@ def has_connector(dataset_id: str) -> bool: def requirements_for(dataset_id: str) -> Path | None: - """Return the connector's ``requirements.txt``, or ``None`` if it declares none. + """Find a connector's requirements file. Args: dataset_id: The dataset ID, as ``org/name``. Returns: - The path to the connector's ``requirements.txt``, or ``None``. + The path to ``requirements.txt``, or ``None`` if the file is absent. Raises: - LookupError: If no module exists for the id. + LookupError: If no connector with a ``dataset.yaml`` card exists for the ID. """ # ruff: ignore[docstring-extraneous-exception] (raised by connector_dir, not directly here) path = connector_dir(dataset_id) / "requirements.txt" return path if path.is_file() else None def known_ids() -> list[str]: - """List the dataset IDs that the connector cards declare, without importing a connector. + """List dataset IDs from connector cards without importing connectors. Returns: - The declared dataset IDs, sorted. + The sorted dataset IDs, with duplicates removed. Raises: - TimeNetInvalidCardError: If a card cannot be read or declares no ``dataset_id``. + TimeNetInvalidCardError: If a card is unreadable, is not a YAML mapping, or lacks ``dataset_id``. """ ids: set[str] = set() for root in _datasets.__path__: diff --git a/packages/timenet/src/timenet/builders.py b/packages/timenet/src/timenet/builders.py index 2cb174b3d..0dcde36b4 100644 --- a/packages/timenet/src/timenet/builders.py +++ b/packages/timenet/src/timenet/builders.py @@ -28,12 +28,12 @@ @dataclass(frozen=True) class BuildOptions: - """Options of a build that do not change the files of the release. + """Build settings that do not change the files of the release. Attributes: keep_cache: Keep the raw download cache after the build. - progress: Receives the progress events of the build, or ``None``. - quiet: Ask the backend to print nothing of its own, also from an isolated child process. + progress: The callback for build progress events, or ``None``. + quiet: Suppress output from the backend and its child processes. """ keep_cache: bool = False @@ -42,10 +42,10 @@ class BuildOptions: class BuilderBackend(Protocol): - """Something that can build a dataset release from its connector.""" + """An interface for building dataset releases from connectors.""" def dataset_ids(self) -> Iterable[str]: - """Return the IDs of the datasets that this backend can build. + """List the datasets this backend can build. Implementations must answer without importing a connector. @@ -53,7 +53,7 @@ def dataset_ids(self) -> Iterable[str]: The dataset IDs. Raises: - TimeNetInvalidCardError: If the card of a connector cannot be read. + TimeNetInvalidCardError: If a connector card cannot be read. """ ... @@ -145,59 +145,56 @@ def build_prepared( pinned_by: "Mapping[DatasetRef, PayloadSource]", options: BuildOptions, ) -> None: - """Build exactly this release into the store. + """Build the requested dataset version into a local registry. - The store already holds each parent of the release with its payload sources. If - ``pinned_by`` is not empty, publish the release only if the build writes each pinned file - again with the same path, size and checksum. + The local registry must already contain all parents and their payload sources. + Publish only if all pinned files keep the same path, size, and checksum. Args: release: The exact dataset ID and version to build. store: The local registry to build into. - force: Build the release again, even if ``store`` already holds it. - pinned_by: The releases that already read files of this release, with their pins. - options: The options of the build. Send its progress events to ``options.progress``. + force: Rebuild even if ``store`` already holds the release. + pinned_by: The releases that pin files in this release, with their file records. + options: Build settings, including ``options.progress`` to receive build events. Raises: - TimeNetDatasetNotFoundError: If this backend does not know the dataset ID of ``release``. - TimeNetVersionUnavailableError: If the connector declares another version than - ``release``. - TimeFFormatError: If the build does not write a pinned file again. Nothing is published - then. - TimeNetBuildError: If the build runs but fails. + TimeNetDatasetNotFoundError: If this backend has no connector for the dataset ID. + TimeNetVersionUnavailableError: If the connector declares a different version. + TimeFFormatError: If the build omits or changes a pinned file. + TimeNetBuildError: If the build fails. """ ... def find_builder(dataset_id: str) -> BuilderBackend | None: - """Return the first registered backend whose dataset IDs hold this dataset ID. + """Find the first registered backend that lists the dataset ID. Args: - dataset_id: The dataset id. + dataset_id: The dataset ID. Returns: - The backend, or ``None`` when no installed package claims the id. + The backend, or ``None`` if no backend lists the dataset ID. Raises: TimeFValidationError: If a registered backend does not implement ``dataset_ids()``. - TimeNetInvalidCardError: If the card of a connector cannot be read. + TimeNetInvalidCardError: If a connector card cannot be read. """ # ruff: ignore[docstring-extraneous-exception] - raised by _claim and the backend claim = _claim(dataset_id) return None if claim is None else claim[1] def _claim(dataset_id: str) -> tuple[EntryPoint, BuilderBackend] | None: - """Return the first registered entry point whose backend claims a dataset ID, with that backend. + """Find the first entry point and backend that list the dataset ID. Args: dataset_id: The dataset ID. Returns: - The entry point and its backend, or ``None`` if no backend claims the dataset. + The entry point and backend, or ``None`` if no backend lists the dataset ID. Raises: TimeFValidationError: If a registered backend does not implement ``dataset_ids()``. - TimeNetInvalidCardError: If the card of a connector cannot be read. + TimeNetInvalidCardError: If a connector card cannot be read. """ # ruff: ignore[docstring-extraneous-exception] - the card error is raised by the backend for entry in entry_points(group=ENTRY_POINT_GROUP): backend = entry.load()() @@ -212,38 +209,36 @@ def _claim(dataset_id: str) -> tuple[EntryPoint, BuilderBackend] | None: def find_card(dataset_id: str) -> DatasetMetadata | None: - """Return the card of the installed connector that claims this dataset. + """Read a dataset's card from the first registered backend that lists it. Args: - dataset_id: The dataset id. + dataset_id: The dataset ID. Returns: - The card, or ``None`` when no installed package claims the dataset. + The card, or ``None`` if no backend lists the dataset or the backend has no card. Raises: TimeFValidationError: If a registered backend does not implement ``dataset_ids()``. - TimeNetInvalidCardError: If the card of the connector cannot be read. + TimeNetInvalidCardError: If a connector card cannot be read. """ # ruff: ignore[docstring-extraneous-exception] - raised by _claim and the backend claim = _claim(dataset_id) return None if claim is None else claim[1].card(dataset_id) def exact_builder(reference: DatasetRef) -> BuilderBackend | None: - """Return the backend that can build exactly this release. + """Find a backend whose card matches the requested dataset version. - A connector builds only the dataset version that its card declares. So a pinned release can only - be built by a connector whose card declares that exact version. + Check only the first registered backend that lists the dataset ID. Args: - reference: The dataset ID and the version. + reference: The dataset ID and version to build. Returns: - The backend, or ``None`` if no installed backend claims the dataset or its card declares - another version. + The backend, or ``None`` if no backend lists the dataset or its card is absent or has another version. Raises: TimeFValidationError: If a registered backend does not implement ``dataset_ids()``. - TimeNetInvalidCardError: If the card of the connector cannot be read. + TimeNetInvalidCardError: If a connector card cannot be read. """ # ruff: ignore[docstring-extraneous-exception] - raised by _claim and the backend claim = _claim(reference.dataset_id) if claim is None: diff --git a/packages/timenet/src/timenet/client.py b/packages/timenet/src/timenet/client.py index 64d529a0e..c4b6d23ca 100644 --- a/packages/timenet/src/timenet/client.py +++ b/packages/timenet/src/timenet/client.py @@ -443,15 +443,11 @@ def _obtain( # ruff: ignore[too-many-arguments] """Find a version that the local registry lacks in ``fetch_from``, or build it. Returns: - The manifest of the version, from ``fetch_from`` or from the local registry after the - build. If the manifest comes from ``fetch_from``, the caller then copies the files of the - version. + The manifest from ``fetch_from`` or the local build. Raises: - TimeNetDatasetNotFoundError: If neither ``fetch_from`` nor a connector provides the version, - or if the connector declares another version. In the first case, the message names each - keyword that the call did not use and that can provide the version. - TimeNetInvalidCardError: If the card of the connector cannot be read. + TimeNetDatasetNotFoundError: If neither source provides the release or the connector declares another version. + TimeNetInvalidCardError: If the connector's card cannot be read. """ # ruff: ignore[docstring-extraneous-exception] - raised by declared_version if fetch_from is not None: try: diff --git a/packages/timenet/src/timenet/coordinator/__init__.py b/packages/timenet/src/timenet/coordinator/__init__.py index 5f62b44bd..ff32d1012 100644 --- a/packages/timenet/src/timenet/coordinator/__init__.py +++ b/packages/timenet/src/timenet/coordinator/__init__.py @@ -1,5 +1,5 @@ -"""Plan and run builds and reads. +"""Plan and run dataset builds and reads. -This package is internal to TimeNet and has no stable interface. To build a dataset, use the -``timenet-build`` command. To read one, use ``TimeNet``. +This internal package has no stable interface. +Use ``timenet-build`` to build datasets and ``TimeNet`` to read them. """ diff --git a/packages/timenet/src/timenet/coordinator/build.py b/packages/timenet/src/timenet/coordinator/build.py index b7a8a9a99..9b7774097 100644 --- a/packages/timenet/src/timenet/coordinator/build.py +++ b/packages/timenet/src/timenet/coordinator/build.py @@ -1,8 +1,4 @@ -"""Build a release after each parent release that it needs. - -A copy step uses :func:`~timenet.registry.copy_release`. A build step uses ``build_prepared()`` of -the backend that builds exactly that release. -""" +"""Prepare parent releases and build a dataset into a local registry.""" from collections.abc import Mapping @@ -31,38 +27,35 @@ def build_release( # ruff: ignore[too-many-arguments] open_access_only: bool = False, options: BuildOptions | None = None, ) -> Plan: - """Build a release into the local store, after each release that its build needs. + """Prepare a release's dependencies and build it into a local registry. - The function copies each missing parent, and each missing payload source of a parent, from - ``fetch_from``, or builds it if ``build_parents`` is set. If ``store`` already holds the - requested release, the function builds it again only if ``force`` is set. + Copy missing dependencies from ``fetch_from``, or build them if ``build_parents`` is set. + Reuse an existing release unless ``force`` is set. Args: - dataset: The dataset ID, as ``org/name``, or one exact release, as ``org/name@1.2.0``. + dataset: A dataset ID such as ``org/name``, or an exact release such as ``org/name@1.2.0``. store: The local registry to build into. fetch_from: The registry to copy missing parents from, or ``None`` to copy nothing. - build_parents: Build missing parents with the backends that build them. - force: Build the requested release again, even if ``store`` holds it, and plan its parents. - pinned_by: The releases that already read files of the requested release, with their pins. - The build publishes the release only if it writes each pinned file again. - open_access_only: Refuse each release of the plan that does not declare ``access: open``, - the requested release included, before any step runs. - options: The options of each build, such as the progress callback. + build_parents: Build missing parents with installed connectors. + force: Rebuild the requested release and prepare its parents even if the release exists. + pinned_by: Releases that already read the requested release's files, with the pins the new build must match. + open_access_only: Require ``access: open`` for all releases and their lineage before running the plan. + options: Build settings, or ``None`` for defaults. Returns: - The plan that ran, with its steps in the order in which they ran. + The completed plan, with steps in execution order. Raises: TimeNetDatasetNotFoundError: If no installed backend builds the dataset or a missing parent. - TimeNetVersionUnavailableError: If ``dataset`` names another version than the card declares. - TimeNetVersionConflictError: If ``store`` holds a copied release with other files. - TimeFValidationError: If ``dataset`` is not a valid reference, or the parents form a cycle. + TimeNetVersionUnavailableError: If the connector declares a different version. + TimeNetVersionConflictError: If ``store`` holds a different copy of a release being copied. + TimeFValidationError: If the dataset reference, backend, or dependency plan is invalid. TimeNetMissingParentError: If a parent is missing and ``build_parents`` is not set. - TimeNetAccessError: If a release of the plan is not open where open access is required. - TimeNetInvalidCardError: If the card of an installed connector cannot be read. - TimeNetFileError: If ``store`` holds a damaged parent that the plan cannot copy again. - TimeFFormatError: If a build does not write again the files that other releases pin. - TimeNetBuildError: If a build runs but fails. + TimeNetAccessError: If a release or its lineage lacks required open access. + TimeNetInvalidCardError: If an installed connector's card cannot be read. + TimeNetFileError: If ``store`` holds a damaged parent and no replacement is available. + TimeFFormatError: If a build omits or changes a pinned file. + TimeNetBuildError: If a build fails. """ # ruff: ignore[docstring-extraneous-exception] - raised by the plan, the copy and the backends options = options or BuildOptions() dataset_id, version = split_ref(dataset) @@ -99,16 +92,15 @@ def build_release( # ruff: ignore[too-many-arguments] def _copy( release: DatasetRef, fetch_from: BaseRegistry, store: LocalRegistry, *, progress: ProgressCallback | None ) -> None: - """Run one copy step. + """Copy a release and its required payload sources into the local registry. - If ``store`` holds an intact copy of the release, the function copies only each payload source - that ``store`` lacks or holds damaged. + If the release is intact, copy only its missing or damaged payload sources. Args: - release: The release of the copy step. + release: The release to copy. fetch_from: The registry to copy from. - store: The local registry to copy to. - progress: Receives the progress events of each release that is copied, or ``None``. + store: The destination registry. + progress: The callback for copy progress events, or ``None``. """ held = _intact_manifest(store, release) if held is None: @@ -122,15 +114,14 @@ def _copy( def _intact_manifest(store: LocalRegistry, reference: DatasetRef) -> Manifest | None: - """Return the manifest of a release that the store holds with every file at its recorded size. + """Read a release's manifest and check its file sizes. Args: store: The local registry. - reference: The release. + reference: The release to check. Returns: - The manifest, or ``None`` if ``store`` lacks the release or a file of it, or holds a file of - another size. + The manifest, or ``None`` if the release or a file is missing or a file has the wrong size. """ try: manifest = store.get_manifest(reference.dataset_id, str(reference.version)) @@ -140,17 +131,16 @@ def _intact_manifest(store: LocalRegistry, reference: DatasetRef) -> Manifest | def _build(step: Step, store: LocalRegistry, *, force: bool, options: BuildOptions) -> None: - """Run one build step with the backend that builds exactly its release. + """Build a planned release and check its required files. Args: step: The build step. store: The local registry to build into. - force: Build the release again, even if ``store`` holds it. - options: The options of the build. + force: Rebuild even if ``store`` already holds the release. + options: The build settings. Raises: - TimeNetDatasetNotFoundError: If no installed backend builds the exact release, or if the - backend does not put the release or a payload source of it into ``store``. + TimeNetDatasetNotFoundError: If no backend builds this version or the result lacks a release or payload source. """ builder = exact_builder(step.release) if builder is None: diff --git a/packages/timenet/src/timenet/coordinator/plan.py b/packages/timenet/src/timenet/coordinator/plan.py index d4e3d575d..4600b620f 100644 --- a/packages/timenet/src/timenet/coordinator/plan.py +++ b/packages/timenet/src/timenet/coordinator/plan.py @@ -49,7 +49,7 @@ class Step: @dataclass(frozen=True) class Plan: - """The steps that get each release that a build needs, in the order in which they run.""" + """The copy and build steps for a dataset, in execution order.""" steps: tuple[Step, ...] @@ -71,13 +71,13 @@ class _Release: @dataclass class _Graph: - """A graph of required releases and their dependencies.""" + """The releases a build needs and the dependencies between them.""" output: WritableRegistry fetch_from: BaseRegistry | None build_parents: bool open_only: bool - """True if each release that the walk visits must declare ``access: open``.""" + """Whether each visited release and its lineage must declare ``access: open``.""" searched: str """The registries that the plan looks in, for the message of a missing release.""" releases: dict[DatasetRef, _Release] = field(default_factory=dict) @@ -88,12 +88,14 @@ class _Graph: """For each payload source: each release that reads its files, with its pins on them.""" def visit(self, reference: DatasetRef, needed_by: DatasetRef, pinned: PayloadSource | None = None) -> None: - """Add a release and its dependencies to the graph. + """Add a release and its required dependencies to the graph. + + Use :meth:`order` to check the completed graph for cycles. Args: reference: The release to add. needed_by: The release that needs ``reference``. - pinned: The expected file records for a payload source, or ``None`` to open it as a parent. + pinned: File pins from ``needed_by``, or ``None`` to open ``reference`` as a parent. """ opened = pinned is None if pinned is not None: @@ -203,36 +205,30 @@ def plan_parents( # ruff: ignore[too-many-arguments] force_build: bool = False, open_access_only: bool = False, ) -> list[Step]: - """Plan the parent releases needed to build a dataset. + """Plan the copy and build steps for a dataset's parents. - Each parent needs its payload sources as well. The plan copies missing releases from - ``fetch_from`` or builds them if ``build_parents`` is true. + Copy missing parents and their payload sources from ``fetch_from``, or build them if enabled. + Remote registries and ``open_access_only`` require open access for releases and their lineage. Args: dataset_id: The dataset to build. - output: The destination registry. - fetch_from: The registry to copy missing parents from, or ``None`` to copy nothing. + output: The registry that the build writes to. + fetch_from: The registry to copy missing parents from for a local output, or ``None``. build_parents: Build missing parents with their installed connectors. - force_build: Build the requested release again, even if the output holds it, and plan its - parents. - open_access_only: Require ``access: open`` of each release, as for an output that is not - local. + force_build: Plan parents for a rebuild even if the requested release already exists. + open_access_only: Require ``access: open`` for each release, including the requested release. Returns: - The steps to copy or build parents, with dependencies first. + The copy and build steps, with dependencies first. Raises: LookupError: If no installed connector declares a card for ``dataset_id``. - TimeNetInvalidCardError: If reading an installed connector's card fails. - TimeFValidationError: If parents form a cycle, a connector version differs, or copying targets a remote output. - TimeNetMissingParentError: If a required release is missing and ``build_parents`` is false. + TimeNetInvalidCardError: If an installed connector's card cannot be read. + TimeFValidationError: If parents form a cycle, versions conflict, or a copy targets a remote output. + TimeNetMissingParentError: If a parent or payload source is missing and ``build_parents`` is false. TimeNetDatasetNotFoundError: If a parent is missing and no installed connector builds it. - TimeNetFileError: If a local output holds a damaged copy of a parent, and the plan cannot - copy the parent again. - TimeNetAccessError: If the output is not local or ``open_access_only`` is set, and the - requested release or a release that the plan visits does not declare ``access: open`` - in its card or lineage. Also if ``fetch_from`` is not local and the plan would copy - such a release from it. + TimeNetFileError: If a local parent is damaged and no replacement is available. + TimeNetAccessError: If a release or its lineage lacks required open access. """ # ruff: ignore[docstring-extraneous-exception] - raised by the helpers of the walk if fetch_from is not None and not isinstance(output, LocalRegistry): # A registry that is not local accepts a child only if it already holds the parents. A copy @@ -341,20 +337,17 @@ def _locate( ) -> tuple[Manifest | None, Literal["copy"] | None]: # The damage is raised from a variable, the access error by require_open_access. # ruff: ignore[docstring-extraneous-exception] - """Find a release in ``output`` or ``fetch_from``. + """Find a release in the output or the source registry. - In a local output, a copy with a missing file or a file of another size is damaged. If - ``open_only`` is set, the release must declare ``access: open``. + A local copy is intact when all files exist at their recorded sizes. + Enforce open access for the release and its lineage when ``open_only`` or a copy requires it. Returns: - ``(manifest, None)`` from ``output``, ``(manifest, "copy")`` from ``fetch_from``, or ``(None, None)`` if absent. + The manifest and action (``None`` to reuse, ``"copy"`` to fetch), or ``(None, None)`` if absent. Raises: - TimeNetFileError: If a local output holds a damaged copy, and ``fetch_from`` does not hold - the release. - TimeNetAccessError: If the card of the release, or a card in its lineage, does not declare - ``access: open``, and ``open_only`` is set or the ``fetch_from`` registry to copy it - from is not local. + TimeNetFileError: If the local copy is damaged and the source registry lacks the release. + TimeNetAccessError: If the release or its lineage lacks required open access. """ manifest = _manifest(output, reference) damage = None @@ -393,13 +386,13 @@ def _card_to_build( # ruff: ignore[too-many-arguments] pinned: bool, searched: str, ) -> DatasetMetadata: - """Find a dataset card for building a missing release. + """Find a connector card for building a missing release. Returns: - The installed connector's card for the exact release in ``reference``. + The connector card for the exact dataset ID and version in ``reference``. Raises: - TimeNetMissingParentError: If ``build_parents`` is not set. + TimeNetMissingParentError: If ``build_parents`` is false. TimeNetAccessError: If ``open_only`` is set and the card does not declare ``access: open``. """ # ruff: ignore[docstring-extraneous-exception] - the access error is raised by require_open_access if not build_parents: diff --git a/packages/timenet/src/timenet/errors.py b/packages/timenet/src/timenet/errors.py index 4c4a125c9..f675cbaa2 100644 --- a/packages/timenet/src/timenet/errors.py +++ b/packages/timenet/src/timenet/errors.py @@ -163,14 +163,12 @@ class TimeFNullValuesError(TimeFValidationError): class TimeNetVersionUnavailableError(TimeNetDatasetNotFoundError, TimeFValidationError): - """The installed connector of a dataset builds another version than the requested one. - - A connector builds only the version that its card declares. + """The installed connector cannot build the requested dataset version. Attributes: release: The requested release. declared: The version that the connector builds. - hint: What the caller can do instead, added by the front end, or ``None``. + hint: Suggested next steps to append to the error message, or ``None``. """ def __init__(self, release: "DatasetRef", declared: "Version") -> None: From 5a98da8b7444735dfffbaf7cb1085c968e58a50a Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Thu, 5 Nov 2026 14:14:08 +0100 Subject: [PATCH 9/9] fix(build)!: skip unreadable connector cards and look up a builder once One connector card that could not be read made known_ids() raise. So every lookup through the installed connectors failed, also for other datasets. Now known_ids() skips that card with a ConnectorCardWarning, and only its own dataset fails. timenet-build build refused org/name@version. It now takes the version and checks it against the card of the connector. builders.installed_builder() walks the entry points once and returns an InstalledBuilder: the backend with its card. builders.declared() also checks a requested version, and it raises the one "no installed connector builds" error. Both replace find_builder() and exact_builder(). The check for a backend without dataset_ids() goes, because the builder protocol requires the method. The coordinator docstring says that the front ends share the planner. BREAKING CHANGE: builders.find_builder() is gone. Use builders.installed_builder(). Part of TNET-157. --- .../src/timenet_connectors/builder/backend.py | 7 +- .../src/timenet_connectors/builder/cli.py | 18 ++-- .../src/timenet_connectors/discovery.py | 47 ++++++--- .../tests/test_build_backend.py | 23 ++--- .../tests/test_build_cli.py | 18 ++++ .../tests/test_build_plan.py | 3 +- packages/timenet/src/timenet/builders.py | 98 +++++++++---------- packages/timenet/src/timenet/client.py | 14 +-- .../src/timenet/coordinator/__init__.py | 3 +- .../timenet/src/timenet/coordinator/build.py | 20 ++-- packages/timenet/src/timenet/errors.py | 4 + packages/timenet/tests/test_builders.py | 66 +++++-------- packages/timenet/tests/test_client.py | 18 ++-- 13 files changed, 177 insertions(+), 162 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py index c37faa5cd..02b1251a7 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py @@ -18,11 +18,8 @@ def dataset_ids(self) -> list[str]: # ruff: ignore[no-self-use] (BuilderBackend """List this package's dataset IDs without importing connectors. Returns: - The sorted dataset IDs. - - Raises: - TimeNetInvalidCardError: If a card is unreadable, is not a YAML mapping, or lacks ``dataset_id``. - """ # ruff: ignore[docstring-extraneous-exception] - raised by known_ids + The sorted dataset IDs. A card that cannot be read is skipped with a warning. + """ return known_ids() def knows(self, dataset_id: str) -> bool: # ruff: ignore[no-self-use] (BuilderBackend protocol method) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index d2fd18bf6..f026c56f6 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -23,7 +23,7 @@ ) import typer -from timenet.builders import BuilderBackend, find_builder, find_card +from timenet.builders import BuilderBackend, declared, find_card, installed_builder from timenet.cache import human_bytes from timenet.cli.runner import run_cli from timenet.cli.ui import console @@ -46,7 +46,7 @@ open_registry, open_writable_registry, ) -from timenet.types.references import DatasetRef +from timenet.types.references import DatasetRef, split_ref from timenet.writer.progress import ProgressStage, WriteProgressEvent from timenet_connectors.builder.env import run_isolated from timenet_connectors.discovery import known_ids, resolve @@ -88,7 +88,7 @@ def _resolve_target(out: str | None) -> WritableRegistry | Path: @app.command() # ruff: ignore[too-many-arguments, too-many-positional-arguments] (a typer command: one parameter per option) def build( - dataset_id: str, + release: str = typer.Argument(..., metavar="DATASET_ID[@VERSION]"), out: str | None = typer.Option( None, "--out", @@ -141,16 +141,20 @@ def build( Raises: BadParameter: If ``dataset_id`` has no known connector. + TimeNetVersionUnavailableError: If the connector cannot build the requested version. TimeNetMissingParentError: If a parent is missing. The hint of the error names the flags. TimeNetFileError: If a copy of a parent in the output is damaged, or a fetched file does not match its manifest record. The hint of the error names the flags. TimeNetAccessError: If the output is remote and gets a release without access declared ``open``. - """ + """ # ruff: ignore[docstring-extraneous-exception] - raised by declared and the build # known_ids() imports no connector. So an unknown id fails before any environment exists. + dataset_id, version = split_ref(release) known = known_ids() if dataset_id not in known: raise typer.BadParameter(f"no connector for {dataset_id!r}; known: {', '.join(known) or '(none)'}") + if version is not None: + declared(dataset_id, version) forwarded = out if out is not None else settings().registry if forwarded is None: forwarded = str(settings().registry_path) @@ -219,9 +223,9 @@ def build_one( # ruff: ignore[too-many-arguments] isolated = settings(isolation=override).isolation == "on" def build_parent(step: Step) -> object: - builder = find_builder(step.release.dataset_id) - if builder is not None and not _in_repo(builder): - return _build_with(builder, step, target) + found = installed_builder(step.release.dataset_id) + if found is not None and not _in_repo(found.backend): + return _build_with(found.backend, step, target) # Each parent builds like the dataset itself. An isolated child makes the plan again. The # child finds every parent in the output, so it only opens the parents. return _build( diff --git a/packages/timenet-connectors/src/timenet_connectors/discovery.py b/packages/timenet-connectors/src/timenet_connectors/discovery.py index 313333934..ebdec3c4a 100644 --- a/packages/timenet-connectors/src/timenet_connectors/discovery.py +++ b/packages/timenet-connectors/src/timenet_connectors/discovery.py @@ -11,11 +11,12 @@ import importlib.util from pathlib import Path import pkgutil +import warnings import yaml from timenet.connectors import BaseConnector -from timenet.errors import TimeNetInvalidCardError +from timenet.errors import ConnectorCardWarning import timenet_connectors.datasets as _datasets @@ -86,30 +87,46 @@ def requirements_for(dataset_id: str) -> Path | None: def known_ids() -> list[str]: """List dataset IDs from connector cards without importing connectors. + A card that cannot be read, is not a YAML mapping, or lacks ``dataset_id`` is skipped with a + :class:`~timenet.errors.ConnectorCardWarning`, so it fails only its own dataset. + Returns: The sorted dataset IDs, with duplicates removed. - - Raises: - TimeNetInvalidCardError: If a card is unreadable, is not a YAML mapping, or lacks ``dataset_id``. """ ids: set[str] = set() for root in _datasets.__path__: # A flat id sits at ``datasets//``, an ``org/name`` id one level deeper. for card in (*Path(root).glob("*/dataset.yaml"), *Path(root).glob("*/*/dataset.yaml")): - # A card that cannot be read fails each lookup, as DatasetMetadata.from_yaml fails for it. - try: - raw = yaml.safe_load(card.read_text(encoding="utf-8")) - except (OSError, yaml.YAMLError) as error: - raise TimeNetInvalidCardError(f"could not read dataset card {card}: {error}") from error - if not isinstance(raw, dict): - raise TimeNetInvalidCardError(f"dataset card {card} must be a YAML mapping, got {type(raw).__name__}") - declared = raw.get("dataset_id") - if not declared: - raise TimeNetInvalidCardError(f"dataset card {card} must declare a dataset_id") - ids.add(declared) + declared = _declared_id(card) + if declared is not None: + ids.add(declared) return sorted(ids) +def _declared_id(card: Path) -> str | None: + """Read the dataset ID of a card, or warn and return ``None`` for a card that cannot be read. + + Args: + card: The ``dataset.yaml`` file. + + Returns: + The declared dataset ID, or ``None``. + """ + try: + raw = yaml.safe_load(card.read_text(encoding="utf-8")) + except (OSError, yaml.YAMLError) as error: + problem = f"could not read dataset card {card}: {error}" + else: + if not isinstance(raw, dict): + problem = f"dataset card {card} must be a YAML mapping, got {type(raw).__name__}" + elif not raw.get("dataset_id"): + problem = f"dataset card {card} must declare a dataset_id" + else: + return raw["dataset_id"] + warnings.warn(f"{problem}; its dataset is skipped", ConnectorCardWarning, stacklevel=3) + return None + + def _connector_spec(dataset_id: str) -> importlib.machinery.ModuleSpec | None: """Find a connector module's spec without importing the module itself. diff --git a/packages/timenet-connectors/tests/test_build_backend.py b/packages/timenet-connectors/tests/test_build_backend.py index d7d08ec3a..e17d31ab8 100644 --- a/packages/timenet-connectors/tests/test_build_backend.py +++ b/packages/timenet-connectors/tests/test_build_backend.py @@ -4,8 +4,8 @@ import pytest -from timenet.builders import find_card -from timenet.errors import TimeNetInvalidCardError +from timenet.builders import installed_builder +from timenet.errors import ConnectorCardWarning, TimeNetInvalidCardError from timenet.manifest import Manifest, PayloadSource from timenet.manifest.files import FilePart from timenet.reader import TimeFReader @@ -66,18 +66,19 @@ def test_a_card_that_cannot_be_read_raises_and_only_a_missing_connector_gives_no ("title: no id\n", "dataset card {} must declare a dataset_id"), ], ) -def test_a_card_that_cannot_be_read_fails_each_lookup_with_the_card_error(tmp_path, monkeypatch, text, message): - for name, card in [("good", "dataset_id: test/good\n"), ("broken", text)]: - (tmp_path / "test" / name).mkdir(parents=True) - (tmp_path / "test" / name / "dataset.yaml").write_text(card) - monkeypatch.setattr(datasets_module, "__path__", [str(tmp_path)]) +def test_a_card_that_cannot_be_read_is_skipped_with_a_warning(tmp_path, monkeypatch, text, message): + (tmp_path / "test" / "broken").mkdir(parents=True) + (tmp_path / "test" / "broken" / "dataset.yaml").write_text(text) + monkeypatch.setattr(datasets_module, "__path__", [*datasets_module.__path__, str(tmp_path)]) expected = re.escape(message.format(tmp_path / "test" / "broken" / "dataset.yaml")) - with pytest.raises(TimeNetInvalidCardError, match=expected): - known_ids() + with pytest.warns(ConnectorCardWarning, match=expected): + assert "timenet/hello-world" in known_ids() # The installed entry point loads the builder backend of this package, which lists its dataset IDs. - with pytest.raises(TimeNetInvalidCardError, match=expected): - find_card("test/good") + with pytest.warns(ConnectorCardWarning, match=expected): + found = installed_builder("timenet/hello-world") + assert found is not None + assert found.card.dataset_id == "timenet/hello-world" def test_build_runs_in_an_isolated_environment_by_default(tmp_path, monkeypatch): diff --git a/packages/timenet-connectors/tests/test_build_cli.py b/packages/timenet-connectors/tests/test_build_cli.py index c104a4ce9..93e4b8fc1 100644 --- a/packages/timenet-connectors/tests/test_build_cli.py +++ b/packages/timenet-connectors/tests/test_build_cli.py @@ -6,6 +6,7 @@ import pytest from typer.testing import CliRunner +from timenet.errors import TimeNetVersionUnavailableError from timenet.manifest import Manifest from timenet.reader import TimeFReader from timenet.registry import open_local_version @@ -89,6 +90,23 @@ def _unexpected_pipeline(*args, **kwargs): assert called["isolated"] is True +def test_build_takes_an_exact_release_and_refuses_another_version(monkeypatch, tmp_path): + built = [] + + def _fake_isolated(dataset_id, root, **kwargs): + built.append(dataset_id) + return Path(root) / "timenet/hello-world/1.0.0" + + monkeypatch.setattr(cli_module, "run_isolated", _fake_isolated) + result = runner.invoke(app, ["build", "timenet/hello-world@1.0.0", "--out", str(tmp_path)]) + + assert result.exit_code == 0 + assert built == ["timenet/hello-world"] + result = runner.invoke(app, ["build", "timenet/hello-world@9.9.9", "--out", str(tmp_path)]) + assert isinstance(result.exception, TimeNetVersionUnavailableError) + assert built == ["timenet/hello-world"] + + def test_build_rejects_an_unknown_id_without_importing_anything(monkeypatch, tmp_path): # A connector's dependencies live in its own environment, not this one, so importing one here # to answer "which ids exist?" would raise ModuleNotFoundError instead of the unknown-id error. diff --git a/packages/timenet-connectors/tests/test_build_plan.py b/packages/timenet-connectors/tests/test_build_plan.py index dbcc89edf..303ea6c94 100644 --- a/packages/timenet-connectors/tests/test_build_plan.py +++ b/packages/timenet-connectors/tests/test_build_plan.py @@ -449,8 +449,7 @@ def _isolated_builds(calls: list) -> object: def run_isolated(dataset_id, root, *, force=False, **_): calls.append((dataset_id, force)) registry = LocalRegistry(Path(root)) - backend = builders.find_builder(dataset_id) - metadata = None if backend is None else backend.card(dataset_id) + metadata = builders.find_card(dataset_id) assert metadata is not None with open_parents(metadata, registry) as context: dataset = TimeFDataset(metadata=metadata) if context.parents else make_dataset() diff --git a/packages/timenet/src/timenet/builders.py b/packages/timenet/src/timenet/builders.py index 0dcde36b4..9d5c7c6fa 100644 --- a/packages/timenet/src/timenet/builders.py +++ b/packages/timenet/src/timenet/builders.py @@ -8,13 +8,13 @@ from collections.abc import Iterable, Mapping from dataclasses import dataclass -from importlib.metadata import EntryPoint, entry_points +from importlib.metadata import entry_points from pathlib import Path from typing import TYPE_CHECKING, Protocol -from timenet.errors import TimeFValidationError +from timenet.errors import TimeNetDatasetNotFoundError, TimeNetVersionUnavailableError from timenet.progress import ProgressCallback -from timenet.types import DatasetMetadata +from timenet.types import DatasetMetadata, Version from timenet.types.references import DatasetRef @@ -166,82 +166,80 @@ def build_prepared( ... -def find_builder(dataset_id: str) -> BuilderBackend | None: - """Find the first registered backend that lists the dataset ID. +@dataclass(frozen=True) +class InstalledBuilder: + """A dataset's build backend and connector card. - Args: - dataset_id: The dataset ID. + Attributes: + backend: The installed backend that builds the dataset. + card: The dataset's connector card. + """ - Returns: - The backend, or ``None`` if no backend lists the dataset ID. + backend: BuilderBackend + card: DatasetMetadata - Raises: - TimeFValidationError: If a registered backend does not implement ``dataset_ids()``. - TimeNetInvalidCardError: If a connector card cannot be read. - """ # ruff: ignore[docstring-extraneous-exception] - raised by _claim and the backend - claim = _claim(dataset_id) - return None if claim is None else claim[1] + @property + def release(self) -> DatasetRef: + """The release that the card declares.""" + return DatasetRef(dataset_id=self.card.dataset_id, version=self.card.dataset_version) -def _claim(dataset_id: str) -> tuple[EntryPoint, BuilderBackend] | None: - """Find the first entry point and backend that list the dataset ID. +def installed_builder(dataset_id: str) -> InstalledBuilder | None: + """Find the first registered backend that lists a dataset, and read its card. Args: dataset_id: The dataset ID. Returns: - The entry point and backend, or ``None`` if no backend lists the dataset ID. + The installed builder, or ``None`` if no backend lists the dataset or the backend has no card for it. Raises: - TimeFValidationError: If a registered backend does not implement ``dataset_ids()``. - TimeNetInvalidCardError: If a connector card cannot be read. - """ # ruff: ignore[docstring-extraneous-exception] - the card error is raised by the backend + TimeNetInvalidCardError: If the dataset's card cannot be read. + """ # ruff: ignore[docstring-extraneous-exception] - raised by the backend for entry in entry_points(group=ENTRY_POINT_GROUP): backend = entry.load()() - dataset_ids = getattr(backend, "dataset_ids", None) - if not callable(dataset_ids): - raise TimeFValidationError( - f"the builder backend of entry point {entry.name!r} ({entry.value}) must implement dataset_ids()" - ) - if dataset_id in dataset_ids(): - return entry, backend + if dataset_id in backend.dataset_ids(): + card = backend.card(dataset_id) + return None if card is None else InstalledBuilder(backend, card) return None -def find_card(dataset_id: str) -> DatasetMetadata | None: - """Read a dataset's card from the first registered backend that lists it. +def declared(dataset_id: str, version: Version | str | None = None) -> InstalledBuilder: + """Find a dataset's build backend and check its version. Args: dataset_id: The dataset ID. + version: The version to build, or ``None`` for the version that the card declares. Returns: - The card, or ``None`` if no backend lists the dataset or the backend has no card. + The installed builder, whose ``release`` is the release to build. Raises: - TimeFValidationError: If a registered backend does not implement ``dataset_ids()``. - TimeNetInvalidCardError: If a connector card cannot be read. - """ # ruff: ignore[docstring-extraneous-exception] - raised by _claim and the backend - claim = _claim(dataset_id) - return None if claim is None else claim[1].card(dataset_id) - + TimeNetDatasetNotFoundError: If no installed connector builds the dataset. + TimeNetVersionUnavailableError: If the connector builds another version. + TimeNetInvalidCardError: If the dataset's card cannot be read. + """ # ruff: ignore[docstring-extraneous-exception] - the card error is raised by the backend + found = installed_builder(dataset_id) + if found is None: + raise TimeNetDatasetNotFoundError(f"no installed connector builds {dataset_id!r}") + if version is not None: + wanted = DatasetRef(dataset_id=dataset_id, version=version) + if wanted.version != found.card.dataset_version: + raise TimeNetVersionUnavailableError(wanted, found.card.dataset_version) + return found -def exact_builder(reference: DatasetRef) -> BuilderBackend | None: - """Find a backend whose card matches the requested dataset version. - Check only the first registered backend that lists the dataset ID. +def find_card(dataset_id: str) -> DatasetMetadata | None: + """Read a dataset's card from the first registered backend that lists it. Args: - reference: The dataset ID and version to build. + dataset_id: The dataset ID. Returns: - The backend, or ``None`` if no backend lists the dataset or its card is absent or has another version. + The card, or ``None`` if no backend lists the dataset or the backend has no card. Raises: - TimeFValidationError: If a registered backend does not implement ``dataset_ids()``. - TimeNetInvalidCardError: If a connector card cannot be read. - """ # ruff: ignore[docstring-extraneous-exception] - raised by _claim and the backend - claim = _claim(reference.dataset_id) - if claim is None: - return None - card = claim[1].card(reference.dataset_id) - return claim[1] if card is not None and card.dataset_version == reference.version else None + TimeNetInvalidCardError: If the card cannot be read. + """ # ruff: ignore[docstring-extraneous-exception] - raised by the backend + found = installed_builder(dataset_id) + return None if found is None else found.card diff --git a/packages/timenet/src/timenet/client.py b/packages/timenet/src/timenet/client.py index c4b6d23ca..a3156a837 100644 --- a/packages/timenet/src/timenet/client.py +++ b/packages/timenet/src/timenet/client.py @@ -14,7 +14,7 @@ from pathlib import Path from typing import TYPE_CHECKING, TypeAlias, TypeVar -from timenet.builders import exact_builder, find_builder +from timenet.builders import installed_builder from timenet.config import settings from timenet.dataset import TimeFDataset from timenet.errors import ( @@ -401,7 +401,7 @@ def _prepare( Raises: TimeNetMissingParentError: If a payload source is missing and cannot be copied or built. TimeNetInvalidCardError: If a connector's dataset card cannot be read. - """ # ruff: ignore[docstring-extraneous-exception] - raised by _obtain and exact_builder + """ # ruff: ignore[docstring-extraneous-exception] - raised by _obtain and installed_builder try: manifest = registry.get_manifest(dataset_id, version) except TimeNetDatasetNotFoundError as miss: @@ -417,9 +417,10 @@ def _prepare( ) except TimeNetMissingParentError as missing: source = missing.missing - builder = exact_builder(source) if auto_build and source not in built else None - if builder is None: + found = installed_builder(source.dataset_id) if auto_build and source not in built else None + if found is None or found.release != source: raise + builder = found.backend pins = next(pinned for pinned in manifest.payload_sources.values() if pinned.release == source) builder.build( source.dataset_id, @@ -454,12 +455,13 @@ def _obtain( # ruff: ignore[too-many-arguments] return fetch_from.get_manifest(dataset_id, version) except TimeNetDatasetNotFoundError: pass - builder = find_builder(dataset_id) if auto_build else None - if builder is None: + found = installed_builder(dataset_id) if auto_build else None + if found is None: hint = _missing_hint(auto_build=auto_build, fetch_from=fetch_from) if hint is None: raise miss raise TimeNetDatasetNotFoundError(f"{miss}. {hint}") from miss + builder = found.backend # A connector produces one declared version. Reject a pin it cannot satisfy before the # build runs, so a wrong pin fails fast instead of after a full build. Sentinels are # not pins, so a fresh build satisfies them. diff --git a/packages/timenet/src/timenet/coordinator/__init__.py b/packages/timenet/src/timenet/coordinator/__init__.py index ff32d1012..fce0d199d 100644 --- a/packages/timenet/src/timenet/coordinator/__init__.py +++ b/packages/timenet/src/timenet/coordinator/__init__.py @@ -1,5 +1,4 @@ """Plan and run dataset builds and reads. -This internal package has no stable interface. -Use ``timenet-build`` to build datasets and ``TimeNet`` to read them. +Not part of the user API: use ``timenet-build`` to build datasets and ``TimeNet`` to read them. """ diff --git a/packages/timenet/src/timenet/coordinator/build.py b/packages/timenet/src/timenet/coordinator/build.py index 9b7774097..1a913442f 100644 --- a/packages/timenet/src/timenet/coordinator/build.py +++ b/packages/timenet/src/timenet/coordinator/build.py @@ -2,9 +2,9 @@ from collections.abc import Mapping -from timenet.builders import BuildOptions, exact_builder, find_card +from timenet.builders import BuildOptions, declared from timenet.coordinator.plan import Plan, Step, plan_parents -from timenet.errors import TimeFValidationError, TimeNetDatasetNotFoundError, TimeNetVersionUnavailableError +from timenet.errors import TimeFValidationError, TimeNetDatasetNotFoundError from timenet.manifest import Manifest, PayloadSource from timenet.progress import ProgressCallback, StepFinished, StepStarted from timenet.registry import copy_release @@ -59,14 +59,8 @@ def build_release( # ruff: ignore[too-many-arguments] """ # ruff: ignore[docstring-extraneous-exception] - raised by the plan, the copy and the backends options = options or BuildOptions() dataset_id, version = split_ref(dataset) - card = find_card(dataset_id) - if card is None: - raise TimeNetDatasetNotFoundError(f"no installed connector builds {dataset_id!r}") - if version is not None: - wanted = DatasetRef(dataset_id=dataset_id, version=version) - if wanted.version != card.dataset_version: - raise TimeNetVersionUnavailableError(wanted, card.dataset_version) - requested = DatasetRef(dataset_id=card.dataset_id, version=card.dataset_version) + found = declared(dataset_id, version) + card, requested = found.card, found.release steps = plan_parents( dataset_id, store, @@ -141,10 +135,8 @@ def _build(step: Step, store: LocalRegistry, *, force: bool, options: BuildOptio Raises: TimeNetDatasetNotFoundError: If no backend builds this version or the result lacks a release or payload source. - """ - builder = exact_builder(step.release) - if builder is None: - raise TimeNetDatasetNotFoundError(f"no installed connector builds {step.release}") + """ # ruff: ignore[docstring-extraneous-exception] - raised by declared and prepare_local_copy + builder = declared(step.release.dataset_id, step.release.version).backend if options.progress is not None: options.progress(StepStarted(step.release, "build")) builder.build_prepared(step.release, store, force=force, pinned_by=step.pinned_by, options=options) diff --git a/packages/timenet/src/timenet/errors.py b/packages/timenet/src/timenet/errors.py index f675cbaa2..970d9d509 100644 --- a/packages/timenet/src/timenet/errors.py +++ b/packages/timenet/src/timenet/errors.py @@ -198,6 +198,10 @@ class TimeNetWarning(UserWarning): """Base class for all TimeNet warnings.""" +class ConnectorCardWarning(TimeNetWarning): + """An unreadable or invalid connector card excludes its dataset from listings.""" + + class SpanOutsideWindowWarning(TimeNetWarning): """An annotation's span falls outside the window of the series it names, and is kept.""" diff --git a/packages/timenet/tests/test_builders.py b/packages/timenet/tests/test_builders.py index 2b5a0843d..ca0edeebc 100644 --- a/packages/timenet/tests/test_builders.py +++ b/packages/timenet/tests/test_builders.py @@ -1,10 +1,7 @@ -from types import SimpleNamespace - import pytest -from timenet import builders -from timenet.builders import exact_builder, find_builder, find_card -from timenet.errors import TimeFValidationError +from timenet.builders import declared, installed_builder +from timenet.errors import TimeNetDatasetNotFoundError, TimeNetVersionUnavailableError from timenet.testing import install_backends, make_dataset from timenet.types.references import DatasetRef @@ -24,46 +21,27 @@ def build_prepared(self, release, store, *, force, pinned_by, options): raise AssertionError(f"the lookup does not build {release}") -def test_find_builder_returns_none_when_nothing_claims_the_id(monkeypatch): - install_backends(monkeypatch, _Backend("a/b")) - assert find_builder("x/y") is None - - -def test_find_builder_returns_the_first_backend_whose_dataset_ids_hold_the_id(monkeypatch): - wanted = _Backend("a/c", "x/y") - install_backends(monkeypatch, _Backend("a/b"), wanted, _Backend("x/y")) - assert find_builder("x/y") is wanted - - -def test_find_builder_names_the_entry_point_of_a_backend_without_dataset_ids(monkeypatch): - class _NoIds: - def knows(self, dataset_id): - return True - - # install_backends() gives entry points without a name, so this test makes its own. - entry = SimpleNamespace(name="old", value="old_package:Builder", load=lambda: _NoIds) - monkeypatch.setattr(builders, "entry_points", lambda group: [entry]) - with pytest.raises(TimeFValidationError, match=r"'old' \(old_package:Builder\) must implement dataset_ids\(\)"): - find_builder("x/y") - - -def test_find_builder_returns_none_with_no_registered_backends(monkeypatch): - monkeypatch.setattr(builders, "entry_points", lambda group: []) - assert find_builder("x/y") is None - - -def test_find_card_returns_the_card_of_the_backend_that_claims_the_id(monkeypatch): +def test_installed_builder_returns_the_first_backend_whose_dataset_ids_hold_the_id(monkeypatch): card = make_dataset().metadata - backends = [_Backend("a/b"), _Backend(card.dataset_id, card=card)] - install_backends(monkeypatch, *backends) - assert find_card(card.dataset_id) is card - assert find_card("x/y") is None + wanted = _Backend("a/c", card.dataset_id, card=card) + install_backends(monkeypatch, _Backend("a/b"), wanted, _Backend(card.dataset_id, card=card)) + found = installed_builder(card.dataset_id) + assert found is not None + assert (found.backend, found.card) == (wanted, card) + assert installed_builder("x/y") is None -def test_exact_builder_returns_the_backend_only_for_the_version_of_its_card(monkeypatch): +def test_declared_returns_the_release_of_the_card_and_refuses_another_version(monkeypatch): card = make_dataset().metadata - backend = _Backend(card.dataset_id, card=card) - install_backends(monkeypatch, backend) - assert exact_builder(DatasetRef(dataset_id=card.dataset_id, version=card.dataset_version)) is backend - assert exact_builder(DatasetRef.parse(f"{card.dataset_id}@2.0.0")) is None - assert exact_builder(DatasetRef.parse("x/y@1.0.0")) is None + install_backends(monkeypatch, _Backend(card.dataset_id, card=card)) + release = DatasetRef(dataset_id=card.dataset_id, version=card.dataset_version) + assert declared(card.dataset_id).release == release + assert declared(card.dataset_id, card.dataset_version).release == release + with pytest.raises(TimeNetVersionUnavailableError) as raised: + declared(card.dataset_id, "2.0.0") + assert (raised.value.release, raised.value.declared) == ( + DatasetRef.parse(f"{card.dataset_id}@2.0.0"), + card.dataset_version, + ) + with pytest.raises(TimeNetDatasetNotFoundError, match="no installed connector builds 'x/y'"): + declared("x/y") diff --git a/packages/timenet/tests/test_client.py b/packages/timenet/tests/test_client.py index 1ddcf3a21..785c6e160 100644 --- a/packages/timenet/tests/test_client.py +++ b/packages/timenet/tests/test_client.py @@ -7,6 +7,7 @@ import pytest from timenet import client as client_module +from timenet.builders import InstalledBuilder from timenet.client import TimeNet from timenet.connectors import BaseConnector from timenet.dataset import Record, RegularAxis, Signal, Source, TimeFDataset @@ -89,6 +90,11 @@ def build(self, dataset_id, root, *, force=False, build_parents=False, fetch_fro return root / dataset_id / self.VERSION +def _installed_builders(builder, card=None): + """Return a stand-in for ``installed_builder`` that answers every dataset with ``builder``, and with ``card`` or its card.""" + return lambda dataset_id: InstalledBuilder(builder, card or builder.card(dataset_id)) + + def test_a_local_child_copies_its_missing_payload_source_from_fetch_from(tmp_path): origin, reference = _child_of_the_demo(tmp_path / "origin") cache = tmp_path / "cache" @@ -115,7 +121,7 @@ def test_a_missing_version_is_copied_from_fetch_from_before_it_is_built(tmp_path if in_fetch_from: _child_of_the_demo(origin.root) calls = [] - monkeypatch.setattr(client_module, "find_builder", lambda dataset_id: _Builder(calls)) + monkeypatch.setattr(client_module, "installed_builder", _installed_builders(_Builder(calls))) with TimeNet(tmp_path / "local").open("test/child", fetch_from=origin) as reader: assert reader.read().records @@ -230,7 +236,7 @@ def test_a_damaged_release_is_repaired_from_fetch_from_and_never_built_over(tmp_ control.write_bytes(b"cut short") calls = [] install_backends(monkeypatch, _Builder(calls)) - monkeypatch.setattr(client_module, "find_builder", lambda dataset_id: _Builder(calls)) + monkeypatch.setattr(client_module, "installed_builder", _installed_builders(_Builder(calls))) with pytest.raises(TimeNetFileError) as caught: TimeNet(registry).open("test/child") @@ -259,7 +265,7 @@ def test_a_corrupted_copy_from_fetch_from_names_the_transfer_without_the_repair_ def test_a_remote_client_takes_no_fetch_from_and_never_builds(tmp_path, monkeypatch): calls = [] - monkeypatch.setattr(client_module, "find_builder", lambda dataset_id: _Builder(calls)) + monkeypatch.setattr(client_module, "installed_builder", _installed_builders(_Builder(calls))) remote = RemoteRegistry( "http://api.local", transport=httpx.MockTransport(lambda request: httpx.Response(404)), cache_dir=tmp_path ) @@ -558,7 +564,7 @@ def build(self, dataset_id, root, *, force=False, build_parents=False, fetch_fro return Path(root) / dataset_id / "1.0.0" builder = _Builder() - monkeypatch.setattr(client_module, "find_builder", lambda dataset_id: builder) + monkeypatch.setattr(client_module, "installed_builder", _installed_builders(builder, make_dataset().metadata)) client = TimeNet(registry_root, storage_path=tmp_path / "store") with pytest.raises(TimeNetDatasetNotFoundError, match=r"builds version 1\.0\.0, not the requested 9\.9\.9"): @@ -586,7 +592,7 @@ def build(self, dataset_id, root, *, force=False, build_parents=False, fetch_fro empty_root = tmp_path / "reg" empty_root.mkdir() - monkeypatch.setattr(client_module, "find_builder", lambda dataset_id: _Builder()) + monkeypatch.setattr(client_module, "installed_builder", _installed_builders(_Builder(), make_dataset().metadata)) client = TimeNet(empty_root, storage_path=tmp_path / "store") assert str(client.load("timenet/hello-world", pin).metadata.dataset_version) == "1.0.0" @@ -613,7 +619,7 @@ def build(self, dataset_id, root, *, force=False, build_parents=False, fetch_fro if entry_point == "open_torch": pytest.importorskip("torch") builder = _Builder() - monkeypatch.setattr(client_module, "find_builder", lambda dataset_id: builder) + monkeypatch.setattr(client_module, "installed_builder", _installed_builders(builder, make_dataset().metadata)) client = TimeNet(registry_root, storage_path=tmp_path / "store") # The hint names the keywords that could obtain the release, in a sentence of its own.