From 3ab5779ac5a401e6a81db5ef8235c938c8397ff5 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 13:56:53 +0100 Subject: [PATCH 1/6] feat(client)!: read releases through the coordinator open(), load() and open_torch() now read through read_release(), the read coordinator. A read opens each release where it is and copies nothing. A local client asks its own registry first, then the registry that fetch_from names, then the installed connectors if auto_build is set. A remote client asks only its own registry and never builds. Without a version, a read takes the newest version of the first source that serves the dataset. So a copy in the local registry answers, and the read asks no other source and builds nothing. An empty version string means the same. With org/x@latest or version="latest", a read takes the highest version across all the sources. _read_sources() maps fetch_from and auto_build onto that list of sources. The error hints still name the two keywords. A missing release or payload source names each keyword that the call did not use. An access error on a local client with auto_build=False names auto_build=True. A remote client keeps its hint to build the dataset locally, and its other errors name no keyword. _prepare(), _obtain() and _require_open() go away, and the client no longer calls prepare_local_copy(). The before_fetch callback of prepare_local_copy() had no other caller, so it goes too. No caller passes pinned_by to plan_release_build() any more, so that parameter goes as well. BREAKING CHANGE: open(fetch_from=...) reads in place and copies nothing into the local registry. A damaged local copy raises TimeNetFileError also with fetch_from, and the error has no repair hint. "latest" is no longer the same as no version. It is the highest version across the local registry, fetch_from and, with auto_build, the installed connectors. So a read of "latest" builds a newer version that a connector declares, and it needs each registry to be reachable. A payload source that the local registry lacks is read from fetch_from in place. A registry that lists a release without serving it falls through to a build when auto_build is set, also for a read. The message for a missing release names each source, and the hint names fetch_from= as a registry to read from. The before_fetch argument of prepare_local_copy() is gone. Part of TNET-157. --- .../src/timenet_connectors/api.py | 5 +- .../tests/test_build_plan.py | 7 +- packages/timenet/src/timenet/client.py | 298 +++++++----------- .../timenet/src/timenet/coordinator/build.py | 10 +- .../timenet/src/timenet/coordinator/plan.py | 6 +- .../src/timenet/registry/dependencies.py | 15 +- .../timenet/tests/coordinator/test_build.py | 14 - .../timenet/tests/registry/test_remote.py | 3 +- packages/timenet/tests/test_client.py | 149 ++++----- 9 files changed, 205 insertions(+), 302 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/api.py b/packages/timenet-connectors/src/timenet_connectors/api.py index 4674cdeda..25c5d7188 100644 --- a/packages/timenet-connectors/src/timenet_connectors/api.py +++ b/packages/timenet-connectors/src/timenet_connectors/api.py @@ -111,8 +111,9 @@ def load( Args: dataset_id: The dataset ID (``org/name``). - version: The version declared by the connector, or ``None`` to load the latest version. - fetch_from: The registry, or its URI, to copy missing parents and payload sources from. + version: The version to load, which the card must declare, or ``None`` for the latest. + fetch_from: The registry, or its URI, that the build copies missing parent releases + from, and that the read reads missing payload sources from, where they are. build_parents: Build missing parent releases from their installed connectors. Returns: diff --git a/packages/timenet-connectors/tests/test_build_plan.py b/packages/timenet-connectors/tests/test_build_plan.py index 531d73db3..48acf8bf2 100644 --- a/packages/timenet-connectors/tests/test_build_plan.py +++ b/packages/timenet-connectors/tests/test_build_plan.py @@ -567,7 +567,7 @@ def test_load_obtains_a_missing_parent_before_it_builds_and_reads(tmp_path, monk assert calls == (builds if obtain == "fetch_from" else [(PARENT.dataset_id, False), *builds]) -def test_load_copies_a_missing_payload_source_through_fetch_from(tmp_path, monkeypatch): +def test_load_reads_a_missing_payload_source_through_fetch_from(tmp_path, monkeypatch): out = LocalRegistry(tmp_path / "out") _parent_without_its_source(out) origin = LocalRegistry(tmp_path / "origin") @@ -575,14 +575,15 @@ def test_load_copies_a_missing_payload_source_through_fetch_from(tmp_path, monke monkeypatch.setenv("TIMENET_HOME", str(tmp_path / "home")) monkeypatch.setenv("TIMENET_REGISTRY", str(out.root)) # The build has nothing to do, and no builder backend can build the payload source, so only the - # read can copy it. + # read can get it. monkeypatch.setattr(api_module, "build", lambda dataset_id, **options: None) monkeypatch.setattr(builders, "entry_points", lambda group: []) dataset = timenet_connectors.load(PARENT.dataset_id, fetch_from=origin) - assert out.exists(GRAND.dataset_id, str(GRAND.version)) assert len(dataset.records) == 2 + # The read opens the payload source in origin, where it is, and copies nothing. + assert not out.exists(GRAND.dataset_id, str(GRAND.version)) def test_a_pinned_parent_build_hands_its_pins_to_the_isolated_child(tmp_path, monkeypatch): diff --git a/packages/timenet/src/timenet/client.py b/packages/timenet/src/timenet/client.py index da7ece6d4..726748ec8 100644 --- a/packages/timenet/src/timenet/client.py +++ b/packages/timenet/src/timenet/client.py @@ -2,9 +2,10 @@ This class wraps a :class:`~timenet.registry.BaseRegistry` (the catalog) and a local storage path (the download cache). It exposes these methods: ``list``, ``get``, ``search``, ``download``, -``open``, ``load``, and ``open_torch``. Against a local registry, ``open`` and ``load`` build a -dataset the registry does not have when an installed package registers a connector for its id; -against a remote registry they never run connector code. +``open``, ``load``, and ``open_torch``. ``open`` and ``load`` read a release where it is, through +the read coordinator in :mod:`timenet.coordinator.read`. Against a local registry, they build a +release that only an installed connector provides. Against a remote registry, they never run +connector code. """ # The public API has a method named ``list``. Deferred annotations keep the type hint @@ -14,16 +15,13 @@ from pathlib import Path from typing import TYPE_CHECKING, TypeAlias, TypeVar -from timenet.builders import installed_builder from timenet.config import settings -from timenet.coordinator.build import plan_release_build, run_plan from timenet.coordinator.read import read_release from timenet.dataset import TimeFDataset from timenet.errors import ( TimeFValidationError, TimeNetAccessError, TimeNetDatasetNotFoundError, - TimeNetFileError, TimeNetMissingParentError, ) from timenet.manifest import Manifest @@ -31,12 +29,10 @@ from timenet.registry import BaseRegistry, LocalRegistry, open_registry from timenet.registry._paths import release_dir from timenet.registry.base import ProgressCallback -from timenet.registry.dependencies import prepare_local_copy -from timenet.registry.version import DatasetVersion -from timenet.sources import InstalledConnectors +from timenet.sources import InstalledConnectors, ReleaseSource from timenet.types import DatasetMetadata, Domain, License, Task from timenet.types.access import require_open_access -from timenet.types.references import DatasetRef, is_latest, split_ref +from timenet.types.references import DatasetRef, split_ref if TYPE_CHECKING: @@ -49,6 +45,7 @@ T = TypeVar("T") _Metadatas: TypeAlias = list[DatasetMetadata] _OrList: TypeAlias = T | list[T] | None +_Sources: TypeAlias = list[ReleaseSource] def _resolve_ref(dataset_id: str, version: str | None) -> tuple[str, str | None]: @@ -56,18 +53,18 @@ def _resolve_ref(dataset_id: str, version: str | None) -> tuple[str, str | None] Args: dataset_id: A dataset id. It can have a suffix of ``@`` or ``@latest``. - version: An explicit version, or ``None`` for the latest version. + version: An explicit version, ``"latest"``, or ``None`` or ``""`` for no version. Returns: - The bare dataset id and the resolved version. ``None`` means the latest version. + The bare dataset id and the requested version: exact, ``"latest"``, or ``None`` for no version. Raises: TimeFValidationError: The ref and ``version`` both give a version. """ ref_id, ref_version = split_ref(dataset_id) - if ref_version is not None and version is not None: + if ref_version is not None and version: raise TimeFValidationError(f"version given twice: '@{ref_version}' in the id and version={version!r}") - return ref_id, ref_version if ref_version is not None else version + return ref_id, ref_version if ref_version is not None else (version or None) class TimeNet: @@ -199,8 +196,7 @@ def open( auto_build: bool = True, fetch_from: str | BaseRegistry | None = None, ) -> TimeFReader: - # ruff: ignore[docstring-extraneous-exception] (raised by _reject_unhosted_access, _prepare, and the builder) - """Prepare a dataset version and its payload sources, then open it for lazy reads. + """Open a dataset version for lazy reads, where it is. Opening reads the version's manifest, but neither its catalog nor any value. The reader's reads leave stored values behind value handles unless their load plan names them. Close @@ -208,68 +204,73 @@ def open( from their manifests upfront and read payload ranges on demand, reusing complete cached releases. - Against a local registry, the call needs the version and its payload sources. If the call - builds the version, the build also needs the declared parents of the version and their - payload sources. For each of these releases, the call uses the first step that works. It - reuses a complete local copy, or copies the release from ``fetch_from``, or builds the - release from its installed connector. The call builds only if ``auto_build`` is set. If no - step works, the call raises an error. + The call reads each release where it is, and copies nothing. A local client asks its own + registry first, then the registry that ``fetch_from`` names, then the installed connectors + if ``auto_build`` is set. Without a version, the call reads the newest version of the first + of them that serves the dataset. An exact version comes from the first of them that serves + it, and ``"latest"`` is the highest version that any of them provides. Each payload source + of the version resolves on its own, at its exact version. If the call builds the version, + the build also needs the declared parents of the version and their payload sources. + If only a connector provides a release, the call builds it into the local registry first. The call builds a payload source with its own connector, at the exact version of the source. The build gets the pins of the version. The build runs in staging. It publishes the source only if it reproduces the pinned files. A local copy is damaged if its manifest is present but a file is missing or has another - size. If ``fetch_from`` is set, the call repairs a damaged copy from that registry. The call - never builds a release over a damaged copy. Against a remote registry, ``open`` never runs + size. The call checks each release that it opens in a local registry before it reads or + builds anything. It never reads past a damaged copy to a later source, and it never builds + a release over a damaged copy. Against a remote registry, ``open`` never runs connector code. If a remote client gets ``fetch_from``, ``open`` raises an error. Args: dataset_id: The dataset id. - version: The version string, or ``None`` for the latest. - auto_build: Build a missing local release from its connector. Set false to raise instead. - fetch_from: The registry, or its URI, to copy missing local releases from. + version: An exact version, ``"latest"``, or ``None`` for no version. + auto_build: Build a release that only an installed connector provides. Set false to + raise instead. + fetch_from: A registry, or its URI, that a local client reads a release from if its own + registry does not provide it. Returns: A reader for the version. Raises: - TimeNetAccessError: If the dataset is not open access and the registry is not local. - Also if the call must copy a release without access declared ``open`` from a - registry that is not local. - TimeNetDatasetNotFoundError: If the version is absent and nothing builds it. - TimeNetVersionUnavailableError: If the version is absent and the connector declares another - version. The build does not start then. - TimeNetInvalidCardError: If the card of the connector cannot be read. - TimeNetMissingParentError: If a release that the version or its build needs is absent and - the call cannot obtain it. The message names the keyword that can obtain it. - TimeNetFileError: If a local copy is damaged and ``fetch_from`` is not given, or if a - copied file does not match its manifest record. - TimeFFormatError: If the build of a payload source does not reproduce the pinned files. - TimeNetBuildError: If the build runs but fails. + TimeNetAccessError: If a registry that is not local lists the version or a payload + source but does not serve its files, and no later source provides it. + TimeNetDatasetNotFoundError: If no source provides the version. + TimeNetVersionUnavailableError: If no source provides the version, and the installed + connector declares another version. The build does not start then. + TimeNetInvalidCardError: If the card of a connector cannot be read. + TimeNetMissingParentError: If no source provides a payload source, or a release that a + build needs. The message names each keyword that the call did not use. + TimeNetFileError: If a release that the call opens in a local registry has a missing + file or a file of another size. + TimeFFormatError: If a payload source differs from the files that the version pins, + for example because its build does not reproduce them. + TimeNetBuildError: If a build runs but fails. TimeFValidationError: If a remote client gets ``fetch_from``. - """ + """ # ruff: ignore[docstring-extraneous-exception] (raised by read_release and the builder) dataset_id, version = _resolve_ref(dataset_id, version) - registry = self._registry - if not isinstance(registry, LocalRegistry): - if fetch_from is not None: - raise TimeFValidationError( - "fetch_from copies releases into a local registry; a remote client reads from its own registry" - ) - self._reject_unhosted_access(dataset_id, version) - return TimeFReader(read_release(dataset_id, version, sources=[registry], store=None)) - source = open_registry(fetch_from) if isinstance(fetch_from, str) else fetch_from + sources, store = self._read_sources(auto_build=auto_build, fetch_from=fetch_from) + # Maps fetch_from and auto_build onto hints for a local client. try: - return TimeFReader(_prepare(registry, dataset_id, version, auto_build=auto_build, fetch_from=source)) - except TimeNetMissingParentError as error: - error.hint = _missing_hint(auto_build=auto_build, fetch_from=source) + return TimeFReader(read_release(dataset_id, version, sources=sources, store=store)) + except TimeNetAccessError as error: + if store is None: + error.hint = _unhosted_hint(dataset_id) + elif not auto_build: + error.hint = "Build it from its connector with auto_build=True." raise - except TimeNetFileError as error: - # If fetch_from is not set, the error names a damaged local copy. If fetch_from is set, - # the error names a copied file. - if source is None: - error.hint = "Repair it with fetch_from=." + except TimeNetMissingParentError as error: + if store is not None: + error.hint = _missing_hint(auto_build=auto_build, fetch_from=fetch_from) raise + except TimeNetDatasetNotFoundError as error: + hint = None if store is None else _missing_hint(auto_build=auto_build, fetch_from=fetch_from) + # A subclass, such as TimeNetVersionUnavailableError, names its own fix. + if hint is None or type(error) is not TimeNetDatasetNotFoundError: + raise + raise TimeNetDatasetNotFoundError(f"{str(error).rstrip('.')}. {hint}") from error def load( self, @@ -286,32 +287,65 @@ def load( Args: dataset_id: The dataset id. - version: The version string, or ``None`` for the latest. - auto_build: Build a missing local release from its connector. Set false to raise instead. - fetch_from: The registry, or its URI, to copy missing local releases from. + version: An exact version, ``"latest"``, or ``None``, as in :meth:`open`. + auto_build: Build a release that only an installed connector provides. Set false to + raise instead. + fetch_from: A registry, or its URI, that a local client reads a release from if its own + registry does not provide it. Returns: The dataset with every signal array, time offset, and text value loaded. Raises: - TimeNetAccessError: If the dataset is not open access and the registry is not local. - Also if the call must copy a release without access declared ``open`` from a - registry that is not local. - TimeNetDatasetNotFoundError: If the version is absent and nothing builds it. - TimeNetVersionUnavailableError: If the version is absent and the connector declares another - version. The build does not start then. - TimeNetInvalidCardError: If the card of the connector cannot be read. - TimeNetMissingParentError: If a release that the version or its build needs is absent and - the call cannot obtain it. - TimeNetFileError: If a local copy is damaged and ``fetch_from`` is not given, or if a - copied file does not match its manifest record. - TimeFFormatError: If the build of a payload source does not reproduce the pinned files. - TimeNetBuildError: If the build runs but fails. + TimeNetAccessError: If a registry that is not local lists the version or a payload + source but does not serve its files, and no later source provides it. + TimeNetDatasetNotFoundError: If no source provides the version. + TimeNetVersionUnavailableError: If no source provides the version, and the installed + connector declares another version. The build does not start then. + TimeNetInvalidCardError: If the card of a connector cannot be read. + TimeNetMissingParentError: If no source provides a payload source, or a release that a + build needs. + TimeNetFileError: If a release that the call opens in a local registry has a missing + file or a file of another size. + TimeFFormatError: If a payload source differs from the files that the version pins. + TimeNetBuildError: If a build runs but fails. TimeFValidationError: If a remote client gets ``fetch_from``. """ # ruff: ignore[docstring-extraneous-exception] (all come from open()) with self.open(dataset_id, version, auto_build=auto_build, fetch_from=fetch_from) as reader: return reader.read(load=LoadPlan.ALL_EAGER) + def _read_sources( + self, *, auto_build: bool, fetch_from: str | BaseRegistry | None + ) -> tuple[_Sources, LocalRegistry | None]: + """Return the sources of a read, in order, and the local registry that its builds go to. + + Maps ``fetch_from`` and ``auto_build`` onto a list of sources for the read coordinator. A + local client reads from its own registry, then from ``fetch_from``, then from the installed + connectors if ``auto_build`` is set. Its builds go to its own registry. A remote client + reads only from its own registry, and it never builds. + + Returns: + The sources, and the local registry, or ``None`` for a remote client. + + Raises: + TimeFValidationError: If a remote client gets ``fetch_from``. + """ + registry = self._registry + if not isinstance(registry, LocalRegistry): + if fetch_from is not None: + raise TimeFValidationError( + "fetch_from adds a registry to the reads of a local registry; a remote client reads from its " + "own registry" + ) + return [registry], None + source = open_registry(fetch_from) if isinstance(fetch_from, str) else fetch_from + sources: _Sources = [registry] + if source is not None: + sources.append(source) + if auto_build: + sources.append(InstalledConnectors()) + return sources, registry + def _reject_unhosted_access(self, dataset_id: str, version: str | None) -> None: """Check dataset access against the registry's requirements. @@ -328,7 +362,7 @@ def _reject_unhosted_access(self, dataset_id: str, version: str | None) -> None: manifest = self._registry.get_manifest(dataset_id, version) require_open_access(manifest.metadata, manifest.lineage) except TimeNetAccessError as error: - error.hint = f"TimeNet does not host its data, so build it locally with `timenet-build build {dataset_id}`." + error.hint = _unhosted_hint(dataset_id) raise def open_torch( @@ -349,9 +383,11 @@ def open_torch( Args: dataset_id: The dataset id. - version: The version string, or ``None`` for the latest. - auto_build: Build a missing local release from its connector. Set false to raise instead. - fetch_from: The registry, or its URI, to copy missing local releases from. + version: An exact version, ``"latest"``, or ``None``, as in :meth:`open`. + auto_build: Build a release that only an installed connector provides. Set false to + raise instead. + fetch_from: A registry, or its URI, that a local client reads a release from if its own + registry does not provide it. load: The plan that each batch loads with. A text-only plan, for example, skips the arrays of candidate and target Records. @@ -366,117 +402,27 @@ def open_torch( ) -def _require_open(manifest: Manifest) -> None: - """Reject a release without access declared ``open``. +def _unhosted_hint(dataset_id: str) -> str: + """Tell a remote client how to get a dataset whose data its registry does not serve. - This check runs before the client copies the files of the release from a registry that is not - local. + Returns: + The hint as a full sentence. """ - require_open_access(manifest.metadata, manifest.lineage) + return f"TimeNet does not host its data, so build it locally with `timenet-build build {dataset_id}`." -def _missing_hint(*, auto_build: bool, fetch_from: BaseRegistry | None) -> str | None: - """Name the keywords of :meth:`TimeNet.open` that can obtain a missing release. +def _missing_hint(*, auto_build: bool, fetch_from: str | BaseRegistry | None) -> str | None: + """Name the keywords of :meth:`TimeNet.open` that can get a missing release. Returns: The hint as a full sentence, or ``None`` if the call already used both keywords. """ ways = [] if fetch_from is None: - ways.append("copy it with fetch_from=") + ways.append("read it from a registry that holds it with fetch_from=") if not auto_build: ways.append("build it from its connector with auto_build=True") if not ways: return None hint = " or ".join(ways) return f"{hint[0].upper()}{hint[1:]}." - - -def _prepare( - registry: LocalRegistry, dataset_id: str, version: str | None, *, auto_build: bool, fetch_from: BaseRegistry | None -) -> DatasetVersion: - """Prepare a local dataset version and the releases that supply its payload files. - - The function copies missing releases from ``fetch_from`` or builds them when ``auto_build`` is set. - A payload source build must reproduce the exact version and pinned files. - - Returns: - The release with each payload source attached. - - Raises: - TimeNetMissingParentError: If a missing payload source cannot be copied or built. - TimeNetInvalidCardError: If a connector's card cannot be read. - """ # ruff: ignore[docstring-extraneous-exception] - raised by _obtain, claim and the build - try: - manifest = registry.get_manifest(dataset_id, version) - except TimeNetDatasetNotFoundError as miss: - manifest = _obtain(registry, dataset_id, version, miss, auto_build=auto_build, fetch_from=fetch_from) - reference = DatasetRef(dataset_id=dataset_id, version=manifest.metadata.dataset_version) - # A registry that is not local serves only releases with access declared ``open``. - check = _require_open if fetch_from is not None and fetch_from.requires_open_access else None - built: set[DatasetRef] = set() - while True: - try: - return prepare_local_copy( - registry.root, reference, manifest=manifest, fetch_from=fetch_from, before_fetch=check - ) - except TimeNetMissingParentError as missing: - source = missing.missing - 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 - pins = next(pinned for pinned in manifest.payload_sources.values() if pinned.release == source) - # TODO(adrian): Remove temporary glue which maps fetch_from and auto_build onto a list - # of sources until the client takes the list itself. - plan = plan_release_build( - str(source), - store=registry, - parents_from=[*([fetch_from] if fetch_from is not None else []), InstalledConnectors()], - pinned_by={reference: pins}, - ) - run_plan(plan, registry) - built.add(source) - - -def _obtain( # ruff: ignore[too-many-arguments] - registry: LocalRegistry, - dataset_id: str, - version: str | None, - miss: TimeNetDatasetNotFoundError, - *, - auto_build: bool, - fetch_from: BaseRegistry | None, -) -> Manifest: - """Get a missing version's manifest from ``fetch_from``, or build the version locally. - - A local build needs ``auto_build`` and an installed connector for the dataset. - - Returns: - The manifest from ``fetch_from`` without its files, or the local manifest after a build. - - Raises: - TimeNetDatasetNotFoundError: If neither ``fetch_from`` nor a connector provides the version. - TimeNetVersionUnavailableError: If the connector declares a different version. - TimeNetInvalidCardError: If the connector's card cannot be read. - """ # ruff: ignore[docstring-extraneous-exception] - raised by the build - if fetch_from is not None: - try: - return fetch_from.get_manifest(dataset_id, version) - except TimeNetDatasetNotFoundError: - pass - if not auto_build or installed_builder(dataset_id) 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 - # TODO(adrian): Remove temporary glue which maps fetch_from and auto_build onto a list of - # sources until the client takes the list itself. The build prepares the declared parents of - # the version in the same order: reuse, copy, then build. Sentinels such as "latest" are not - # pins, so a fresh build satisfies them. - plan = plan_release_build( - dataset_id if is_latest(version) else f"{dataset_id}@{version}", - store=registry, - parents_from=[*([fetch_from] if fetch_from is not None else []), InstalledConnectors()], - ) - run_plan(plan, registry) - return registry.get_manifest(dataset_id, version) diff --git a/packages/timenet/src/timenet/coordinator/build.py b/packages/timenet/src/timenet/coordinator/build.py index b62364dbc..65ed5e381 100644 --- a/packages/timenet/src/timenet/coordinator/build.py +++ b/packages/timenet/src/timenet/coordinator/build.py @@ -1,26 +1,24 @@ """Plan and run release builds with dependencies first.""" -from collections.abc import Mapping, Sequence +from collections.abc import Sequence from typing import assert_never from timenet.builders import BuildOptions, declared from timenet.coordinator.plan import BuildStep, CopyStep, Plan, ReadStep, plan_steps -from timenet.manifest import PayloadSource from timenet.progress import StepFinished, StepStarted from timenet.registry import copy_release from timenet.registry.dependencies import prepare_local_copy from timenet.registry.local import LocalRegistry from timenet.sources import ReleaseSource -from timenet.types.references import DatasetRef, is_latest, split_ref +from timenet.types.references import is_latest, split_ref -def plan_release_build( # ruff: ignore[too-many-arguments] +def plan_release_build( dataset_id: str, *, store: LocalRegistry, parents_from: Sequence[ReleaseSource] = (), force: bool = False, - pinned_by: Mapping[DatasetRef, PayloadSource] | None = None, open_access_only: bool = False, ) -> Plan: """Plan a release build in a local registry. @@ -33,7 +31,6 @@ def plan_release_build( # ruff: ignore[too-many-arguments] store: The local registry to write to. parents_from: Sources to copy or build missing dependencies from, in preference order. force: Plan a forced rebuild and its parents even if the requested release already exists. - pinned_by: File pins grouped by dependent release that the build must preserve. open_access_only: Require ``access: open`` for every needed release. Returns: @@ -52,7 +49,6 @@ def plan_release_build( # ruff: ignore[too-many-arguments] parents_from=parents_from, force_build=force, open_access_only=open_access_only, - pinned_by=pinned_by, ) return Plan(tuple(steps)) diff --git a/packages/timenet/src/timenet/coordinator/plan.py b/packages/timenet/src/timenet/coordinator/plan.py index e9f0d7bf7..219a5635f 100644 --- a/packages/timenet/src/timenet/coordinator/plan.py +++ b/packages/timenet/src/timenet/coordinator/plan.py @@ -566,14 +566,13 @@ def _check_agreement(release: DatasetRef, readers: Mapping[DatasetRef, PayloadSo ) -def plan_steps( # ruff: ignore[too-many-arguments] +def plan_steps( builder: InstalledBuilder, output: LocalRegistry, *, parents_from: Sequence[ReleaseSource], force_build: bool, open_access_only: bool, - pinned_by: Mapping[DatasetRef, PayloadSource] | None = None, ) -> tuple[DatasetRef, list[Step]]: """Plan a dataset build and all required releases. @@ -585,7 +584,6 @@ def plan_steps( # ruff: ignore[too-many-arguments] parents_from: The sources to search for missing releases, in order. force_build: Rebuild the requested release even if the output holds it. open_access_only: Require open access for every release. - pinned_by: The releases that pin files of the requested release, with their file requirements. Returns: The requested release and ordered steps, ending with its build if needed. @@ -596,8 +594,6 @@ def plan_steps( # ruff: ignore[too-many-arguments] root = builder.release # The graph leaves the output out of the sources of its copies and builds. graph = _Graph(output, parents_from, open_only=open_access_only) - if pinned_by: - graph.pins[root] = dict(pinned_by) # 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, damage = (None, None) if force_build else graph.in_output(root) diff --git a/packages/timenet/src/timenet/registry/dependencies.py b/packages/timenet/src/timenet/registry/dependencies.py index 280f0c275..3ac532374 100644 --- a/packages/timenet/src/timenet/registry/dependencies.py +++ b/packages/timenet/src/timenet/registry/dependencies.py @@ -272,7 +272,6 @@ def prepare_local_copy( # ruff: ignore[too-many-arguments] manifest: Manifest | None = None, force: bool = False, progress_cb: Callable[[int], None] | None = None, - before_fetch: Callable[[Manifest], object] | None = None, ) -> DatasetVersion: """Ensure complete local copies of a release and of each of its payload sources. @@ -294,9 +293,6 @@ def prepare_local_copy( # ruff: ignore[too-many-arguments] manifest: An already-loaded manifest for the requested release. force: Fetch the requested release again, even if it is cached. progress_cb: Report downloaded bytes. - before_fetch: A callback that gets the manifest of each release before this function - downloads the files of the release. For example, the callback can check the access - requirement of the release. Returns: The requested release with its payload sources attached and checked against its pins. @@ -319,7 +315,6 @@ def open_complete(ref: DatasetRef, *, requested: bool) -> DatasetVersion: manifest=manifest if requested else None, force=force and requested, progress_cb=progress_cb, - before_fetch=before_fetch, ) return DatasetVersion(release, pafs.LocalFileSystem(), str(directory)) @@ -362,7 +357,6 @@ def _ensure_dataset_version_available_locally( # ruff: ignore[too-many-argument manifest: Manifest | None, force: bool, progress_cb: Callable[[int], None] | None, - before_fetch: Callable[[Manifest], object] | None = None, ) -> Manifest: """Reuse or fetch one complete release, and repair a damaged copy only by fetching it. @@ -391,9 +385,7 @@ def _ensure_dataset_version_available_locally( # ruff: ignore[too-many-argument else: return local if fetch_from is not None: - return _download_dataset_version( - directory, reference, fetch_from, manifest=manifest, progress_cb=progress_cb, before_fetch=before_fetch - ) + return _download_dataset_version(directory, reference, fetch_from, manifest=manifest, progress_cb=progress_cb) raise TimeNetDatasetNotFoundError(f"complete release {reference} is missing at {directory}") @@ -417,14 +409,13 @@ def _load_complete_local_manifest( return manifest if complete_copy(directory, manifest) else None -def _download_dataset_version( # ruff: ignore[too-many-arguments] +def _download_dataset_version( directory: Path, reference: DatasetRef, registry: "BaseRegistry", *, manifest: Manifest | None, progress_cb: Callable[[int], None] | None, - before_fetch: Callable[[Manifest], object] | None = None, ) -> Manifest: """Check a release's identity and download its files. @@ -436,8 +427,6 @@ def _download_dataset_version( # ruff: ignore[too-many-arguments] if manifest is None: manifest = registry.get_manifest(reference.dataset_id, str(reference.version)) validate_manifest_reference(manifest, reference) - if before_fetch is not None: - before_fetch(manifest) registry.download_release(reference, directory, force=True, manifest=manifest, progress_cb=progress_cb) return manifest diff --git a/packages/timenet/tests/coordinator/test_build.py b/packages/timenet/tests/coordinator/test_build.py index 16e22d550..7f8e95ace 100644 --- a/packages/timenet/tests/coordinator/test_build.py +++ b/packages/timenet/tests/coordinator/test_build.py @@ -119,20 +119,6 @@ def _store_child_without_parent(registry): 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_a_build_passes_the_pins_of_the_caller_to_the_backend(store, fake_parent, pins): - plan = plan_release_build(PARENT_REF.dataset_id, store=store, pinned_by={CHILD_REF: pins}) - run_plan(plan, store) - assert fake_parent.calls == [(PARENT_REF, False, {CHILD_REF: pins})] - assert plan.steps == (_build(PARENT_REF, pinned_by={CHILD_REF: pins}),) - assert store.exists(PARENT_REF.dataset_id, str(PARENT_REF.version)) - - def test_plan_release_build_rejects_a_version_the_connector_does_not_declare(store, fake): with pytest.raises(TimeNetVersionUnavailableError) as caught: plan_release_build("a/b@2.0.0", store=store) diff --git a/packages/timenet/tests/registry/test_remote.py b/packages/timenet/tests/registry/test_remote.py index 7d8d6ef7c..1190d58ff 100644 --- a/packages/timenet/tests/registry/test_remote.py +++ b/packages/timenet/tests/registry/test_remote.py @@ -578,7 +578,7 @@ def _gated_release(origin: Path, page: str) -> tuple[str, DatasetRef]: @pytest.mark.parametrize("page", ["own", "ancestor"]) -def test_a_local_client_checks_access_before_it_copies_from_a_remote_registry(tmp_path, page): +def test_a_local_client_checks_access_before_it_reads_from_a_remote_registry(tmp_path, page): dataset_id, release = _gated_release(tmp_path / "origin", page) transport, requests = build_fake(next((tmp_path / "origin").rglob("manifest.json")).parent) remote = RemoteRegistry("http://api.local", transport=transport, cache_dir=tmp_path / "cache") @@ -587,6 +587,7 @@ def test_a_local_client_checks_access_before_it_copies_from_a_remote_registry(tm TimeNet(tmp_path / "local").open(dataset_id, auto_build=False, fetch_from=remote) assert caught.value.requirements == (AccessRequirement(release, Access.CREDENTIALED, "https://example.org/dua"),) + assert caught.value.hint == "Build it from its connector with auto_build=True." assert not any("/download/" in request.url.path or request.url.host == "blob.local" for request in requests) assert not (tmp_path / "local").exists() diff --git a/packages/timenet/tests/test_client.py b/packages/timenet/tests/test_client.py index 5c94a5523..e7cbabd6f 100644 --- a/packages/timenet/tests/test_client.py +++ b/packages/timenet/tests/test_client.py @@ -2,11 +2,12 @@ from types import SimpleNamespace from _demo_child import _child_of_the_demo +from _fake_registry import build_service_fake import httpx from pydantic import ValidationError import pytest -from timenet import builders as builders_module, client as client_module +from timenet import builders as builders_module from timenet.client import TimeNet from timenet.connectors import BaseConnector from timenet.dataset import Record, RegularAxis, Signal, Source, TimeFDataset @@ -62,24 +63,7 @@ def build(self, release, store, *, force, pinned_by, options): store.put_dataset(dataset, force=force, values_backend=self.BACKEND, pinned_by=pinned_by or None) -def _record_build_requests(monkeypatch): - """Record the dataset and the ``parents_from`` list of each build that the client asks for. - - Returns: - The list that receives one ``(dataset, parents_from)`` entry for each request. - """ - requests = [] - plan_release_build = client_module.plan_release_build - - def record(dataset_id, **options): - requests.append((dataset_id, options.get("parents_from"))) - return plan_release_build(dataset_id, **options) - - monkeypatch.setattr(client_module, "plan_release_build", record) - return requests - - -def test_a_local_child_copies_its_missing_payload_source_from_fetch_from(tmp_path): +def test_a_local_child_reads_its_missing_payload_source_through_fetch_from(tmp_path): origin, reference = _child_of_the_demo(tmp_path / "origin") cache = tmp_path / "cache" shutil.copytree(origin.root / "test/child/1.0.0", cache / "test/child/1.0.0") @@ -88,35 +72,30 @@ def test_a_local_child_copies_its_missing_payload_source_from_fetch_from(tmp_pat client.open("test/child", auto_build=False) assert (caught.value.missing, caught.value.needed_by) == (reference, DatasetRef.parse("test/child@1.0.0")) assert str(caught.value).endswith( - ". Copy it with fetch_from= or build it from its connector with auto_build=True." + ". Read it from a registry that holds it with fetch_from= or build it from its connector with " + "auto_build=True." ) with client.open("test/child", fetch_from=str(origin.root)) as reader: - assert (cache / reference.dataset_id / "1.0.0/control.duckdb").is_file() - shutil.rmtree(origin.root) assert reader.read().records[0].signals[0].to_arrow().equals(make_dataset().records[0].signals[0].to_arrow()) - with client.open("test/child") as reader: - assert reader._control is None - assert reader.read().records + # The read opens the payload source in origin, where it is, and copies nothing. + assert not (cache / reference.dataset_id).exists() @pytest.mark.parametrize("in_fetch_from", [True, False]) -def test_a_missing_version_is_copied_from_fetch_from_before_it_is_built(tmp_path, monkeypatch, in_fetch_from): +def test_a_missing_version_is_read_from_fetch_from_before_it_is_built(tmp_path, monkeypatch, in_fetch_from): origin = LocalRegistry(tmp_path / "origin") if in_fetch_from: _child_of_the_demo(origin.root) calls = [] install_backends(monkeypatch, _Builder(calls)) - requests = _record_build_requests(monkeypatch) with TimeNet(tmp_path / "local").open("test/child", fetch_from=origin) as reader: assert reader.read().records if in_fetch_from: - assert calls == requests == [] - assert (tmp_path / "local/test/child/1.0.0/manifest.json").is_file() + assert calls == [] + assert not (tmp_path / "local").exists() return - # The build gets the same registry to copy its own missing parents from. - assert requests == [("test/child", [origin, InstalledConnectors()])] assert calls == [(DatasetRef.parse("test/child@1.0.0"), False, {})] @@ -183,16 +162,16 @@ def test_a_missing_payload_source_builds_with_its_own_connector_against_the_pins 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") install_backends(monkeypatch, _Builder(calls)) - requests = _record_build_requests(monkeypatch) with pytest.raises(TimeNetMissingParentError, match=r"auto_build=True\.$"): TimeNet(registry).open("test/child", auto_build=False) assert calls == [] if case == "other-version": # No installed connector declares the exact version of the source that the child pins. - with pytest.raises(TimeNetMissingParentError, match=r"\. Copy it with fetch_from=\.$"): - TimeNet(registry).open("test/child") - assert calls == requests == [] + hint = r"\. Read it from a registry that holds it with fetch_from=\.$" + with pytest.raises(TimeNetMissingParentError, match=hint): + TimeNet(registry).open("test/child@1.0.0") + assert calls == [] return if case == "differs": pytest.importorskip("zarr") @@ -200,23 +179,17 @@ def test_a_missing_payload_source_builds_with_its_own_connector_against_the_pins with pytest.raises(TimeFFormatError, match=r"Nothing was published"): TimeNet(registry).open("test/child") assert not registry.exists(reference.dataset_id, str(reference.version)) - fetch_from = None else: - # A registry to copy from that lacks the source: the build gets it for the parents of the source. - fetch_from = LocalRegistry(tmp_path / "empty") - with TimeNet(registry).open("test/child", fetch_from=fetch_from) as reader: + # A registry to read from that lacks the source, so the source falls through to its connector. + with TimeNet(registry).open("test/child", fetch_from=LocalRegistry(tmp_path / "empty")) as reader: signal = reader.read().records[0].signals[0] assert signal.to_arrow().equals(make_dataset().records[0].signals[0].to_arrow()) - # The source builds with its own connector, which prepares its parents in the same order, against - # the pins of the child that reads it. - assert requests == [ - (str(reference), [InstalledConnectors()] if fetch_from is None else [fetch_from, InstalledConnectors()]) - ] + # The source builds with its own connector, against the pins of the child that reads it. assert calls == [(reference, False, {DatasetRef.parse("test/child@1.0.0"): pins})] @pytest.mark.parametrize("damaged", ["requested", "payload-source"]) -def test_a_damaged_release_is_repaired_from_fetch_from_and_never_built_over(tmp_path, monkeypatch, damaged): +def test_a_damaged_release_raises_and_is_never_built_over(tmp_path, monkeypatch, damaged): registry, reference = _child_of_the_demo(tmp_path / "registry") origin = tmp_path / "origin" shutil.copytree(registry.root, origin) @@ -227,42 +200,33 @@ def test_a_damaged_release_is_repaired_from_fetch_from_and_never_built_over(tmp_ calls = [] install_backends(monkeypatch, _Builder(calls)) - with pytest.raises(TimeNetFileError) as caught: - TimeNet(registry).open("test/child") - assert str(caught.value) == ( - f"{release}: file 'control.duckdb' has 9 bytes, expected {size:,} bytes. Repair it with fetch_from=." - ) - with TimeNet(registry).open("test/child", fetch_from=str(origin)) as reader: - assert reader.read().records - assert control.stat().st_size == size + # The intact copy in fetch_from does not help: the read stops at the damaged copy, which comes first. + for fetch_from in (None, str(origin)): + with pytest.raises(TimeNetFileError) as caught: + TimeNet(registry).open("test/child", fetch_from=fetch_from) + assert str(caught.value) == f"{release}: file 'control.duckdb' has 9 bytes, expected {size:,} bytes." assert calls == [] -def test_a_corrupted_copy_from_fetch_from_names_the_transfer_without_the_repair_hint(tmp_path): - origin, _ = _child_of_the_demo(tmp_path / "origin") - control = origin.root / "test/child/1.0.0/control.duckdb" - data = control.read_bytes() - control.write_bytes(bytes([data[0] ^ 1]) + data[1:]) - - with pytest.raises(TimeNetFileError) as caught: - TimeNet(tmp_path / "local").open("test/child", auto_build=False, fetch_from=origin) - - # The registry to copy from served the file, so a hint to copy from a registry would not help. - assert caught.value.owner == f"download of test/child@1.0.0 from {origin}" - assert caught.value.hint is None - - def test_a_remote_client_takes_no_fetch_from_and_never_builds(tmp_path, monkeypatch): calls = [] install_backends(monkeypatch, _Builder(calls)) - remote = RemoteRegistry( - "http://api.local", transport=httpx.MockTransport(lambda request: httpx.Response(404)), cache_dir=tmp_path - ) + # The remote registry holds the child, but not its payload source, which the installed backend builds. + origin, reference = _child_of_the_demo(tmp_path / "origin") + shutil.rmtree(origin.root / reference.dataset_id) + transport, _ = build_service_fake(origin.root) + remote = RemoteRegistry("http://api.local", transport=transport, cache_dir=tmp_path / "cache") - with pytest.raises(TimeFValidationError, match="fetch_from copies releases into a local registry"): + message = "fetch_from adds a registry to the reads of a local registry; a remote client reads from its own registry" + with pytest.raises(TimeFValidationError, match=message): TimeNet(remote).open("test/child", fetch_from=str(tmp_path)) - with pytest.raises(TimeNetDatasetNotFoundError): + # Neither fetch_from nor auto_build helps a remote client, so the messages name no keyword. + with pytest.raises(TimeNetMissingParentError) as missing: TimeNet(remote).open("test/child") + assert str(missing.value) == f"payload source {reference} of test/child@1.0.0 is not in registry http://api.local." + with pytest.raises(TimeNetDatasetNotFoundError) as caught: + TimeNet(remote).open(reference.dataset_id) + assert str(caught.value) == f"{reference.dataset_id} is in none of the sources: registry http://api.local." assert calls == [] @@ -554,16 +518,16 @@ def test_a_missing_version_that_no_connector_builds_names_the_keyword_that_can_g ): monkeypatch.setattr(builders_module, "entry_points", lambda group: []) client = TimeNet(tmp_path / "reg", storage_path=tmp_path / "store") - with pytest.raises(TimeNetDatasetNotFoundError) as miss: - LocalRegistry(tmp_path / "reg").get_manifest("x/y") source = None if fetch_from is None else LocalRegistry(tmp_path / fetch_from) with pytest.raises(TimeNetDatasetNotFoundError) as caught: client.open("x/y", fetch_from=source) + sources = [LocalRegistry(tmp_path / "reg"), *([] if source is None else [source]), InstalledConnectors()] + message = f"x/y is in none of the sources: {', '.join(str(entry) for entry in sources)}." # The hint names only the keyword that the call did not use. - expected = f"{miss.value}. Copy it with fetch_from=." if source is None else str(miss.value) - assert str(caught.value) == expected + hint = " Read it from a registry that holds it with fetch_from=." if source is None else "" + assert str(caught.value) == message + hint @pytest.mark.parametrize("pin", [None, "latest", ""]) @@ -591,12 +555,33 @@ def test_load_does_not_build_when_auto_build_is_false(registry_root, tmp_path, m 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. - hint = r"\. Copy it with fetch_from= or build it from its connector with auto_build=True\.$" + hint = ( + r"\. Read it from a registry that holds it with fetch_from= or build it from its connector with " + r"auto_build=True\.$" + ) with pytest.raises(TimeNetDatasetNotFoundError, match=hint): getattr(client, entry_point)("timenet/hello-world@9.9.9", auto_build=False) assert calls == [] # auto_build=False skips the connector entirely +def test_latest_builds_a_newer_version_that_the_connector_declares(registry_root, tmp_path, monkeypatch): + calls = [] + monkeypatch.setattr(_Builder, "VERSION", "2.0.0") + install_backends(monkeypatch, _Builder(calls)) + with TimeNet(registry_root).open("timenet/hello-world@latest") as reader: + assert str(reader.metadata.dataset_version) == "2.0.0" + assert calls == [(DatasetRef.parse("timenet/hello-world@2.0.0"), False, {})] + + +def test_a_read_never_builds_without_auto_build_even_for_a_newer_declared_version(registry_root, tmp_path, monkeypatch): + calls = [] + monkeypatch.setattr(_Builder, "VERSION", "2.0.0") + install_backends(monkeypatch, _Builder(calls)) + with TimeNet(registry_root).open("timenet/hello-world@latest", auto_build=False) as reader: + assert str(reader.metadata.dataset_version) == "1.0.0" + assert calls == [] + + def _card(dataset_id, access, access_url=None): return DatasetMetadata( dataset_id=dataset_id, @@ -612,13 +597,15 @@ def _card(dataset_id, access, access_url=None): def _hosted(client, metadata, lineage=()): """Serve one manifest from a registry that is not local, so the build-your-own gate applies.""" - class _Hosted: - requires_open_access = True - + class _Hosted(RemoteRegistry): def get_manifest(self, dataset_id, version=None): return SimpleNamespace(metadata=metadata, lineage=lineage) - client._registry = _Hosted() + client._registry = _Hosted( + "http://api.local", + transport=httpx.MockTransport(lambda request: httpx.Response(404)), + cache_dir=client._storage, + ) def test_load_rejects_a_credentialed_dataset_from_a_hosted_registry(tmp_path): From 3baaf74b88a2ad7994f69cd2e8b29f79f992fe92 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 17:16:05 +0100 Subject: [PATCH 2/6] refactor(client): mark the read hints of fetch_from and auto_build as glue The comment on the read hints and the docstring of _read_sources now start with "Glue:". They say that this code stays until the client takes a list of sources. Part of TNET-157. --- packages/timenet/src/timenet/client.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/timenet/src/timenet/client.py b/packages/timenet/src/timenet/client.py index 726748ec8..639e543bd 100644 --- a/packages/timenet/src/timenet/client.py +++ b/packages/timenet/src/timenet/client.py @@ -252,7 +252,8 @@ def open( """ # ruff: ignore[docstring-extraneous-exception] (raised by read_release and the builder) dataset_id, version = _resolve_ref(dataset_id, version) sources, store = self._read_sources(auto_build=auto_build, fetch_from=fetch_from) - # Maps fetch_from and auto_build onto hints for a local client. + # Glue: maps fetch_from and auto_build onto hints for a local client, until the client + # takes a list of sources. try: return TimeFReader(read_release(dataset_id, version, sources=sources, store=store)) except TimeNetAccessError as error: @@ -319,10 +320,11 @@ def _read_sources( ) -> tuple[_Sources, LocalRegistry | None]: """Return the sources of a read, in order, and the local registry that its builds go to. - Maps ``fetch_from`` and ``auto_build`` onto a list of sources for the read coordinator. A - local client reads from its own registry, then from ``fetch_from``, then from the installed - connectors if ``auto_build`` is set. Its builds go to its own registry. A remote client - reads only from its own registry, and it never builds. + Glue: maps ``fetch_from`` and ``auto_build`` onto a list of sources for the read + coordinator, until the client takes the list itself. A local client reads from its own + registry, then from ``fetch_from``, then from the installed connectors if ``auto_build`` is + set. Its builds go to its own registry. A remote client reads only from its own registry, + and it never builds. Returns: The sources, and the local registry, or ``None`` for a remote client. From 11ec407c8e4f4f96309a1278d1696ef303113c9e Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Thu, 5 Nov 2026 11:41:12 +0100 Subject: [PATCH 3/6] refactor: mark the glue of the client reads as a TODO The glue comment on the read hints and the glue note in the docstring of _read_sources() now start with TODO(adrian), so a search finds each one that is left. Part of TNET-157. --- packages/timenet/src/timenet/client.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/timenet/src/timenet/client.py b/packages/timenet/src/timenet/client.py index 639e543bd..b7d7e4c38 100644 --- a/packages/timenet/src/timenet/client.py +++ b/packages/timenet/src/timenet/client.py @@ -252,8 +252,8 @@ def open( """ # ruff: ignore[docstring-extraneous-exception] (raised by read_release and the builder) dataset_id, version = _resolve_ref(dataset_id, version) sources, store = self._read_sources(auto_build=auto_build, fetch_from=fetch_from) - # Glue: maps fetch_from and auto_build onto hints for a local client, until the client - # takes a list of sources. + # TODO(adrian): Remove temporary glue which maps fetch_from and auto_build onto hints for + # a local client until the client takes a list of sources. try: return TimeFReader(read_release(dataset_id, version, sources=sources, store=store)) except TimeNetAccessError as error: @@ -320,11 +320,11 @@ def _read_sources( ) -> tuple[_Sources, LocalRegistry | None]: """Return the sources of a read, in order, and the local registry that its builds go to. - Glue: maps ``fetch_from`` and ``auto_build`` onto a list of sources for the read - coordinator, until the client takes the list itself. A local client reads from its own - registry, then from ``fetch_from``, then from the installed connectors if ``auto_build`` is - set. Its builds go to its own registry. A remote client reads only from its own registry, - and it never builds. + TODO(adrian): Remove temporary glue which maps ``fetch_from`` and ``auto_build`` onto a + list of sources for the read coordinator until the client takes the list itself. A local + client reads from its own registry, then from ``fetch_from``, then from the installed + connectors if ``auto_build`` is set. Its builds go to its own registry. A remote client + reads only from its own registry, and it never builds. Returns: The sources, and the local registry, or ``None`` for a remote client. From f0d20c4394c0890feae71fa083555f9d4c23d320 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 19:06:30 +0100 Subject: [PATCH 4/6] docs: shorten the docstrings and docs of the client reads Say only what a caller of open(), load() and open_torch() needs to know. Part of TNET-157. --- .../src/timenet_connectors/api.py | 3 +-- packages/timenet/src/timenet/client.py | 13 ++++--------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/api.py b/packages/timenet-connectors/src/timenet_connectors/api.py index 25c5d7188..769433a43 100644 --- a/packages/timenet-connectors/src/timenet_connectors/api.py +++ b/packages/timenet-connectors/src/timenet_connectors/api.py @@ -112,8 +112,7 @@ def load( Args: dataset_id: The dataset ID (``org/name``). version: The version to load, which the card must declare, or ``None`` for the latest. - fetch_from: The registry, or its URI, that the build copies missing parent releases - from, and that the read reads missing payload sources from, where they are. + fetch_from: The registry, or its URI, that provides missing parents and payload sources. build_parents: Build missing parent releases from their installed connectors. Returns: diff --git a/packages/timenet/src/timenet/client.py b/packages/timenet/src/timenet/client.py index b7d7e4c38..da34c9853 100644 --- a/packages/timenet/src/timenet/client.py +++ b/packages/timenet/src/timenet/client.py @@ -1,11 +1,8 @@ """The :class:`TimeNet` class is the SDK's single entry point for using TimeNet from code. This class wraps a :class:`~timenet.registry.BaseRegistry` (the catalog) and a local storage path -(the download cache). It exposes these methods: ``list``, ``get``, ``search``, ``download``, -``open``, ``load``, and ``open_torch``. ``open`` and ``load`` read a release where it is, through -the read coordinator in :mod:`timenet.coordinator.read`. Against a local registry, they build a -release that only an installed connector provides. Against a remote registry, they never run -connector code. +(the download cache). ``open`` and ``load`` read a release where it is, and a local client builds a +release that only an installed connector provides. """ # The public API has a method named ``list``. Deferred annotations keep the type hint @@ -321,10 +318,8 @@ def _read_sources( """Return the sources of a read, in order, and the local registry that its builds go to. TODO(adrian): Remove temporary glue which maps ``fetch_from`` and ``auto_build`` onto a - list of sources for the read coordinator until the client takes the list itself. A local - client reads from its own registry, then from ``fetch_from``, then from the installed - connectors if ``auto_build`` is set. Its builds go to its own registry. A remote client - reads only from its own registry, and it never builds. + list of sources until the client takes the list itself. A remote client gets only its own + registry, and no local registry. Returns: The sources, and the local registry, or ``None`` for a remote client. From 14b57772b66113c8d0f03b377e91424e7c367d13 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Thu, 5 Nov 2026 00:22:52 +0100 Subject: [PATCH 5/6] docs: shorten the docstrings of the client reads Shorter docstrings for the code that this change adds or changes. Part of TNET-157. --- .../src/timenet_connectors/api.py | 4 +-- packages/timenet/src/timenet/client.py | 31 +++++++------------ .../src/timenet/registry/dependencies.py | 16 +++++----- 3 files changed, 22 insertions(+), 29 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/api.py b/packages/timenet-connectors/src/timenet_connectors/api.py index 769433a43..f57b12dcf 100644 --- a/packages/timenet-connectors/src/timenet_connectors/api.py +++ b/packages/timenet-connectors/src/timenet_connectors/api.py @@ -112,8 +112,8 @@ def load( Args: dataset_id: The dataset ID (``org/name``). version: The version to load, which the card must declare, or ``None`` for the latest. - fetch_from: The registry, or its URI, that provides missing parents and payload sources. - build_parents: Build missing parent releases from their installed connectors. + fetch_from: The registry or URI that supplies missing parents and payload sources. + build_parents: Build missing parent releases with installed connectors. Returns: The loaded dataset. diff --git a/packages/timenet/src/timenet/client.py b/packages/timenet/src/timenet/client.py index da34c9853..bef66e354 100644 --- a/packages/timenet/src/timenet/client.py +++ b/packages/timenet/src/timenet/client.py @@ -1,9 +1,4 @@ -"""The :class:`TimeNet` class is the SDK's single entry point for using TimeNet from code. - -This class wraps a :class:`~timenet.registry.BaseRegistry` (the catalog) and a local storage path -(the download cache). ``open`` and ``load`` read a release where it is, and a local client builds a -release that only an installed connector provides. -""" +"""Browse, download, and read datasets with :class:`TimeNet`.""" # The public API has a method named ``list``. Deferred annotations keep the type hint # ``list[str]`` resolved to the builtin type, not to the method. @@ -315,17 +310,15 @@ def load( def _read_sources( self, *, auto_build: bool, fetch_from: str | BaseRegistry | None ) -> tuple[_Sources, LocalRegistry | None]: - """Return the sources of a read, in order, and the local registry that its builds go to. + """Return the ordered sources for a read and the local registry for builds. - TODO(adrian): Remove temporary glue which maps ``fetch_from`` and ``auto_build`` onto a - list of sources until the client takes the list itself. A remote client gets only its own - registry, and no local registry. + Remote reads use only the configured registry. Returns: - The sources, and the local registry, or ``None`` for a remote client. + The source list, and the local registry for builds or ``None`` for a remote client. Raises: - TimeFValidationError: If a remote client gets ``fetch_from``. + TimeFValidationError: If ``fetch_from`` is set for a remote registry. """ registry = self._registry if not isinstance(registry, LocalRegistry): @@ -344,14 +337,14 @@ def _read_sources( return sources, registry def _reject_unhosted_access(self, dataset_id: str, version: str | None) -> None: - """Check dataset access against the registry's requirements. + """Enforce the registry's open access requirement. Args: dataset_id: The dataset ID without a version suffix. - version: The version string, or ``None`` for the latest. + version: The resolved version, or ``None`` for the latest. Raises: - TimeNetAccessError: If the dataset or its lineage lacks open access and the registry requires it. + TimeNetAccessError: If the dataset lacks open access and the registry requires it. """ if not self._registry.requires_open_access: return @@ -400,19 +393,19 @@ def open_torch( def _unhosted_hint(dataset_id: str) -> str: - """Tell a remote client how to get a dataset whose data its registry does not serve. + """Return instructions for building a dataset locally. Returns: - The hint as a full sentence. + A sentence with the dataset's build command. """ return f"TimeNet does not host its data, so build it locally with `timenet-build build {dataset_id}`." def _missing_hint(*, auto_build: bool, fetch_from: str | BaseRegistry | None) -> str | None: - """Name the keywords of :meth:`TimeNet.open` that can get a missing release. + """Suggest options for reading or building a missing release. Returns: - The hint as a full sentence, or ``None`` if the call already used both keywords. + A sentence with unused options, or ``None`` if both options are already enabled. """ ways = [] if fetch_from is None: diff --git a/packages/timenet/src/timenet/registry/dependencies.py b/packages/timenet/src/timenet/registry/dependencies.py index 3ac532374..42a7fd599 100644 --- a/packages/timenet/src/timenet/registry/dependencies.py +++ b/packages/timenet/src/timenet/registry/dependencies.py @@ -358,14 +358,16 @@ def _ensure_dataset_version_available_locally( # ruff: ignore[too-many-argument force: bool, progress_cb: Callable[[int], None] | None, ) -> Manifest: - """Reuse or fetch one complete release, and repair a damaged copy only by fetching it. + """Reuse a complete local release or fetch it from a registry. + + Local checks compare file sizes. Downloads also check file hashes. Returns: - The release manifest. File hashes are checked during transfer, not on every read. + The complete release's manifest. Raises: - TimeNetDatasetNotFoundError: If the release is absent and this function cannot fetch it. - TimeNetFileError: If a local file is damaged and ``fetch_from`` is not given, or a fetched file does not match. + TimeNetDatasetNotFoundError: If the release is missing and ``fetch_from`` is unset or lacks it. + TimeNetFileError: If a local file is damaged and ``fetch_from`` is unset, or a downloaded file is damaged. TimeFFormatError: If a manifest identifies a different release. """ # ruff: ignore[docstring-extraneous-exception] - raised by the helpers if manifest is not None: @@ -417,12 +419,10 @@ def _download_dataset_version( manifest: Manifest | None, progress_cb: Callable[[int], None] | None, ) -> Manifest: - """Check a release's identity and download its files. - - Run ``before_fetch`` after checking the manifest and before downloading files. + """Download a complete release after checking its dataset ID and version. Returns: - The manifest of the downloaded and verified release. + The manifest for the downloaded and verified release. """ if manifest is None: manifest = registry.get_manifest(reference.dataset_id, str(reference.version)) From a2fc3c42a91044e450d40a956be170bc5142fe39 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Thu, 5 Nov 2026 10:28:12 +0100 Subject: [PATCH 6/6] test: trim the tests of the client reads Drop tests that cannot fail or repeat other tests, and sharpen the rest. Part of TNET-157. --- packages/timenet/tests/test_client.py | 93 ++++----------------------- 1 file changed, 11 insertions(+), 82 deletions(-) diff --git a/packages/timenet/tests/test_client.py b/packages/timenet/tests/test_client.py index e7cbabd6f..955e02556 100644 --- a/packages/timenet/tests/test_client.py +++ b/packages/timenet/tests/test_client.py @@ -13,11 +13,9 @@ from timenet.dataset import Record, RegularAxis, Signal, Source, TimeFDataset from timenet.engine import open_parents, run_pipeline from timenet.errors import ( - TimeFFormatError, TimeFValidationError, TimeNetAccessError, TimeNetDatasetNotFoundError, - TimeNetFileError, TimeNetMissingParentError, TimeNetVersionUnavailableError, ) @@ -31,7 +29,7 @@ from timenet.types import Access, DatasetMetadata, Domain, License, Version from timenet.types.access import AccessRequirement from timenet.types.parent_dataset_card import ParentDatasetCard -from timenet.types.reference_keys import ParentAlias, SourceKey +from timenet.types.reference_keys import ParentAlias from timenet.types.references import DatasetRef from timenet.writer import TimeFWriter @@ -63,7 +61,8 @@ def build(self, release, store, *, force, pinned_by, options): store.put_dataset(dataset, force=force, values_backend=self.BACKEND, pinned_by=pinned_by or None) -def test_a_local_child_reads_its_missing_payload_source_through_fetch_from(tmp_path): +def test_a_local_child_reads_its_missing_payload_source_through_fetch_from(tmp_path, monkeypatch): + monkeypatch.setattr(builders_module, "entry_points", lambda group: []) origin, reference = _child_of_the_demo(tmp_path / "origin") cache = tmp_path / "cache" shutil.copytree(origin.root / "test/child/1.0.0", cache / "test/child/1.0.0") @@ -75,6 +74,11 @@ def test_a_local_child_reads_its_missing_payload_source_through_fetch_from(tmp_p ". Read it from a registry that holds it with fetch_from= or build it from its connector with " "auto_build=True." ) + # No installed connector builds the payload source, so with auto_build the hint names only fetch_from. + with pytest.raises( + TimeNetMissingParentError, match=r"\. Read it from a registry that holds it with fetch_from=\.$" + ): + client.open("test/child") with client.open("test/child", fetch_from=str(origin.root)) as reader: assert reader.read().records[0].signals[0].to_arrow().equals(make_dataset().records[0].signals[0].to_arrow()) # The read opens the payload source in origin, where it is, and copies nothing. @@ -153,61 +157,6 @@ def test_resampled_child_reads_and_downloads_without_its_build_only_parent(tmp_p assert reader.read().records[0].signals[0].to_arrow().equals(expected) -@pytest.mark.parametrize("case", ["exact", "differs", "other-version"]) -def test_a_missing_payload_source_builds_with_its_own_connector_against_the_pins(tmp_path, monkeypatch, case): - registry, reference = _child_of_the_demo(tmp_path / "registry") - pins = registry.get_manifest("test/child").payload_sources[SourceKey(str(reference))] - shutil.rmtree(registry.root / reference.dataset_id) - 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") - install_backends(monkeypatch, _Builder(calls)) - - with pytest.raises(TimeNetMissingParentError, match=r"auto_build=True\.$"): - TimeNet(registry).open("test/child", auto_build=False) - assert calls == [] - if case == "other-version": - # No installed connector declares the exact version of the source that the child pins. - hint = r"\. Read it from a registry that holds it with fetch_from=\.$" - with pytest.raises(TimeNetMissingParentError, match=hint): - TimeNet(registry).open("test/child@1.0.0") - assert calls == [] - return - if case == "differs": - pytest.importorskip("zarr") - # A Zarr build writes other files than the Parquet files that the child pins. - with pytest.raises(TimeFFormatError, match=r"Nothing was published"): - TimeNet(registry).open("test/child") - assert not registry.exists(reference.dataset_id, str(reference.version)) - else: - # A registry to read from that lacks the source, so the source falls through to its connector. - with TimeNet(registry).open("test/child", fetch_from=LocalRegistry(tmp_path / "empty")) as reader: - signal = reader.read().records[0].signals[0] - assert signal.to_arrow().equals(make_dataset().records[0].signals[0].to_arrow()) - # The source builds with its own connector, against the pins of the child that reads it. - assert calls == [(reference, False, {DatasetRef.parse("test/child@1.0.0"): pins})] - - -@pytest.mark.parametrize("damaged", ["requested", "payload-source"]) -def test_a_damaged_release_raises_and_is_never_built_over(tmp_path, monkeypatch, damaged): - registry, reference = _child_of_the_demo(tmp_path / "registry") - origin = tmp_path / "origin" - shutil.copytree(registry.root, origin) - release = DatasetRef.parse("test/child@1.0.0") if damaged == "requested" else reference - control = registry.root / release.dataset_id / str(release.version) / "control.duckdb" - size = control.stat().st_size - control.write_bytes(b"cut short") - calls = [] - install_backends(monkeypatch, _Builder(calls)) - - # The intact copy in fetch_from does not help: the read stops at the damaged copy, which comes first. - for fetch_from in (None, str(origin)): - with pytest.raises(TimeNetFileError) as caught: - TimeNet(registry).open("test/child", fetch_from=fetch_from) - assert str(caught.value) == f"{release}: file 'control.duckdb' has 9 bytes, expected {size:,} bytes." - assert calls == [] - - def test_a_remote_client_takes_no_fetch_from_and_never_builds(tmp_path, monkeypatch): calls = [] install_backends(monkeypatch, _Builder(calls)) @@ -530,16 +479,14 @@ def test_a_missing_version_that_no_connector_builds_names_the_keyword_that_can_g assert str(caught.value) == message + hint -@pytest.mark.parametrize("pin", [None, "latest", ""]) -def test_load_accepts_the_latest_sentinels_after_a_build(tmp_path, monkeypatch, pin): - # Every registry reads "latest" and "" as the latest version, so they are not pins the build - # can miss: the version the builder just committed satisfies them. +def test_load_accepts_the_latest_sentinel_after_a_build(tmp_path, monkeypatch): + # "latest" is not a pin the build can miss: the version the builder just committed satisfies it. empty_root = tmp_path / "reg" empty_root.mkdir() install_backends(monkeypatch, _Builder([])) client = TimeNet(empty_root, storage_path=tmp_path / "store") - assert str(client.load("timenet/hello-world", pin).metadata.dataset_version) == "1.0.0" + assert str(client.load("timenet/hello-world", "latest").metadata.dataset_version) == "1.0.0" @pytest.mark.parametrize("entry_point", ["open", "load", "open_torch"]) @@ -564,24 +511,6 @@ def test_load_does_not_build_when_auto_build_is_false(registry_root, tmp_path, m assert calls == [] # auto_build=False skips the connector entirely -def test_latest_builds_a_newer_version_that_the_connector_declares(registry_root, tmp_path, monkeypatch): - calls = [] - monkeypatch.setattr(_Builder, "VERSION", "2.0.0") - install_backends(monkeypatch, _Builder(calls)) - with TimeNet(registry_root).open("timenet/hello-world@latest") as reader: - assert str(reader.metadata.dataset_version) == "2.0.0" - assert calls == [(DatasetRef.parse("timenet/hello-world@2.0.0"), False, {})] - - -def test_a_read_never_builds_without_auto_build_even_for_a_newer_declared_version(registry_root, tmp_path, monkeypatch): - calls = [] - monkeypatch.setattr(_Builder, "VERSION", "2.0.0") - install_backends(monkeypatch, _Builder(calls)) - with TimeNet(registry_root).open("timenet/hello-world@latest", auto_build=False) as reader: - assert str(reader.metadata.dataset_version) == "1.0.0" - assert calls == [] - - def _card(dataset_id, access, access_url=None): return DatasetMetadata( dataset_id=dataset_id,