diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py index bd3c9d7ab..02b1251a7 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py @@ -8,11 +8,19 @@ 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.""" + """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 this package's dataset IDs without importing connectors. + + Returns: + 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) """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..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,10 +46,10 @@ 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 connector_dir, resolve +from timenet_connectors.discovery import known_ids, resolve from timenet_connectors.download import DownloadProgress, ProgressCallback, progress_sink @@ -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,17 +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``. - """ - 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 + """ # 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) @@ -204,7 +207,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``. @@ -220,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 09dd5a445..ebdec3c4a 100644 --- a/packages/timenet-connectors/src/timenet_connectors/discovery.py +++ b/packages/timenet-connectors/src/timenet_connectors/discovery.py @@ -11,10 +11,12 @@ import importlib.util from pathlib import Path import pkgutil +import warnings import yaml from timenet.connectors import BaseConnector +from timenet.errors import ConnectorCardWarning import timenet_connectors.datasets as _datasets @@ -47,20 +49,17 @@ 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: - """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: 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. + Whether a connector package with a ``dataset.yaml`` card exists. """ try: connector_dir(dataset_id) @@ -70,41 +69,64 @@ 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: 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``. + 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 declared by the connector cards on disk, importing nothing. +def known_ids() -> list[str]: + """List dataset IDs from connector cards without importing connectors. - :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. + 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 declared dataset ids, sorted. + The sorted dataset IDs, with duplicates removed. """ 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") - if 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 63ec0616e..e17d31ab8 100644 --- a/packages/timenet-connectors/tests/test_build_backend.py +++ b/packages/timenet-connectors/tests/test_build_backend.py @@ -1,17 +1,21 @@ from pathlib import Path +import re import sys import pytest -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 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,29 @@ 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"), + ("title: no id\n", "dataset card {} must declare a dataset_id"), + ], +) +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.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.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): 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..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. @@ -106,6 +124,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..303ea6c94 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 @@ -43,11 +43,14 @@ 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 + 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) @@ -453,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() @@ -468,8 +463,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 +599,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 +622,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 +664,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 +703,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 +736,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 +753,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..9d5c7c6fa 100644 --- a/packages/timenet/src/timenet/builders.py +++ b/packages/timenet/src/timenet/builders.py @@ -6,25 +6,56 @@ 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.types import DatasetMetadata +from timenet.errors import TimeNetDatasetNotFoundError, TimeNetVersionUnavailableError +from timenet.progress import ProgressCallback +from timenet.types import DatasetMetadata, Version 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: + """Build settings that do not change the files of the release. + + Attributes: + keep_cache: Keep the raw download cache after the build. + progress: The callback for build progress events, or ``None``. + quiet: Suppress output from the backend and its child processes. + """ + + keep_cache: bool = False + progress: ProgressCallback | None = None + quiet: bool = False + + class BuilderBackend(Protocol): - """Something that can build a dataset from its connector.""" + """An interface for building dataset releases from connectors.""" + + def dataset_ids(self) -> Iterable[str]: + """List the datasets this backend can build. + + Implementations must answer without importing a connector. + + Returns: + The dataset IDs. + + Raises: + TimeNetInvalidCardError: If a connector card cannot be read. + """ + ... def knows(self, dataset_id: str) -> bool: """Report whether this backend has a connector for the id. @@ -105,55 +136,110 @@ 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 the requested dataset version into a local registry. -def find_builder(dataset_id: str) -> BuilderBackend | None: - """Return the first registered backend that claims this dataset id. + 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: 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 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. + """ + ... + + +@dataclass(frozen=True) +class InstalledBuilder: + """A dataset's build backend and connector card. + + Attributes: + backend: The installed backend that builds the dataset. + card: The dataset's connector card. + """ + + backend: BuilderBackend + card: DatasetMetadata + + @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 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. + dataset_id: The dataset ID. Returns: - The backend, or ``None`` when no installed package claims the id. - """ + The installed builder, or ``None`` if no backend lists the dataset or the backend has no card for it. + + Raises: + 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()() - if backend.knows(dataset_id): - return 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: - """Return the card of the installed connector that claims this dataset. +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. + dataset_id: The dataset ID. + version: The version to build, or ``None`` for the version that the card declares. Returns: - The card, or ``None`` when no installed package claims the dataset. + The installed builder, whose ``release`` is the release to build. Raises: - 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) + 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: - """Return the backend that can build exactly this release. - - 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. +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 the version. + dataset_id: The dataset ID. Returns: - The backend, or ``None`` if no installed backend claims the dataset or its card declares - another version. + The card, or ``None`` if no backend lists the dataset or the backend has no card. Raises: - TimeNetInvalidCardError: If the card of the connector cannot be read. + TimeNetInvalidCardError: If the card 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 + 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 64d529a0e..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, @@ -443,27 +444,24 @@ 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: 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 7e1691695..fce0d199d 100644 --- a/packages/timenet/src/timenet/coordinator/__init__.py +++ b/packages/timenet/src/timenet/coordinator/__init__.py @@ -1 +1,4 @@ -"""Coordinate dataset builds and reads.""" +"""Plan and run dataset builds and reads. + +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 new file mode 100644 index 000000000..1a913442f --- /dev/null +++ b/packages/timenet/src/timenet/coordinator/build.py @@ -0,0 +1,146 @@ +"""Prepare parent releases and build a dataset into a local registry.""" + +from collections.abc import Mapping + +from timenet.builders import BuildOptions, declared +from timenet.coordinator.plan import Plan, Step, plan_parents +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 +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: + """Prepare a release's dependencies and build it into a local registry. + + Copy missing dependencies from ``fetch_from``, or build them if ``build_parents`` is set. + Reuse an existing release unless ``force`` is set. + + Args: + 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 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 completed plan, with steps in execution order. + + Raises: + TimeNetDatasetNotFoundError: If no installed backend builds the dataset or a missing parent. + 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 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) + found = declared(dataset_id, version) + card, requested = found.card, found.release + 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 {})) + 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 == "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)) + + +def _copy( + release: DatasetRef, fetch_from: BaseRegistry, store: LocalRegistry, *, progress: ProgressCallback | None +) -> None: + """Copy a release and its required payload sources into the local registry. + + If the release is intact, copy only its missing or damaged payload sources. + + Args: + release: The release to copy. + fetch_from: The registry to copy from. + store: The destination registry. + progress: The callback for copy progress events, 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: + """Read a release's manifest and check its file sizes. + + Args: + store: The local registry. + reference: The release to check. + + Returns: + 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)) + 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: + """Build a planned release and check its required files. + + Args: + step: The build step. + store: The local registry to build into. + force: Rebuild even if ``store`` already holds the release. + options: The build settings. + + Raises: + TimeNetDatasetNotFoundError: If no backend builds this version or the result lacks a release or payload source. + """ # 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) + # 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..4600b620f 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 copy and build steps for a dataset, in execution order.""" + + steps: tuple[Step, ...] + + @dataclass(frozen=True) class _Release: """A required release, its dependencies, and its planned action.""" @@ -64,11 +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 + """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) @@ -79,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: @@ -94,13 +105,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,37 +196,39 @@ 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. + """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: Plan a rebuild's parents even if ``output`` holds the requested release. + 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 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 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 @@ -226,6 +239,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 +249,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 +333,21 @@ 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``. + """Find a release in the output or the source registry. - A local output copy is valid only if its files exist and match their recorded sizes. + 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 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 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 @@ -344,14 +362,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,25 +380,25 @@ 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: - """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 false. - TimeNetAccessError: If the card lacks open access and the output requires it. + 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..970d9d509 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,30 @@ 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 cannot build the requested dataset version. + + Attributes: + release: The requested release. + declared: The version that the connector builds. + hint: Suggested next steps to append to the error message, 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}. " + f"Ask for version {self.declared}, or leave out the 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).""" @@ -173,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/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..69b4fde41 --- /dev/null +++ b/packages/timenet/tests/coordinator/test_build.py @@ -0,0 +1,323 @@ +import shutil + +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. " + "Ask for version 1.0.0, or leave out the version." + ) + 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_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) diff --git a/packages/timenet/tests/test_builders.py b/packages/timenet/tests/test_builders.py index 06cd0c53a..ca0edeebc 100644 --- a/packages/timenet/tests/test_builders.py +++ b/packages/timenet/tests/test_builders.py @@ -1,52 +1,47 @@ -from pathlib import Path +import pytest -from timenet import builders -from timenet.builders import find_builder, find_card -from timenet.testing import make_dataset +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 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 load(self): - return lambda: self._backend - - -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_returns_the_backend_that_claims_the_id(monkeypatch): - wanted = _Backend("x/y") - monkeypatch.setattr(builders, "entry_points", lambda group: [_Entry(_Backend("a/b")), _Entry(wanted)]) - assert find_builder("x/y") is wanted - - -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_installed_builder_returns_the_first_backend_whose_dataset_ids_hold_the_id(monkeypatch): + card = make_dataset().metadata + 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_find_card_returns_the_card_of_the_backend_that_claims_the_id(monkeypatch): +def test_declared_returns_the_release_of_the_card_and_refuses_another_version(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]) - assert find_card(card.dataset_id) is card - assert find_card("x/y") 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 0ad9fc02f..785c6e160 100644 --- a/packages/timenet/tests/test_client.py +++ b/packages/timenet/tests/test_client.py @@ -6,7 +6,8 @@ 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.builders import InstalledBuilder from timenet.client import TimeNet from timenet.connectors import BaseConnector from timenet.dataset import Record, RegularAxis, Signal, Source, TimeFDataset @@ -24,7 +25,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 +68,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 @@ -86,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" @@ -112,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 @@ -187,7 +196,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,8 +235,8 @@ 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)) - monkeypatch.setattr(client_module, "find_builder", lambda dataset_id: _Builder(calls)) + install_backends(monkeypatch, _Builder(calls)) + monkeypatch.setattr(client_module, "installed_builder", _installed_builders(_Builder(calls))) with pytest.raises(TimeNetFileError) as caught: TimeNet(registry).open("test/child") @@ -256,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 ) @@ -555,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"): @@ -583,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" @@ -610,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. diff --git a/packages/timenet/tests/test_errors.py b/packages/timenet/tests/test_errors.py index 075de37d1..4e3d72114 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 @@ -64,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"), @@ -125,20 +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."