From 5353e6270dba013413c1a9ee7986a527c2219857 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 00:39:33 +0100 Subject: [PATCH 1/5] feat(build)!: move the build CLI and build() onto the coordinator The build command of timenet-build and timenet_connectors.build() now build through build_release(). They no longer plan and build the parents through build_one(). A local --out gets the release in place. A remote --out gets the release in two steps. First, the release builds into the default local registry with open_access_only set. So a release that does not declare access open fails before any download. Then copy_release() publishes the release and its payload sources. If the remote registry already holds the version, nothing builds. --force is then a usage error, before anything runs, because a published version never changes. The parents now land in the local registry, so --fetch-from works with a remote --out too. If nothing runs, the command prints one status line on stderr that says so, unless --quiet is set. The progress renderer of the CLI shows the copy steps as well as the build steps. It is now the only place that chooses between no output, text lines and live bars. With isolation on, the parent shows only the copy steps, because the child shows its own build lines. --isolation and --no-isolation set the new BuildOptions.isolation, which the backend reads before TIMENET_ISOLATION. The environment of the process stays as it is. timenet_connectors.build() accepts only the dataset ID that a card declares. It and the build command take the release that the card declares from builders.declared(). build_release() checks the requested version against the card. A copy step of build_release() takes a parent with each payload source at every depth, through copy_release(). build_one() copied only the direct payload sources. So a copy now fails if the registry to copy from holds a payload source without its own payload sources. ConnectorBuilder.build() still calls build_one(), because the client still uses it. So build_one() and the hidden --pinned-by option of build stay. build_one() loses its narrate option, because nothing sets it now. BREAKING CHANGE: timenet_connectors.build() raises TimeNetDatasetNotFoundError, not LookupError, for a dataset ID that no card declares, such as the module spelling timenet/hello_world. It raises TimeNetVersionUnavailableError, a TimeFValidationError, for another version. --force with a remote --out of timenet-build build that already holds the version is a usage error. Part of TNET-157. --- .../src/timenet_connectors/api.py | 83 +++---- .../src/timenet_connectors/builder/backend.py | 4 +- .../src/timenet_connectors/builder/cli.py | 225 ++++++++++-------- .../tests/test_build_cli.py | 170 +++++++------ .../tests/test_build_plan.py | 151 ++++++++++-- .../tests/test_build_progress.py | 20 -- .../tests/test_package_api.py | 60 ++--- packages/timenet/src/timenet/builders.py | 4 +- 8 files changed, 423 insertions(+), 294 deletions(-) delete mode 100644 packages/timenet-connectors/tests/test_build_progress.py diff --git a/packages/timenet-connectors/src/timenet_connectors/api.py b/packages/timenet-connectors/src/timenet_connectors/api.py index 1c8a85ea6..4c329b84a 100644 --- a/packages/timenet-connectors/src/timenet_connectors/api.py +++ b/packages/timenet-connectors/src/timenet_connectors/api.py @@ -24,11 +24,12 @@ def build( # ruff: ignore[too-many-arguments] fetch_from: "str | BaseRegistry | None" = None, build_parents: bool = False, ) -> Path: - # ruff: ignore[docstring-extraneous-exception] (raised by _check_version and the parent planner, not directly here) - """Build a dataset into a local registry by connector id. + # ruff: ignore[docstring-extraneous-exception] (raised by build_release, not directly here) + """Build a dataset release into a local registry by its dataset ID. - This is the producer-side one-liner over the engine. The build reuses an already-built version - unless the caller sets ``force``. The default output is the shared local registry + This is the producer-side one-liner over the build coordinator, + :func:`timenet.coordinator.build.build_release`. The build reuses a release that the registry + already holds, unless the caller sets ``force``. The default output is the shared local registry (:func:`timenet.registry.default_registry_path`). The SDK reads from that directory, so a build here loads at once with ``TimeNet().load(dataset_id)``. @@ -38,69 +39,51 @@ def build( # ruff: ignore[too-many-arguments] download, unless ``fetch_from`` or ``build_parents`` obtains them. Args: - dataset_id: The dataset id (``org/name``). - version: The expected dataset version. A connector produces only its own version, so this - value is a guard. If it does not match the connector's metadata, the build stops before it - runs. ``None`` builds the version that the connector declares. + dataset_id: The dataset ID, exactly as the card of the connector declares it (``org/name``). + version: The expected dataset version. A connector builds only the version that its card + declares, so this value is a guard. If it is another version, the build stops before + anything runs. ``None`` builds the version that the card declares. out: Output registry directory. Defaults to the shared local registry. - force: Rebuild even if the registry already has a built version. + force: Build the release again, even if the registry already holds it. keep_cache: Keep the raw download cache. A successful build deletes it by default. fetch_from: The registry, or its URI, to copy missing parent releases from. build_parents: Build missing parent releases from their installed connectors. Returns: - The committed version directory. + The version directory of the release. Raises: - TimeFValidationError: If the caller sets ``version`` and it does not match the connector's - declared version. + TimeNetDatasetNotFoundError: If no installed connector builds ``dataset_id``. + TimeNetVersionUnavailableError: If the caller sets ``version`` and the card declares another + version. The error is also a :class:`~timenet.errors.TimeFValidationError`. TimeNetMissingParentError: If a parent release is missing, with a hint that names the keywords. """ + from timenet.builders import BuildOptions, declared # ruff: ignore[import-outside-top-level] + from timenet.coordinator.build import build_release # ruff: ignore[import-outside-top-level] from timenet.errors import TimeNetMissingParentError # ruff: ignore[import-outside-top-level] - from timenet.registry import default_registry_path # ruff: ignore[import-outside-top-level] - from timenet_connectors.builder.cli import build_one # ruff: ignore[import-outside-top-level] - - if version is not None: - _check_version(dataset_id, version) + from timenet.registry import ( # ruff: ignore[import-outside-top-level] + LocalRegistry, + default_registry_path, + open_registry, + ) + from timenet.registry._paths import release_dir # ruff: ignore[import-outside-top-level, import-private-name] + + release = declared(dataset_id).release root = Path(out).expanduser() if out is not None else default_registry_path() try: - built = build_one( - dataset_id, root, fetch_from=fetch_from, build_parents=build_parents, force=force, keep_cache=keep_cache + build_release( + f"{dataset_id}@{version}" if version else dataset_id, + store=LocalRegistry(root), + fetch_from=open_registry(fetch_from) if isinstance(fetch_from, str) else fetch_from, + build_parents=build_parents, + force=force, + options=BuildOptions(keep_cache=keep_cache), ) except TimeNetMissingParentError as error: error.hint = "Copy it with fetch_from= or build it with build_parents=True." raise - return Path(built) - - -def _check_version(dataset_id: str, version: str) -> None: - # ruff: ignore[docstring-extraneous-exception] (LookupError and TimeNetInvalidCardError come from the calls below) - """Reject a requested version the connector does not build. - - Deliberately import-free, like :func:`timenet_connectors.discovery.requirements_for`. This guard - runs before the isolated build, so asking the connector for its metadata would need the very - dependencies that only the child environment has. It reads the card beside the connector instead. - - Args: - dataset_id: The dataset id (``org/name``). - version: The requested dataset version. - - Raises: - TimeFValidationError: If the connector declares a different version. - LookupError: If no connector exists for the id. - TimeNetInvalidCardError: If the connector's card is missing or invalid. - """ - from timenet.errors import TimeFValidationError # ruff: ignore[import-outside-top-level] - from timenet.types import DatasetMetadata # ruff: ignore[import-outside-top-level] - from timenet_connectors.discovery import connector_dir # ruff: ignore[import-outside-top-level] - - card = DatasetMetadata.from_yaml(connector_dir(dataset_id) / "dataset.yaml") - available = str(card.dataset_version) - if version != available: - raise TimeFValidationError( - f"connector for {dataset_id!r} builds version {available}, not the requested {version}" - ) + return release_dir(root, release) def load( @@ -118,7 +101,7 @@ def load( Args: dataset_id: The dataset id (``org/name``). version: The version string, or ``None`` for the latest. :func:`build` checks this value - against the connector's declared version and rejects a mismatch. + against the version that the card of the connector declares, and rejects another one. fetch_from: The registry, or its URI, that the build and the read copy missing parent releases and payload sources from. build_parents: Build missing parent releases from their installed connectors. diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py index aad953d4b..ba5a35c2c 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py @@ -123,7 +123,7 @@ def build_prepared( """Build a release in a local registry. The registry must already contain all parent releases and their payload sources. - Set ``TIMENET_ISOLATION=off`` to build in this interpreter. + ``options.isolation``, else ``TIMENET_ISOLATION``, set to ``"off"`` builds in this interpreter. Args: release: The exact dataset ID and version to build. @@ -142,7 +142,7 @@ def build_prepared( raise TimeNetDatasetNotFoundError(f"no connector builds {release.dataset_id!r}") if card.dataset_version != release.version: raise TimeNetVersionUnavailableError(release, card.dataset_version) - if settings().isolation == "on": + if settings(isolation=options.isolation).isolation == "on": # The parent reads only the card: the connector and its dependencies exist only in the child. run_isolated_build( release, diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index ce1e73d0a..c4ff52208 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -6,8 +6,7 @@ ``CONNECTOR`` and a ``dataset.yaml`` card. You do not register it here. """ -from collections.abc import Generator, Mapping -from contextlib import contextmanager, nullcontext +from collections.abc import Mapping import json from pathlib import Path @@ -17,6 +16,7 @@ from timenet.cli.runner import run_cli from timenet.cli.ui import console, progress_renderer from timenet.config import settings +from timenet.coordinator.build import build_release from timenet.coordinator.plan import Step, prepare_parents from timenet.engine import publish_pipeline, run_pipeline from timenet.errors import ( @@ -27,20 +27,19 @@ TimeNetRegistryError, ) from timenet.manifest import PayloadSource -from timenet.progress import BytesMoved from timenet.registry import ( BaseRegistry, LocalRegistry, WritableRegistry, + copy_release, local_registry_path, open_registry, open_writable_registry, ) +from timenet.registry._paths import release_dir # ruff: ignore[import-private-name] from timenet.types.references import DatasetRef, split_ref -from timenet.writer.progress import ProgressStage, WriteProgressEvent from timenet_connectors.builder.env import run_isolated from timenet_connectors.discovery import known_ids, resolve -from timenet_connectors.download import DownloadProgress, progress_sink app = typer.Typer(help="Build TimeNet datasets from connectors.", no_args_is_help=True) @@ -85,11 +84,13 @@ def build( help="Output registry: a local directory or a timenet:// / http(s):// / s3:// URL " "(default: $TIMENET_REGISTRY, else the local registry).", ), - force: bool = typer.Option(False, "--force", "-f", help="Rebuild even if the version is already built."), + force: bool = typer.Option( + False, "--force", "-f", help="Build the release again, even if the local registry holds it." + ), fetch_from: str | None = typer.Option( None, "--fetch-from", - help="Copy missing parent releases from this registry URI (local --out only).", + help="Copy missing parent releases from this registry URI.", ), build_parents: bool = typer.Option( False, "--build-parents", help="Build missing parent releases from their installed connectors." @@ -113,31 +114,38 @@ def build( """Run a connector through the engine and write or publish its dataset. A local ``--out`` writes a dataset-layout directory and prints its path to stdout. A remote - ``--out`` (``timenet://`` / ``http(s)://`` / ``s3://``) publishes through the registry and prints - the version. Either way an emoji summary goes to stderr, so scripts can capture stdout. A built - version is reused unless you give ``--force``. + ``--out`` (``timenet://`` / ``http(s)://`` / ``s3://``) gets the release in two steps. First, the + release builds into the default local registry. Then a copy publishes it with its payload + sources, and the command prints the version. If the remote registry already holds the version + and ``--force`` is not set, nothing builds and nothing is copied. Either way an emoji summary goes + to stderr, so scripts can capture stdout. A built version is reused unless you give ``--force``. + A published version never changes. So ``--force`` with a remote ``--out`` that holds the version + is a usage error, and nothing runs. The build runs in an environment built from the connector's ``requirements.txt``. Pass ``--no-isolation`` (or set ``TIMENET_ISOLATION=off``) to run it in the current interpreter instead, which is what you want while writing a connector. - A dataset whose card declares ``parents`` needs those exact parent releases in the output + A dataset whose card declares ``parents`` needs those exact parent releases in the local registry. ``--fetch-from`` names a registry to copy a missing parent from. ``--build-parents`` builds a missing parent from its own connector. If neither option obtains a missing parent, the build fails before any download. The build prepares every parent before the dataset itself. It - never builds a parent over a release that the output already holds. If the output already holds - the version, the build needs only the payload sources of the version. It prepares them the same - way as a parent. + never builds a parent over a release that the local registry already holds. + If the local registry already holds the version, the build needs only the payload sources of + the version. It prepares them the same way as a parent. Raises: - BadParameter: If ``dataset_id`` has no known connector. + BadParameter: If ``dataset_id`` has no known connector. Also if ``--force`` is set and the + remote ``--out`` holds the version. Nothing runs then. 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. + TimeNetFileError: If a copy of a parent in the local registry is damaged, or a copied file + does not match its manifest record. The hint of the error names the flags. TimeNetAccessError: If the output is remote and gets a release without access declared ``open``. - """ # ruff: ignore[docstring-extraneous-exception] - raised by declared and the build + TimeNetVersionConflictError: If the copy to the remote ``--out`` finds other files of the + version, or of a payload source of it, in that registry. Nothing is copied then. + """ # ruff: ignore[docstring-extraneous-exception] - the copy raises TimeNetVersionConflictError # known_ids() imports no connector. So an unknown id fails before any environment exists. dataset_id, version = split_ref(release) known = known_ids() @@ -145,21 +153,22 @@ def build( 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) try: - result = build_one( - dataset_id, - forwarded, - fetch_from=fetch_from, - build_parents=build_parents, - force=force, - keep_cache=keep_cache, - isolation=isolation, - narrate=True, - pinned_by=None if pinned_by is None else _read_pins(pinned_by), - ) + # The parent shows each copy and build step. An isolated child shows its own downloads and shards. + with progress_renderer() as progress: + result = _build_to( + dataset_id, + _resolve_target(out), + fetch_from=None if fetch_from is None else open_registry(fetch_from), + build_parents=build_parents, + force=force, + pinned_by=None if pinned_by is None else _read_pins(pinned_by), + options=BuildOptions( + keep_cache=keep_cache, + progress=progress, + isolation=None if isolation is None else ("on" if isolation else "off"), + ), + ) except TimeNetMissingParentError as error: error.hint = "Fetch it with --fetch-from or build it with --build-parents." raise @@ -176,6 +185,70 @@ def build( typer.echo(result) +def _build_to( # ruff: ignore[too-many-arguments] + dataset_id: str, + target: WritableRegistry | Path, + *, + fetch_from: BaseRegistry | None, + build_parents: bool, + force: bool, + pinned_by: Mapping[DatasetRef, PayloadSource] | None, + options: BuildOptions, +) -> str: + """Build a release into a local output, or build it locally and copy it to a remote output. + + If nothing runs, because the local output holds the release or the remote output holds the + version, this function says so on the console. + + Args: + dataset_id: The dataset ID. + target: The local directory, or the registry that is not local. + fetch_from: The registry to copy missing parents from, or ``None``. + build_parents: Build missing parents with their installed connectors. + force: Build the release again, even if the local registry holds it. + pinned_by: The releases that already read files of the release, with their pins. + options: The options of each build. + + Returns: + The version directory for a local output, or the version for a remote one. + + Raises: + BadParameter: If ``force`` is set and the remote output holds the version. Nothing runs then. + """ + release = declared(dataset_id).release + if isinstance(target, Path): + store, remote = LocalRegistry(target), None + elif target.exists(release.dataset_id, str(release.version)): + if force: + raise typer.BadParameter( + f"the remote --out holds {release}, and a published version never changes. --force builds " + "a release again only for a local --out, or for a version that the remote --out does not hold.", + param_hint="--force", + ) + console.success(f"Nothing to do: '{release.dataset_id}' {release.version} is already in the remote --out") + return str(release.version) + else: + store, remote = LocalRegistry(settings().registry_path), target + # A remote registry holds only open releases. So for a remote output, the build refuses another + # release before any download, as a build into that registry would. + plan = build_release( + dataset_id, + store=store, + fetch_from=fetch_from, + build_parents=build_parents, + force=force, + pinned_by=pinned_by, + open_access_only=remote is not None, + options=options, + ) + if remote is not None: + copy_release(release, store, remote, progress=options.progress) + return str(release.version) + if not plan.steps: + console.success(f"Nothing to do: '{release.dataset_id}' {release.version} is already in {store}") + return str(release_dir(store.root, release)) + + @app.command("build-prepared-release", hidden=True) def build_prepared_release_command( release: str, @@ -210,13 +283,19 @@ def build_one( # ruff: ignore[too-many-arguments] force: bool = False, keep_cache: bool = False, isolation: bool | None = None, - narrate: bool = False, pinned_by: Mapping[DatasetRef, PayloadSource] | None = None, ) -> str: """Prepare a dataset's parents, then build or reuse its release. - The build fails before downloading if it cannot copy or build a missing parent. - A new release must match every file record in ``pinned_by`` before publication. + The ``build()`` method of :class:`~timenet_connectors.builder.backend.ConnectorBuilder` calls + this function. If ``fetch_from`` or ``build_parents`` does not obtain a missing parent, the + build fails before any download. A parent builds the same way as this dataset. A parent never + builds with ``force``. If another builder backend claims a parent, the ``build`` method of that + backend builds the parent with its pins. If ``force`` is not set and the output already holds + the version, the build prepares only the missing payload sources of the version. In that case, + the build does not run the connector of this dataset. The connector runs in an environment + built from its requirements. If isolation is off, the connector runs in this interpreter. A + local output gets a version directory. A remote output gets a published version. Args: dataset_id: The dataset ID. @@ -225,9 +304,10 @@ def build_one( # ruff: ignore[too-many-arguments] 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``. - narrate: Print status and download progress when isolation is off. - pinned_by: The expected file records for this release, grouped by the releases that read them. + isolation: If true, build in an environment built from the connector's requirements. + If ``None``, use the ``TIMENET_ISOLATION`` setting. + pinned_by: The releases that already read files of this release, with their pins. The build + publishes the release only if it reproduces each pinned file. Returns: The committed version directory for a local output, or the published version. @@ -249,7 +329,6 @@ def build_parent(step: Step) -> object: output, isolated=isolated, keep_cache=keep_cache, - narrate=narrate, pinned_by=step.pinned_by, ) @@ -263,9 +342,6 @@ def build_parent(step: Step) -> object: ) committed = None if force else _committed(dataset_id, target, registry) if committed is not None: - if narrate: - built = f"Built '{dataset_id}' → {Path(committed).name}" - console.success(built if isinstance(target, Path) else f"Published '{dataset_id}' → {committed}") return committed return _build( dataset_id, @@ -274,7 +350,6 @@ def build_parent(step: Step) -> object: isolated=isolated, force=force, keep_cache=keep_cache, - narrate=narrate, pinned_by=pinned_by, ) @@ -341,7 +416,6 @@ def _build( # ruff: ignore[too-many-arguments] isolated: bool, force: bool = False, keep_cache: bool = False, - narrate: bool = False, pinned_by: Mapping[DatasetRef, PayloadSource] | None = None, ) -> str: """Build a dataset in a local directory or writable registry. @@ -353,8 +427,7 @@ def _build( # ruff: ignore[too-many-arguments] isolated: Build in an environment built from the connector's requirements. force: Rebuild an existing version. keep_cache: Keep the raw download cache after building. - narrate: Print status and download progress when isolation is off. - pinned_by: Releases that read this dataset's files, with file pins the build must match. + pinned_by: The releases that already read files of this release, with their pins. Returns: The committed version directory for a local output, or the published version. @@ -375,35 +448,23 @@ def _build( # ruff: ignore[too-many-arguments] quiet=console.quiet, pinned_by=pinned_by, ) - if narrate: - console.status("🔧", f"Building '{dataset_id}'…") connector = resolve(dataset_id)() - progress_cb = _report_progress if narrate else None - # A connector's downloads report through the ambient progress sink. _download_progress renders them. - with _download_progress() if narrate else nullcontext(): - if isinstance(target, Path): - version_dir = run_pipeline( - connector, - target, - progress_cb=progress_cb, - force=force, - keep_cache=keep_cache, - pinned_by=pinned_by, - ) - if narrate: - console.success(f"Built '{dataset_id}' → {version_dir.name}") - return str(version_dir) - version = publish_pipeline( + if isinstance(target, Path): + version_dir = run_pipeline( connector, target, - progress_cb=progress_cb, force=force, keep_cache=keep_cache, pinned_by=pinned_by, ) - if narrate: - console.success(f"Published '{dataset_id}' → {version}") - return version + return str(version_dir) + return publish_pipeline( + connector, + target, + force=force, + keep_cache=keep_cache, + pinned_by=pinned_by, + ) def _read_pins(path: Path) -> dict[DatasetRef, PayloadSource]: @@ -416,34 +477,6 @@ def _read_pins(path: Path) -> dict[DatasetRef, PayloadSource]: return {DatasetRef.parse(key): PayloadSource.model_validate(value) for key, value in pins.items()} -def _report_progress(event: WriteProgressEvent) -> None: - """Relay writer progress to the console. - - The shared console coordinates with any live download bars, so this renders above them. - - Args: - event: The writer progress event. - """ - if event.stage is ProgressStage.SHARD_FINALIZED: - console.status("💾", f"wrote shard {event.completed}") - - -@contextmanager -def _download_progress() -> Generator[None]: - """Display download progress during the ``with`` block.""" - with progress_renderer() as render: - if render is None: - with progress_sink(None): - yield - return - - def report(event: DownloadProgress) -> None: - render(BytesMoved(event.url, event.downloaded, event.total)) - - with progress_sink(report): - yield - - def main() -> None: """Entry point for the ``timenet-build`` console script. diff --git a/packages/timenet-connectors/tests/test_build_cli.py b/packages/timenet-connectors/tests/test_build_cli.py index 15cfb60d2..c20d09b64 100644 --- a/packages/timenet-connectors/tests/test_build_cli.py +++ b/packages/timenet-connectors/tests/test_build_cli.py @@ -1,10 +1,10 @@ import json import os -from pathlib import Path import subprocess import sys import pytest +from rich.console import Console from typer.testing import CliRunner from timenet.connectors import BaseConnector @@ -12,7 +12,7 @@ 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.registry import LocalRegistry, open_local_version from timenet.testing import make_dataset from timenet.types import DatasetMetadata, Version from timenet.types.reference_keys import ParentAlias @@ -23,96 +23,118 @@ runner = CliRunner() +HELLO = DatasetRef.parse("timenet/hello-world@1.0.0") -def test_build_runs_isolated_by_default(monkeypatch, tmp_path): - captured = {} +def _isolated_child(calls): + """Return a stand-in for ``run_isolated_build`` that stores the demo dataset and records each call.""" - def _fake_isolated( # ruff: ignore[too-many-arguments] (mirrors run_isolated's full keyword surface) - dataset_id, root, *, force=False, keep_cache=False, quiet=False, pinned_by=None - ): - captured["dataset_id"] = dataset_id - captured["root"] = root - return Path(root) / "timenet/hello-world/1.0.0" + def run(release, store_root, *, force=False, quiet=False, **_): + calls.append({"release": release, "store_root": store_root, "quiet": quiet}) + LocalRegistry(store_root).put_dataset(make_dataset(), force=force) - monkeypatch.setattr(cli_module, "run_isolated", _fake_isolated) - result = runner.invoke(app, ["build", "timenet/hello-world", "--out", str(tmp_path)]) + return run - assert result.exit_code == 0 - assert captured["dataset_id"] == "timenet/hello-world" +def _in_process(calls): + """Return a stand-in for ``run_pipeline`` that stores the demo dataset and records each call.""" -def test_build_no_isolation_runs_in_process(monkeypatch, tmp_path): - called = {"isolated": False, "pipeline": False} + def run(connector, root, **kwargs): + calls.append({"connector": connector, **kwargs}) + LocalRegistry(root).put_dataset(make_dataset()) - monkeypatch.setattr(cli_module, "run_isolated", lambda *a, **k: called.__setitem__("isolated", True)) + return run - def _fake_pipeline(connector, root, **kwargs): - called["pipeline"] = True - return Path(root) / "timenet/hello-world/1.0.0" - monkeypatch.setattr(cli_module, "run_pipeline", _fake_pipeline) - result = runner.invoke(app, ["build", "timenet/hello-world", "--out", str(tmp_path), "--no-isolation"]) +def _unexpected(*args, **kwargs): + raise AssertionError(f"should not have run: {args} {kwargs}") - assert result.exit_code == 0 - assert called["pipeline"] is True - assert called["isolated"] is False +def test_build_runs_isolated_by_default(monkeypatch, tmp_path): + monkeypatch.delenv("TIMENET_ISOLATION", raising=False) + calls = [] + monkeypatch.setattr(backend_module, "run_isolated_build", _isolated_child(calls)) + monkeypatch.setattr(backend_module, "run_pipeline", _unexpected) -def test_build_runs_in_process_when_the_setting_is_off(monkeypatch, tmp_path): - called = {"isolated": False, "pipeline": False} - monkeypatch.setenv("TIMENET_ISOLATION", "off") + result = runner.invoke(app, ["build", "timenet/hello-world", "--out", str(tmp_path)]) - monkeypatch.setattr(cli_module, "run_isolated", lambda *a, **k: called.__setitem__("isolated", True)) + assert result.exit_code == 0, result.output + assert [(call["release"], call["store_root"]) for call in calls] == [(HELLO, tmp_path)] + assert result.stdout == f"{tmp_path / 'timenet/hello-world/1.0.0'}\n" - def _fake_pipeline(connector, root, **kwargs): - called["pipeline"] = True - return Path(root) / "timenet/hello-world/1.0.0" - monkeypatch.setattr(cli_module, "run_pipeline", _fake_pipeline) - result = runner.invoke(app, ["build", "timenet/hello-world", "--out", str(tmp_path)]) +@pytest.mark.parametrize("how", ["flag", "setting"]) +def test_build_runs_in_process_without_isolation(monkeypatch, tmp_path, how): + calls = [] + monkeypatch.setattr(backend_module, "run_isolated_build", _unexpected) + monkeypatch.setattr(backend_module, "run_pipeline", _in_process(calls)) + if how == "flag": + monkeypatch.delenv("TIMENET_ISOLATION", raising=False) + else: + monkeypatch.setenv("TIMENET_ISOLATION", "off") - assert result.exit_code == 0 - assert called["pipeline"] is True - assert called["isolated"] is False + result = runner.invoke( + app, ["build", "timenet/hello-world", "--out", str(tmp_path), *(["--no-isolation"] if how == "flag" else [])] + ) + + assert result.exit_code == 0, result.output + assert len(calls) == 1 def test_build_isolates_when_the_flag_overrides_the_setting(monkeypatch, tmp_path): - called = {"isolated": False} monkeypatch.setenv("TIMENET_ISOLATION", "off") + calls = [] + monkeypatch.setattr(backend_module, "run_isolated_build", _isolated_child(calls)) + monkeypatch.setattr(backend_module, "run_pipeline", _unexpected) - def _fake_isolated( # ruff: ignore[too-many-arguments] (mirrors run_isolated's full keyword surface) - dataset_id, root, *, force=False, keep_cache=False, quiet=False, pinned_by=None - ): - called["isolated"] = True - return Path(root) / "timenet/hello-world/1.0.0" + result = runner.invoke(app, ["build", "timenet/hello-world", "--out", str(tmp_path), "--isolation"]) - def _unexpected_pipeline(*args, **kwargs): - raise AssertionError("an explicit --isolation must win over TIMENET_ISOLATION=off") + assert result.exit_code == 0, result.output + assert len(calls) == 1 - monkeypatch.setattr(cli_module, "run_isolated", _fake_isolated) - monkeypatch.setattr(cli_module, "run_pipeline", _unexpected_pipeline) - result = runner.invoke(app, ["build", "timenet/hello-world", "--out", str(tmp_path), "--isolation"]) - assert result.exit_code == 0 - assert called["isolated"] is True +def test_build_without_isolation_shows_each_build_line_once(monkeypatch, tmp_path): + # In this process, the coordinator sends the build step, and the renderer shows it. Nothing else + # prints a build line. + monkeypatch.setenv("TIMENET_HOME", str(tmp_path / "home")) + result = runner.invoke(app, ["build", "timenet/hello-world", "--out", str(tmp_path / "out"), "--no-isolation"]) -def test_build_takes_an_exact_release_and_refuses_another_version(monkeypatch, tmp_path): - built = [] + assert result.exit_code == 0, result.output + assert result.stderr.count("🔧 Building 'timenet/hello-world'…") == 1 + assert result.stderr.count("✅ Built 'timenet/hello-world' → 1.0.0") == 1 + + +@pytest.mark.parametrize("quiet", [False, True]) +def test_a_build_that_reuses_the_release_says_so_on_stderr(monkeypatch, tmp_path, quiet): + monkeypatch.delenv("TIMENET_ISOLATION", raising=False) + # The line names a long path, so the console must not wrap it. + monkeypatch.setattr(cli_module.console, "rich", Console(stderr=True, width=1000)) + calls = [] + monkeypatch.setattr(backend_module, "run_isolated_build", _isolated_child(calls)) + command = [*(["--quiet"] if quiet else []), "build", "timenet/hello-world", "--out", str(tmp_path)] + assert runner.invoke(app, command).exit_code == 0 + + result = runner.invoke(app, command) + + assert result.exit_code == 0, result.output + assert len(calls) == 1 + assert result.stdout == f"{tmp_path / 'timenet/hello-world/1.0.0'}\n" + reused = f"✅ Nothing to do: 'timenet/hello-world' 1.0.0 is already in local registry {tmp_path}" + assert result.stderr.splitlines() == ([] if quiet else [reused]) - 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) +def test_build_takes_an_exact_release_and_refuses_another_version(monkeypatch, tmp_path): + monkeypatch.delenv("TIMENET_ISOLATION", raising=False) + calls = [] + monkeypatch.setattr(backend_module, "run_isolated_build", _isolated_child(calls)) 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"] + assert result.exit_code == 0, result.output + assert [call["release"] for call in calls] == [HELLO] 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"] + assert len(calls) == 1 def test_build_rejects_an_unknown_id_without_importing_anything(monkeypatch, tmp_path): @@ -141,30 +163,28 @@ def test_build_rejects_an_id_that_no_card_declares_and_names_the_known_ids(tmp_p assert "timenet/hello-world" in result.output -def test_build_forwards_quiet_to_the_isolated_child(monkeypatch, tmp_path): - captured = {} +@pytest.mark.parametrize("quiet", [False, True]) +def test_build_forwards_quiet_to_the_isolated_child(monkeypatch, tmp_path, quiet): + monkeypatch.delenv("TIMENET_ISOLATION", raising=False) # The console is a process-wide singleton; monkeypatch restores it for the tests that follow. monkeypatch.setattr(cli_module.console, "quiet", False) + calls = [] + monkeypatch.setattr(backend_module, "run_isolated_build", _isolated_child(calls)) - def _fake_isolated( # ruff: ignore[too-many-arguments] (mirrors run_isolated's full keyword surface) - dataset_id, root, *, force=False, keep_cache=False, quiet=False, pinned_by=None - ): - captured["quiet"] = quiet - return Path(root) / "timenet/hello-world/1.0.0" - - monkeypatch.setattr(cli_module, "run_isolated", _fake_isolated) - result = runner.invoke(app, ["--quiet", "build", "timenet/hello-world", "--out", str(tmp_path)]) + result = runner.invoke( + app, [*(["--quiet"] if quiet else []), "build", "timenet/hello-world", "--out", str(tmp_path)] + ) - assert result.exit_code == 0 - assert captured["quiet"] is True + assert result.exit_code == 0, result.output + assert [call["quiet"] for call in calls] == [quiet] @pytest.mark.slow def test_isolated_build_prints_the_version_dir_and_narrates_once(tmp_path): - # run_isolated reads the child's last non-empty stdout line as the version directory, so the - # real isolated CLI must keep stdout clean and end on that path. Drive it as a subprocess (not - # CliRunner) so the child's inherited stderr is captured here too, where a double-narration - # regression would surface. + # A script reads the last non-empty stdout line as the version directory, so the real isolated + # CLI must keep stdout clean and end on that path. Drive it as a subprocess (not CliRunner) so + # the child's inherited stderr is captured here too, where a double-narration regression would + # surface: the parent and the child both see the build step. registry = tmp_path / "registry" env = {**os.environ, "TIMENET_HOME": str(tmp_path / "home")} env.pop("TIMENET_ISOLATION", None) # default is "on"; make sure nothing forces it off diff --git a/packages/timenet-connectors/tests/test_build_plan.py b/packages/timenet-connectors/tests/test_build_plan.py index 303ea6c94..37de30e74 100644 --- a/packages/timenet-connectors/tests/test_build_plan.py +++ b/packages/timenet-connectors/tests/test_build_plan.py @@ -440,28 +440,43 @@ def test_a_remote_output_names_the_access_page_of_a_held_parent_from_its_lineage assert caught.value.requirements == (AccessRequirement(GRAND, Access.CREDENTIALED, _GATED["access_url"]),) -def _isolated_builds(calls: list) -> object: - """Return a stand-in for ``run_isolated`` that stores each dataset from its card and records the call. +def _store_from_card(dataset_id: str, root: Path, *, force: bool) -> Path: + """Store a dataset from its card and return its version directory. A dataset with parents imports one Record of each; one without parents holds the demo dataset. """ + registry = LocalRegistry(Path(root)) + 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() + dataset._metadata = metadata + for handle in context.parents.values(): + dataset.import_from(handle, records=[make_dataset().records[0].id]) + registry.put_dataset(dataset, force=force) + return registry.root / dataset_id / str(metadata.dataset_version) + + +def _isolated_builds(calls: list) -> object: + """Return a stand-in for ``run_isolated`` that stores each dataset from its card and records the call.""" def run_isolated(dataset_id, root, *, force=False, **_): calls.append((dataset_id, force)) - registry = LocalRegistry(Path(root)) - 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() - dataset._metadata = metadata - for handle in context.parents.values(): - dataset.import_from(handle, records=[make_dataset().records[0].id]) - registry.put_dataset(dataset, force=force) - return str(registry.root / dataset_id / str(metadata.dataset_version)) + return str(_store_from_card(dataset_id, root, force=force)) return run_isolated +def _prepared_builds(calls: list) -> object: + """Return a stand-in for ``run_isolated_build`` that stores each release from its card and records the call.""" + + def run_isolated_build(release, store_root, *, force=False, **_): + calls.append((release.dataset_id, force)) + _store_from_card(release.dataset_id, store_root, force=force) + + return run_isolated_build + + def test_build_prepares_parents_before_the_isolated_child(tmp_path, monkeypatch): cards = _install_cards(monkeypatch, child={"parent": PARENT, "grand": GRAND}, parent={}) monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) @@ -470,7 +485,7 @@ def test_build_prepares_parents_before_the_isolated_child(tmp_path, monkeypatch) origin.put_dataset(make_dataset()) out = tmp_path / "out" calls = [] - monkeypatch.setattr(cli_module, "run_isolated", _isolated_builds(calls)) + monkeypatch.setattr(backend_module, "run_isolated_build", _prepared_builds(calls)) result = runner.invoke( app, @@ -499,6 +514,7 @@ def test_every_build_entry_point_prepares_parents(tmp_path, monkeypatch, entry): monkeypatch.delenv("TIMENET_ISOLATION", raising=False) calls = [] monkeypatch.setattr(cli_module, "run_isolated", _isolated_builds(calls)) + monkeypatch.setattr(backend_module, "run_isolated_build", _prepared_builds(calls)) build = timenet_connectors.build if entry == "api" else ConnectorBuilder().build out = tmp_path / "out" @@ -527,7 +543,7 @@ def test_load_obtains_a_missing_parent_before_it_builds_and_reads(tmp_path, monk monkeypatch.setenv("TIMENET_HOME", str(tmp_path / "home")) monkeypatch.setenv("TIMENET_REGISTRY", str(tmp_path / "out")) calls = [] - monkeypatch.setattr(cli_module, "run_isolated", _isolated_builds(calls)) + monkeypatch.setattr(backend_module, "run_isolated_build", _prepared_builds(calls)) origin = LocalRegistry(tmp_path / "origin") parent = make_dataset() parent._metadata = cards[PARENT.dataset_id] @@ -599,17 +615,15 @@ 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): - cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={"parent": GRAND}) + cards = _install_cards(monkeypatch, 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))] received, pin_files = [], [] - def run_pipeline(connector, root, *, pinned_by=None, **_): + def run_pipeline(connector, root, *, pinned_by, **_): received.append(pinned_by) - if pinned_by: - output.put_dataset(make_dataset()) - return Path(root) + output.put_dataset(make_dataset(), pinned_by=pinned_by) def child(command, env): # Stands in for the uv child: run the forwarded command line through the CLI in this process. @@ -623,15 +637,18 @@ def child(command, env): monkeypatch.setattr(env_module, "requirements_for", lambda dataset_id: None) monkeypatch.setattr(env_module, "_run_build", child) monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) - monkeypatch.setattr(cli_module, "resolve", lambda dataset_id: _Grand) - monkeypatch.setattr(cli_module, "run_pipeline", run_pipeline) + monkeypatch.setattr(backend_module, "resolve", lambda dataset_id: _Grand) + monkeypatch.setattr(backend_module, "run_pipeline", run_pipeline) - cli_module.build_one(CHILD.dataset_id, output.root, build_parents=True) + # The output holds PARENT, so the build gets only GRAND, the payload source of PARENT. + result = runner.invoke(app, ["build", PARENT.dataset_id, "--out", str(output.root), "--build-parents"]) + assert result.exit_code == 0, result.output # The rebuild of GRAND gets the pins of PARENT, through a file that is gone afterwards. - assert received == [{PARENT: pins}, None] + assert received == [{PARENT: pins}] assert len(pin_files) == 1 assert not pin_files[0].exists() + assert output.exists(GRAND.dataset_id, str(GRAND.version)) STORED = DatasetRef.parse("test/stored@1.0.0") @@ -754,6 +771,7 @@ def test_a_proxy_backend_builds_a_parent_through_the_build_of_its_instance(tmp_p def test_the_cli_hint_names_its_flags(tmp_path, monkeypatch, case): cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) + monkeypatch.setenv("TIMENET_HOME", str(tmp_path / "home")) out = LocalRegistry(tmp_path / "out") if case == "damaged": out.put_dataset(TimeFDataset(metadata=cards[PARENT.dataset_id])) @@ -786,3 +804,90 @@ def test_the_cli_hint_names_its_flags(tmp_path, monkeypatch, case): error, hint = hints[case] assert isinstance(result.exception, error) assert str(result.exception).endswith(hint) + + +def _remote_out(tmp_path, monkeypatch, cards) -> tuple[_Hosted, LocalRegistry, list]: + """Make the ``--out`` of the CLI a registry that is not local, and record each isolated build. + + Returns: + The registry that ``--out`` names, the default local registry, and the list of the builds. + """ + remote = _Hosted(tmp_path / "hosted") + monkeypatch.setattr(cli_module, "_resolve_target", lambda target: remote) + monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) + monkeypatch.setenv("TIMENET_HOME", str(tmp_path / "home")) + monkeypatch.delenv("TIMENET_ISOLATION", raising=False) + calls = [] + monkeypatch.setattr(backend_module, "run_isolated_build", _prepared_builds(calls)) + return remote, LocalRegistry(tmp_path / "home/registry"), calls + + +@pytest.mark.parametrize("gated", ["child", "parent"]) +def test_build_to_a_remote_out_refuses_a_credentialed_release_before_any_download(tmp_path, monkeypatch, gated): + cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) + gated_ref = CHILD if gated == "child" else PARENT + cards[gated_ref.dataset_id] = cards[gated_ref.dataset_id].model_copy(update=_GATED) + origin = LocalRegistry(tmp_path / "origin") + origin.put_dataset(TimeFDataset(metadata=cards[PARENT.dataset_id])) + _, local, calls = _remote_out(tmp_path, monkeypatch, cards) + + command = ["build", CHILD.dataset_id, "--out", "s3://bucket/registry", "--fetch-from", str(origin.root)] + + result = runner.invoke(app, command) + + # The build into the local registry gets the access checks of the remote registry. + assert isinstance(result.exception, TimeNetAccessError), result.output + assert result.exception.dataset == gated_ref + assert calls == [] + assert not local.exists(PARENT.dataset_id, str(PARENT.version)) + + +def test_build_to_a_remote_out_builds_locally_and_copies(tmp_path, monkeypatch): + cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) + origin = LocalRegistry(tmp_path / "origin") + parent = make_dataset() + parent._metadata = cards[PARENT.dataset_id] + origin.put_dataset(parent) + remote, local, calls = _remote_out(tmp_path, monkeypatch, cards) + command = ["build", CHILD.dataset_id, "--out", "s3://bucket/registry", "--fetch-from", str(origin.root)] + + result = runner.invoke(app, command) + + assert result.exit_code == 0, result.output + assert result.stdout.strip() == str(CHILD.version) + # The parent lands in the local registry first, and the child builds there. Then the copy takes + # the child and its payload source, the parent, to the remote registry. + assert calls == [(CHILD.dataset_id, False)] + for registry in (local, remote): + assert registry.exists(CHILD.dataset_id, str(CHILD.version)) + assert registry.exists(PARENT.dataset_id, str(PARENT.version)) + # A remote registry that holds the version gets nothing, and nothing builds. + shutil.rmtree(local.root) + result = runner.invoke(app, command) + assert (result.exit_code, result.stdout.strip()) == (0, str(CHILD.version)) + assert result.stderr.splitlines() == ["✅ Nothing to do: 'test/child' 1.0.0 is already in the remote --out"] + assert calls == [(CHILD.dataset_id, False)] + assert not local.root.exists() + + +def _files(root: Path) -> dict[Path, tuple[int, int]]: + """Return the size and the modification time of each file under ``root``, by its path.""" + return {path: (path.stat().st_size, path.stat().st_mtime_ns) for path in root.rglob("*") if path.is_file()} + + +def test_force_is_a_usage_error_if_the_remote_out_holds_the_version(tmp_path, monkeypatch): + cards = _install_cards(monkeypatch, child={}, parent={}) + _, local, calls = _remote_out(tmp_path, monkeypatch, cards) + command = ["build", CHILD.dataset_id, "--out", "s3://bucket/registry"] + assert runner.invoke(app, command).exit_code == 0 + held = _files(local.root) + + result = runner.invoke(app, [*command, "--force"]) + + # A published version never changes. So the command refuses before anything downloads or builds. + assert result.exit_code == 2, result.output + assert "Invalid value for --force" in result.output + # The error box wraps the message and draws a border on each line. + assert "a published version never changes" in " ".join(result.output.replace("│", " ").split()) + assert calls == [(CHILD.dataset_id, False)] + assert _files(local.root) == held diff --git a/packages/timenet-connectors/tests/test_build_progress.py b/packages/timenet-connectors/tests/test_build_progress.py deleted file mode 100644 index 5df32d217..000000000 --- a/packages/timenet-connectors/tests/test_build_progress.py +++ /dev/null @@ -1,20 +0,0 @@ -"""Tests for the timenet-build download-progress rendering.""" - -import io -import sys - -from rich.console import Console - -from timenet.cli.ui import console -from timenet_connectors.builder.cli import _download_progress -from timenet_connectors.download.progress import DownloadProgress, report_progress - - -def test_download_progress_shows_the_downloads_of_a_connector(monkeypatch): - captured = io.StringIO() - monkeypatch.setattr(console, "rich", Console(file=captured, width=200)) - monkeypatch.setattr(console, "quiet", False) - monkeypatch.setattr(sys.stderr, "isatty", lambda: False, raising=False) - with _download_progress(): - report_progress(DownloadProgress("https://host/a.zip", 3, 9)) # routed through the ambient sink - assert captured.getvalue().splitlines() == ["⬇️ a.zip 33% (9 B)"] diff --git a/packages/timenet-connectors/tests/test_package_api.py b/packages/timenet-connectors/tests/test_package_api.py index c4e6db51a..a5a5d3eeb 100644 --- a/packages/timenet-connectors/tests/test_package_api.py +++ b/packages/timenet-connectors/tests/test_package_api.py @@ -1,16 +1,16 @@ -from pathlib import Path - import pytest -from timenet.errors import TimeFValidationError +from timenet.errors import TimeNetDatasetNotFoundError, TimeNetVersionUnavailableError +from timenet.registry import LocalRegistry +from timenet.testing import make_dataset from timenet.types import ClassificationTask +from timenet.types.references import DatasetRef import timenet_connectors -from timenet_connectors import discovery -from timenet_connectors.builder import cli as cli_module +from timenet_connectors.builder import backend as backend_module def _unexpected_isolation(*args, **kwargs): - raise AssertionError(f"run_isolated should not have run: {args} {kwargs}") + raise AssertionError(f"run_isolated_build should not have run: {args} {kwargs}") def _unexpected_resolve(*args, **kwargs): @@ -36,47 +36,53 @@ def test_build_then_load_round_trip(monkeypatch, tmp_path): assert set(y.to_pylist()) == {"above_zero", "below_zero"} -def test_build_runs_in_an_isolated_environment_by_default(monkeypatch, tmp_path): +@pytest.mark.parametrize("version", [None, "1.0.0"]) +def test_build_runs_in_an_isolated_environment_by_default(monkeypatch, tmp_path, version): + # The version check reads the card, so it does not import the connector either. The connector + # needs dependencies that only the environment of the child has. monkeypatch.delenv("TIMENET_ISOLATION", raising=False) + monkeypatch.setattr(backend_module, "resolve", _unexpected_resolve) calls = [] - def fake_isolated(dataset_id, root, *, force=False, keep_cache=False, **_): - calls.append((dataset_id, root, force, keep_cache)) - return Path(root) / "timenet" / "hello-world" / "1.0.0" + def fake_isolated(release, store_root, *, force=False, keep_cache=False, **_): + calls.append((release, store_root, force, keep_cache)) + LocalRegistry(store_root).put_dataset(make_dataset()) - monkeypatch.setattr(cli_module, "run_isolated", fake_isolated) + monkeypatch.setattr(backend_module, "run_isolated_build", fake_isolated) - version_dir = timenet_connectors.build("timenet/hello-world", out=str(tmp_path), force=True) + version_dir = timenet_connectors.build("timenet/hello-world", version=version, out=str(tmp_path), force=True) - assert calls == [("timenet/hello-world", tmp_path, True, False)] + assert calls == [(DatasetRef.parse("timenet/hello-world@1.0.0"), tmp_path, True, False)] assert version_dir == tmp_path / "timenet" / "hello-world" / "1.0.0" -def test_version_guard_does_not_import_the_connector(monkeypatch, tmp_path): - # The guard runs before the isolated build, so importing the connector here would need the very - # dependencies that only the child environment has. +def test_build_rejects_a_version_the_connector_does_not_declare(monkeypatch, tmp_path): monkeypatch.delenv("TIMENET_ISOLATION", raising=False) - monkeypatch.setattr(discovery, "resolve", _unexpected_resolve) - monkeypatch.setattr(cli_module, "run_isolated", lambda dataset_id, root, **kwargs: Path(root) / "built") + monkeypatch.setattr(backend_module, "resolve", _unexpected_resolve) + monkeypatch.setattr(backend_module, "run_isolated_build", _unexpected_isolation) - version_dir = timenet_connectors.build("timenet/hello-world", version="1.0.0", out=str(tmp_path)) + with pytest.raises( + TimeNetVersionUnavailableError, match=r"builds version 1\.0\.0, not the requested 9\.9\.9" + ) as caught: + timenet_connectors.build("timenet/hello-world", version="9.9.9", out=tmp_path) - assert version_dir == tmp_path / "built" + assert str(caught.value.declared) == "1.0.0" + assert not tmp_path.joinpath("timenet").exists() -def test_version_mismatch_is_rejected_without_importing_the_connector(monkeypatch, tmp_path): - monkeypatch.delenv("TIMENET_ISOLATION", raising=False) - monkeypatch.setattr(discovery, "resolve", _unexpected_resolve) - monkeypatch.setattr(cli_module, "run_isolated", _unexpected_isolation) +@pytest.mark.parametrize("dataset_id", ["nope/nothing", "timenet/hello_world"]) +def test_build_accepts_only_the_dataset_id_of_a_card(monkeypatch, tmp_path, dataset_id): + # The module path of the connector accepts the spelling with "_", but a dataset ID must match its card. + monkeypatch.setattr(backend_module, "run_isolated_build", _unexpected_isolation) - with pytest.raises(TimeFValidationError, match=r"builds version 1\.0\.0"): - timenet_connectors.build("timenet/hello-world", version="9.9.9", out=str(tmp_path)) + with pytest.raises(TimeNetDatasetNotFoundError, match=f"no installed connector builds {dataset_id!r}"): + timenet_connectors.build(dataset_id, out=tmp_path) def test_build_runs_in_process_when_isolation_is_off(monkeypatch, tmp_path): monkeypatch.setenv("TIMENET_HOME", str(tmp_path / "home")) monkeypatch.setenv("TIMENET_ISOLATION", "off") - monkeypatch.setattr(cli_module, "run_isolated", _unexpected_isolation) + monkeypatch.setattr(backend_module, "run_isolated_build", _unexpected_isolation) version_dir = timenet_connectors.build("timenet/hello-world", out=tmp_path / "registry") diff --git a/packages/timenet/src/timenet/builders.py b/packages/timenet/src/timenet/builders.py index 9d5c7c6fa..936d373a1 100644 --- a/packages/timenet/src/timenet/builders.py +++ b/packages/timenet/src/timenet/builders.py @@ -10,7 +10,7 @@ from dataclasses import dataclass from importlib.metadata import entry_points from pathlib import Path -from typing import TYPE_CHECKING, Protocol +from typing import TYPE_CHECKING, Literal, Protocol from timenet.errors import TimeNetDatasetNotFoundError, TimeNetVersionUnavailableError from timenet.progress import ProgressCallback @@ -34,11 +34,13 @@ class BuildOptions: 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. + isolation: Build in an environment of its own (``"on"``) or here (``"off"``); ``None`` keeps ``TIMENET_ISOLATION``. """ keep_cache: bool = False progress: ProgressCallback | None = None quiet: bool = False + isolation: Literal["on", "off"] | None = None class BuilderBackend(Protocol): From f47d6a1e328f8bfebffdb48eb153aca3b78fdfca Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 09:05:26 +0100 Subject: [PATCH 2/5] fix(build): pass --quiet to the child and copy parents from a remote --out The build command now sets BuildOptions.quiet from its --quiet option. Since the built-in backend reads only that option, --quiet silences the isolated child again. timenet_connectors.build() keeps the default, so its child prints its build lines and downloads. A remote --out without --fetch-from now copies a missing parent from the remote registry into the local one. Before this stack, a build into the remote registry found the parents there, so timenet-build build child --out worked when the remote held the parent. Now it works again, and only the child builds. --fetch-from still wins when it is given. The help text names the default. Part of TNET-157. --- .../src/timenet_connectors/builder/cli.py | 18 ++++++++++++------ .../tests/test_build_plan.py | 17 +++++++++++++++++ .../tests/test_package_api.py | 7 ++++--- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index c4ff52208..84d38fb8e 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -90,7 +90,7 @@ def build( fetch_from: str | None = typer.Option( None, "--fetch-from", - help="Copy missing parent releases from this registry URI.", + help="Copy missing parent releases from this registry URI (default for a remote --out: that registry).", ), build_parents: bool = typer.Option( False, "--build-parents", help="Build missing parent releases from their installed connectors." @@ -127,10 +127,11 @@ def build( instead, which is what you want while writing a connector. A dataset whose card declares ``parents`` needs those exact parent releases in the local - registry. ``--fetch-from`` names a registry to copy a missing parent from. ``--build-parents`` - builds a missing parent from its own connector. If neither option obtains a missing parent, the - build fails before any download. The build prepares every parent before the dataset itself. It - never builds a parent over a release that the local registry already holds. + registry. ``--fetch-from`` names a registry to copy a missing parent from. For a remote + ``--out``, that registry is the default, because it holds the parents of its releases. + ``--build-parents`` builds a missing parent from its own connector. If neither option obtains a + missing parent, the build fails before any download. The build prepares every parent before the + dataset itself. It never builds a parent over a release that the local registry already holds. If the local registry already holds the version, the build needs only the payload sources of the version. It prepares them the same way as a parent. @@ -166,6 +167,7 @@ def build( options=BuildOptions( keep_cache=keep_cache, progress=progress, + quiet=console.quiet, isolation=None if isolation is None else ("on" if isolation else "off"), ), ) @@ -203,7 +205,8 @@ def _build_to( # ruff: ignore[too-many-arguments] Args: dataset_id: The dataset ID. target: The local directory, or the registry that is not local. - fetch_from: The registry to copy missing parents from, or ``None``. + fetch_from: The registry to copy missing parents from, or ``None``. For a remote output, + ``None`` copies them from the remote output. build_parents: Build missing parents with their installed connectors. force: Build the release again, even if the local registry holds it. pinned_by: The releases that already read files of the release, with their pins. @@ -229,6 +232,9 @@ def _build_to( # ruff: ignore[too-many-arguments] return str(release.version) else: store, remote = LocalRegistry(settings().registry_path), target + # A build into the remote registry found the parents there. So without --fetch-from, the build + # copies a missing parent from the remote registry into the local one. + fetch_from = fetch_from if fetch_from is not None else remote # A remote registry holds only open releases. So for a remote output, the build refuses another # release before any download, as a build into that registry would. plan = build_release( diff --git a/packages/timenet-connectors/tests/test_build_plan.py b/packages/timenet-connectors/tests/test_build_plan.py index 37de30e74..768f799e5 100644 --- a/packages/timenet-connectors/tests/test_build_plan.py +++ b/packages/timenet-connectors/tests/test_build_plan.py @@ -870,6 +870,23 @@ def test_build_to_a_remote_out_builds_locally_and_copies(tmp_path, monkeypatch): assert not local.root.exists() +def test_build_to_a_remote_out_copies_a_missing_parent_from_that_registry(tmp_path, monkeypatch): + cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) + remote, local, calls = _remote_out(tmp_path, monkeypatch, cards) + parent = make_dataset() + parent._metadata = cards[PARENT.dataset_id] + remote.put_dataset(parent) + + result = runner.invoke(app, ["build", CHILD.dataset_id, "--out", "s3://bucket/registry"]) + + # Without --fetch-from, the build copies the parent from the remote --out, which holds it, into + # the local registry. So only the child builds. + assert result.exit_code == 0, result.output + assert calls == [(CHILD.dataset_id, False)] + assert local.exists(PARENT.dataset_id, str(PARENT.version)) + assert remote.exists(CHILD.dataset_id, str(CHILD.version)) + + def _files(root: Path) -> dict[Path, tuple[int, int]]: """Return the size and the modification time of each file under ``root``, by its path.""" return {path: (path.stat().st_size, path.stat().st_mtime_ns) for path in root.rglob("*") if path.is_file()} diff --git a/packages/timenet-connectors/tests/test_package_api.py b/packages/timenet-connectors/tests/test_package_api.py index a5a5d3eeb..64bfba9a6 100644 --- a/packages/timenet-connectors/tests/test_package_api.py +++ b/packages/timenet-connectors/tests/test_package_api.py @@ -44,15 +44,16 @@ def test_build_runs_in_an_isolated_environment_by_default(monkeypatch, tmp_path, monkeypatch.setattr(backend_module, "resolve", _unexpected_resolve) calls = [] - def fake_isolated(release, store_root, *, force=False, keep_cache=False, **_): - calls.append((release, store_root, force, keep_cache)) + def fake_isolated(release, store_root, *, force=False, keep_cache=False, quiet=False, **_): + calls.append((release, store_root, force, keep_cache, quiet)) LocalRegistry(store_root).put_dataset(make_dataset()) monkeypatch.setattr(backend_module, "run_isolated_build", fake_isolated) version_dir = timenet_connectors.build("timenet/hello-world", version=version, out=str(tmp_path), force=True) - assert calls == [(DatasetRef.parse("timenet/hello-world@1.0.0"), tmp_path, True, False)] + # The child is not quiet: it shows its downloads and shards. + assert calls == [(DatasetRef.parse("timenet/hello-world@1.0.0"), tmp_path, True, False, False)] assert version_dir == tmp_path / "timenet" / "hello-world" / "1.0.0" From 18547f4121b2e84f5ef26b3e735464e3a5065e1d Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 18:51:57 +0100 Subject: [PATCH 3/5] docs: shorten the docstrings and docs of the build command Say only what a caller of the build command and of build() needs to know. Part of TNET-157. --- .../src/timenet_connectors/api.py | 36 ++++++------------- .../src/timenet_connectors/builder/cli.py | 30 ++++++---------- 2 files changed, 21 insertions(+), 45 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/api.py b/packages/timenet-connectors/src/timenet_connectors/api.py index 4c329b84a..43be731aa 100644 --- a/packages/timenet-connectors/src/timenet_connectors/api.py +++ b/packages/timenet-connectors/src/timenet_connectors/api.py @@ -27,23 +27,14 @@ def build( # ruff: ignore[too-many-arguments] # ruff: ignore[docstring-extraneous-exception] (raised by build_release, not directly here) """Build a dataset release into a local registry by its dataset ID. - This is the producer-side one-liner over the build coordinator, - :func:`timenet.coordinator.build.build_release`. The build reuses a release that the registry - already holds, unless the caller sets ``force``. The default output is the shared local registry - (:func:`timenet.registry.default_registry_path`). The SDK reads from that directory, so a build - here loads at once with ``TimeNet().load(dataset_id)``. - - Like ``timenet-build build``, this runs the connector in an environment built from its - ``requirements.txt``. Set ``TIMENET_ISOLATION=off`` to run it in this interpreter instead, which - is what you want while writing a connector. Missing parent releases fail the build before any - download, unless ``fetch_from`` or ``build_parents`` obtains them. + The build reuses a release that the registry already holds, unless you set ``force``. The + connector runs in an environment built from its ``requirements.txt``, or in this interpreter if + ``TIMENET_ISOLATION=off``. Args: dataset_id: The dataset ID, exactly as the card of the connector declares it (``org/name``). - version: The expected dataset version. A connector builds only the version that its card - declares, so this value is a guard. If it is another version, the build stops before - anything runs. ``None`` builds the version that the card declares. - out: Output registry directory. Defaults to the shared local registry. + version: The version that the card must declare. ``None`` accepts any version. + out: The local registry directory. Defaults to the shared one that ``TimeNet`` reads. force: Build the release again, even if the registry already holds it. keep_cache: Keep the raw download cache. A successful build deletes it by default. fetch_from: The registry, or its URI, to copy missing parent releases from. @@ -54,10 +45,8 @@ def build( # ruff: ignore[too-many-arguments] Raises: TimeNetDatasetNotFoundError: If no installed connector builds ``dataset_id``. - TimeNetVersionUnavailableError: If the caller sets ``version`` and the card declares another - version. The error is also a :class:`~timenet.errors.TimeFValidationError`. - TimeNetMissingParentError: If a parent release is missing, with a hint that names the - keywords. + TimeNetVersionUnavailableError: If ``version`` is set and the card declares another version. + TimeNetMissingParentError: If a parent release is missing. Its hint names the two keywords. """ from timenet.builders import BuildOptions, declared # ruff: ignore[import-outside-top-level] from timenet.coordinator.build import build_release # ruff: ignore[import-outside-top-level] @@ -95,15 +84,12 @@ def load( ) -> "TimeFDataset": """Build a dataset if needed, then load it into memory. - This is the one-call sugar over :func:`build` plus :meth:`timenet.client.TimeNet.load`. For a - clear producer and consumer split, call :func:`build` and ``TimeNet().load`` yourself. + This runs :func:`build`, then :meth:`timenet.client.TimeNet.load`. Args: - dataset_id: The dataset id (``org/name``). - version: The version string, or ``None`` for the latest. :func:`build` checks this value - against the version that the card of the connector declares, and rejects another one. - fetch_from: The registry, or its URI, that the build and the read copy missing parent - releases and payload sources from. + 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, to copy missing parents and payload sources from. build_parents: Build missing parent releases from their installed connectors. Returns: diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index 84d38fb8e..65dd4e7dd 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -199,14 +199,12 @@ def _build_to( # ruff: ignore[too-many-arguments] ) -> str: """Build a release into a local output, or build it locally and copy it to a remote output. - If nothing runs, because the local output holds the release or the remote output holds the - version, this function says so on the console. + If nothing runs because the output already holds the release, a status line says so. Args: dataset_id: The dataset ID. target: The local directory, or the registry that is not local. - fetch_from: The registry to copy missing parents from, or ``None``. For a remote output, - ``None`` copies them from the remote output. + fetch_from: The registry to copy missing parents from. ``None`` uses a remote output, if any. build_parents: Build missing parents with their installed connectors. force: Build the release again, even if the local registry holds it. pinned_by: The releases that already read files of the release, with their pins. @@ -293,30 +291,22 @@ def build_one( # ruff: ignore[too-many-arguments] ) -> str: """Prepare a dataset's parents, then build or reuse its release. - The ``build()`` method of :class:`~timenet_connectors.builder.backend.ConnectorBuilder` calls - this function. If ``fetch_from`` or ``build_parents`` does not obtain a missing parent, the - build fails before any download. A parent builds the same way as this dataset. A parent never - builds with ``force``. If another builder backend claims a parent, the ``build`` method of that - backend builds the parent with its pins. If ``force`` is not set and the output already holds - the version, the build prepares only the missing payload sources of the version. In that case, - the build does not run the connector of this dataset. The connector runs in an environment - built from its requirements. If isolation is off, the connector runs in this interpreter. A - local output gets a version directory. A remote output gets a published version. + A missing parent comes from ``fetch_from`` or ``build_parents``, or the build fails before any + download. If the output already holds the version and ``force`` is not set, the build only + prepares the missing payload sources of the version. Args: dataset_id: The dataset ID. - output: A local directory or a ``timenet://``, ``http(s)://``, or ``s3://`` registry URL. + output: A local directory, or a ``timenet://``, ``http(s)://`` or ``s3://`` URL. fetch_from: The registry, or its URI, to copy missing parent releases from. build_parents: Build missing parents with installed connectors. - force: Rebuild an existing version. + force: Build the release again, even if the output already holds it. keep_cache: Keep the raw download cache after building. - isolation: If true, build in an environment built from the connector's requirements. - If ``None``, use the ``TIMENET_ISOLATION`` setting. - pinned_by: The releases that already read files of this release, with their pins. The build - publishes the release only if it reproduces each pinned file. + isolation: ``True`` builds in the environment of the connector. ``None`` reads ``TIMENET_ISOLATION``. + pinned_by: The pins that other releases hold on this release. The build must reproduce them. Returns: - The committed version directory for a local output, or the published version. + The version directory for a local output, or the published version. """ target = _resolve_target(str(output)) registry = LocalRegistry(target) if isinstance(target, Path) else target From b2a29dc559afd77e118882a311931439081c730e Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Thu, 5 Nov 2026 00:17:53 +0100 Subject: [PATCH 4/5] docs: shorten the docstrings of the build command Shorter docstrings for the code that this change adds or changes. Part of TNET-157. --- .../src/timenet_connectors/api.py | 29 +++++----- .../src/timenet_connectors/builder/cli.py | 53 +++++++++---------- packages/timenet/src/timenet/builders.py | 2 +- 3 files changed, 40 insertions(+), 44 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/api.py b/packages/timenet-connectors/src/timenet_connectors/api.py index 43be731aa..bb2458733 100644 --- a/packages/timenet-connectors/src/timenet_connectors/api.py +++ b/packages/timenet-connectors/src/timenet_connectors/api.py @@ -25,28 +25,27 @@ def build( # ruff: ignore[too-many-arguments] build_parents: bool = False, ) -> Path: # ruff: ignore[docstring-extraneous-exception] (raised by build_release, not directly here) - """Build a dataset release into a local registry by its dataset ID. + """Build a dataset release in a local registry. - The build reuses a release that the registry already holds, unless you set ``force``. The - connector runs in an environment built from its ``requirements.txt``, or in this interpreter if - ``TIMENET_ISOLATION=off``. + The build reuses an existing release unless you set ``force``. The connector runs in an environment + with its required packages, or in this interpreter if ``TIMENET_ISOLATION=off``. Args: - dataset_id: The dataset ID, exactly as the card of the connector declares it (``org/name``). - version: The version that the card must declare. ``None`` accepts any version. - out: The local registry directory. Defaults to the shared one that ``TimeNet`` reads. - force: Build the release again, even if the registry already holds it. - keep_cache: Keep the raw download cache. A successful build deletes it by default. + dataset_id: The connector's dataset ID (``org/name``). + version: The version to build, or ``None`` for the connector's declared version. + out: The output directory, or ``None`` to use the default local registry. + force: Rebuild the release even if it exists in the registry. + keep_cache: Keep downloaded source files after a successful build. fetch_from: The registry, or its URI, to copy missing parent releases from. build_parents: Build missing parent releases from their installed connectors. Returns: - The version directory of the release. + The release's version directory. Raises: TimeNetDatasetNotFoundError: If no installed connector builds ``dataset_id``. - TimeNetVersionUnavailableError: If ``version`` is set and the card declares another version. - TimeNetMissingParentError: If a parent release is missing. Its hint names the two keywords. + TimeNetVersionUnavailableError: If ``version`` differs from the connector's declared version. + TimeNetMissingParentError: If a required parent release is still missing. """ from timenet.builders import BuildOptions, declared # ruff: ignore[import-outside-top-level] from timenet.coordinator.build import build_release # ruff: ignore[import-outside-top-level] @@ -82,13 +81,11 @@ def load( fetch_from: "str | BaseRegistry | None" = None, build_parents: bool = False, ) -> "TimeFDataset": - """Build a dataset if needed, then load it into memory. - - This runs :func:`build`, then :meth:`timenet.client.TimeNet.load`. + """Build a dataset if needed and load it into memory. Args: dataset_id: The dataset ID (``org/name``). - version: The version to load, which the card must declare, or ``None`` for the latest. + 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. build_parents: Build missing parent releases from their installed connectors. diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index 65dd4e7dd..9a244dc82 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -197,24 +197,24 @@ def _build_to( # ruff: ignore[too-many-arguments] pinned_by: Mapping[DatasetRef, PayloadSource] | None, options: BuildOptions, ) -> str: - """Build a release into a local output, or build it locally and copy it to a remote output. + """Build a release locally and copy it to a remote output if needed. - If nothing runs because the output already holds the release, a status line says so. + The build reuses an existing release unless you set ``force``. Args: dataset_id: The dataset ID. - target: The local directory, or the registry that is not local. - fetch_from: The registry to copy missing parents from. ``None`` uses a remote output, if any. + target: The local output directory or remote registry. + fetch_from: The registry to copy missing parents from, or ``None`` to use the remote output if present. build_parents: Build missing parents with their installed connectors. - force: Build the release again, even if the local registry holds it. - pinned_by: The releases that already read files of the release, with their pins. - options: The options of each build. + force: Rebuild the release even if it exists in the local registry. + pinned_by: The file records that dependent releases require this build to match. + options: The settings for each build. Returns: - The version directory for a local output, or the version for a remote one. + The local version directory or remote version as a string. Raises: - BadParameter: If ``force`` is set and the remote output holds the version. Nothing runs then. + BadParameter: If ``force`` is set and the version exists in the remote output. """ release = declared(dataset_id).release if isinstance(target, Path): @@ -289,24 +289,23 @@ def build_one( # ruff: ignore[too-many-arguments] isolation: bool | None = None, pinned_by: Mapping[DatasetRef, PayloadSource] | None = None, ) -> str: - """Prepare a dataset's parents, then build or reuse its release. + """Prepare parent releases and build a dataset. - A missing parent comes from ``fetch_from`` or ``build_parents``, or the build fails before any - download. If the output already holds the version and ``force`` is not set, the build only - prepares the missing payload sources of the version. + A missing parent comes from ``fetch_from`` or ``build_parents``, or the build fails before any download. If the + output has the version and ``force`` is off, the build only prepares missing releases that supply its data files. Args: dataset_id: The dataset ID. output: A local directory, or a ``timenet://``, ``http(s)://`` or ``s3://`` URL. fetch_from: The registry, or its URI, to copy missing parent releases from. build_parents: Build missing parents with installed connectors. - force: Build the release again, even if the output already holds it. - keep_cache: Keep the raw download cache after building. - isolation: ``True`` builds in the environment of the connector. ``None`` reads ``TIMENET_ISOLATION``. - pinned_by: The pins that other releases hold on this release. The build must reproduce them. + force: Rebuild the release even if it exists in the output. + keep_cache: Keep downloaded source files after the build. + isolation: Use a separate environment if true, this process if false, or ``TIMENET_ISOLATION`` if ``None``. + pinned_by: The file records that dependent releases require this build to match. Returns: - The version directory for a local output, or the published version. + The local version directory or published version as a string. """ target = _resolve_target(str(output)) registry = LocalRegistry(target) if isinstance(target, Path) else target @@ -414,19 +413,19 @@ def _build( # ruff: ignore[too-many-arguments] keep_cache: bool = False, pinned_by: Mapping[DatasetRef, PayloadSource] | None = None, ) -> str: - """Build a dataset in a local directory or writable registry. + """Build a dataset in a local directory or remote registry. Args: - dataset_id: The dataset id. - target: The output for a build in the current interpreter. - forwarded: The output path or URL to pass to an isolated build. - isolated: Build in an environment built from the connector's requirements. - force: Rebuild an existing version. - keep_cache: Keep the raw download cache after building. - pinned_by: The releases that already read files of this release, with their pins. + dataset_id: The dataset ID. + target: The output directory or registry when building in this interpreter. + forwarded: The output path or URI when building in a separate environment. + isolated: Build in a separate environment with the connector's required packages. + force: Rebuild the release even if it exists in the output. + keep_cache: Keep downloaded source files after the build. + pinned_by: The file records that dependent releases require this build to match. Returns: - The committed version directory for a local output, or the published version. + The local version directory or published version as a string. """ pinned_by = pinned_by or None if isolated: diff --git a/packages/timenet/src/timenet/builders.py b/packages/timenet/src/timenet/builders.py index 936d373a1..2f479da66 100644 --- a/packages/timenet/src/timenet/builders.py +++ b/packages/timenet/src/timenet/builders.py @@ -34,7 +34,7 @@ class BuildOptions: 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. - isolation: Build in an environment of its own (``"on"``) or here (``"off"``); ``None`` keeps ``TIMENET_ISOLATION``. + isolation: Use a separate environment (``"on"``), this process (``"off"``), or ``TIMENET_ISOLATION`` (``None``). """ keep_cache: bool = False From e84e86f69b034a7c0b493038a1dba5c6b2f83009 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Thu, 5 Nov 2026 10:10:51 +0100 Subject: [PATCH 5/5] test: trim the tests of the build command Drop tests that cannot fail or repeat other tests, and sharpen the rest. Part of TNET-157. --- .../tests/test_build_cli.py | 9 ++---- .../tests/test_build_plan.py | 16 ++++------ .../tests/test_package_api.py | 32 ++++--------------- 3 files changed, 16 insertions(+), 41 deletions(-) diff --git a/packages/timenet-connectors/tests/test_build_cli.py b/packages/timenet-connectors/tests/test_build_cli.py index c20d09b64..d2568224e 100644 --- a/packages/timenet-connectors/tests/test_build_cli.py +++ b/packages/timenet-connectors/tests/test_build_cli.py @@ -54,7 +54,6 @@ def test_build_runs_isolated_by_default(monkeypatch, tmp_path): monkeypatch.delenv("TIMENET_ISOLATION", raising=False) calls = [] monkeypatch.setattr(backend_module, "run_isolated_build", _isolated_child(calls)) - monkeypatch.setattr(backend_module, "run_pipeline", _unexpected) result = runner.invoke(app, ["build", "timenet/hello-world", "--out", str(tmp_path)]) @@ -85,7 +84,6 @@ def test_build_isolates_when_the_flag_overrides_the_setting(monkeypatch, tmp_pat monkeypatch.setenv("TIMENET_ISOLATION", "off") calls = [] monkeypatch.setattr(backend_module, "run_isolated_build", _isolated_child(calls)) - monkeypatch.setattr(backend_module, "run_pipeline", _unexpected) result = runner.invoke(app, ["build", "timenet/hello-world", "--out", str(tmp_path), "--isolation"]) @@ -105,14 +103,13 @@ def test_build_without_isolation_shows_each_build_line_once(monkeypatch, tmp_pat assert result.stderr.count("✅ Built 'timenet/hello-world' → 1.0.0") == 1 -@pytest.mark.parametrize("quiet", [False, True]) -def test_a_build_that_reuses_the_release_says_so_on_stderr(monkeypatch, tmp_path, quiet): +def test_a_build_that_reuses_the_release_says_so_on_stderr(monkeypatch, tmp_path): monkeypatch.delenv("TIMENET_ISOLATION", raising=False) # The line names a long path, so the console must not wrap it. monkeypatch.setattr(cli_module.console, "rich", Console(stderr=True, width=1000)) calls = [] monkeypatch.setattr(backend_module, "run_isolated_build", _isolated_child(calls)) - command = [*(["--quiet"] if quiet else []), "build", "timenet/hello-world", "--out", str(tmp_path)] + command = ["build", "timenet/hello-world", "--out", str(tmp_path)] assert runner.invoke(app, command).exit_code == 0 result = runner.invoke(app, command) @@ -121,7 +118,7 @@ def test_a_build_that_reuses_the_release_says_so_on_stderr(monkeypatch, tmp_path assert len(calls) == 1 assert result.stdout == f"{tmp_path / 'timenet/hello-world/1.0.0'}\n" reused = f"✅ Nothing to do: 'timenet/hello-world' 1.0.0 is already in local registry {tmp_path}" - assert result.stderr.splitlines() == ([] if quiet else [reused]) + assert result.stderr.splitlines() == [reused] def test_build_takes_an_exact_release_and_refuses_another_version(monkeypatch, tmp_path): diff --git a/packages/timenet-connectors/tests/test_build_plan.py b/packages/timenet-connectors/tests/test_build_plan.py index 768f799e5..4df986233 100644 --- a/packages/timenet-connectors/tests/test_build_plan.py +++ b/packages/timenet-connectors/tests/test_build_plan.py @@ -1,6 +1,7 @@ from pathlib import Path import shutil +import click import numpy as np import pytest from typer.testing import CliRunner @@ -634,10 +635,8 @@ def child(command, env): return result.stdout, "", result.exit_code monkeypatch.delenv("TIMENET_ISOLATION", raising=False) - monkeypatch.setattr(env_module, "requirements_for", lambda dataset_id: None) monkeypatch.setattr(env_module, "_run_build", child) monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) - monkeypatch.setattr(backend_module, "resolve", lambda dataset_id: _Grand) monkeypatch.setattr(backend_module, "run_pipeline", run_pipeline) # The output holds PARENT, so the build gets only GRAND, the payload source of PARENT. @@ -822,11 +821,9 @@ def _remote_out(tmp_path, monkeypatch, cards) -> tuple[_Hosted, LocalRegistry, l return remote, LocalRegistry(tmp_path / "home/registry"), calls -@pytest.mark.parametrize("gated", ["child", "parent"]) -def test_build_to_a_remote_out_refuses_a_credentialed_release_before_any_download(tmp_path, monkeypatch, gated): +def test_build_to_a_remote_out_refuses_a_credentialed_release_before_any_download(tmp_path, monkeypatch): cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) - gated_ref = CHILD if gated == "child" else PARENT - cards[gated_ref.dataset_id] = cards[gated_ref.dataset_id].model_copy(update=_GATED) + cards[PARENT.dataset_id] = cards[PARENT.dataset_id].model_copy(update=_GATED) origin = LocalRegistry(tmp_path / "origin") origin.put_dataset(TimeFDataset(metadata=cards[PARENT.dataset_id])) _, local, calls = _remote_out(tmp_path, monkeypatch, cards) @@ -837,7 +834,7 @@ def test_build_to_a_remote_out_refuses_a_credentialed_release_before_any_downloa # The build into the local registry gets the access checks of the remote registry. assert isinstance(result.exception, TimeNetAccessError), result.output - assert result.exception.dataset == gated_ref + assert result.exception.dataset == PARENT assert calls == [] assert not local.exists(PARENT.dataset_id, str(PARENT.version)) @@ -903,8 +900,9 @@ def test_force_is_a_usage_error_if_the_remote_out_holds_the_version(tmp_path, mo # A published version never changes. So the command refuses before anything downloads or builds. assert result.exit_code == 2, result.output - assert "Invalid value for --force" in result.output + output = click.unstyle(result.output) + assert "Invalid value for --force" in output # The error box wraps the message and draws a border on each line. - assert "a published version never changes" in " ".join(result.output.replace("│", " ").split()) + assert "a published version never changes" in " ".join(output.replace("│", " ").split()) assert calls == [(CHILD.dataset_id, False)] assert _files(local.root) == held diff --git a/packages/timenet-connectors/tests/test_package_api.py b/packages/timenet-connectors/tests/test_package_api.py index 64bfba9a6..c9f53d388 100644 --- a/packages/timenet-connectors/tests/test_package_api.py +++ b/packages/timenet-connectors/tests/test_package_api.py @@ -1,6 +1,6 @@ import pytest -from timenet.errors import TimeNetDatasetNotFoundError, TimeNetVersionUnavailableError +from timenet.errors import TimeNetVersionUnavailableError from timenet.registry import LocalRegistry from timenet.testing import make_dataset from timenet.types import ClassificationTask @@ -24,6 +24,7 @@ def test_build_then_load_round_trip(monkeypatch, tmp_path): monkeypatch.setenv("TIMENET_REGISTRY", str(tmp_path / "registry")) # In-process, so the round trip stays in this interpreter instead of shelling out to uv. monkeypatch.setenv("TIMENET_ISOLATION", "off") + monkeypatch.setattr(backend_module, "run_isolated_build", _unexpected_isolation) version_dir = timenet_connectors.build("timenet/test-mean") assert (version_dir / "manifest.json").exists() @@ -36,10 +37,9 @@ def test_build_then_load_round_trip(monkeypatch, tmp_path): assert set(y.to_pylist()) == {"above_zero", "below_zero"} -@pytest.mark.parametrize("version", [None, "1.0.0"]) -def test_build_runs_in_an_isolated_environment_by_default(monkeypatch, tmp_path, version): - # The version check reads the card, so it does not import the connector either. The connector - # needs dependencies that only the environment of the child has. +def test_build_runs_in_an_isolated_environment_by_default(monkeypatch, tmp_path): + # The parent reads only the card. It does not import the connector, which needs dependencies + # that only the environment of the child has. monkeypatch.delenv("TIMENET_ISOLATION", raising=False) monkeypatch.setattr(backend_module, "resolve", _unexpected_resolve) calls = [] @@ -50,7 +50,7 @@ def fake_isolated(release, store_root, *, force=False, keep_cache=False, quiet=F monkeypatch.setattr(backend_module, "run_isolated_build", fake_isolated) - version_dir = timenet_connectors.build("timenet/hello-world", version=version, out=str(tmp_path), force=True) + version_dir = timenet_connectors.build("timenet/hello-world", out=str(tmp_path), force=True) # The child is not quiet: it shows its downloads and shards. assert calls == [(DatasetRef.parse("timenet/hello-world@1.0.0"), tmp_path, True, False, False)] @@ -69,23 +69,3 @@ def test_build_rejects_a_version_the_connector_does_not_declare(monkeypatch, tmp assert str(caught.value.declared) == "1.0.0" assert not tmp_path.joinpath("timenet").exists() - - -@pytest.mark.parametrize("dataset_id", ["nope/nothing", "timenet/hello_world"]) -def test_build_accepts_only_the_dataset_id_of_a_card(monkeypatch, tmp_path, dataset_id): - # The module path of the connector accepts the spelling with "_", but a dataset ID must match its card. - monkeypatch.setattr(backend_module, "run_isolated_build", _unexpected_isolation) - - with pytest.raises(TimeNetDatasetNotFoundError, match=f"no installed connector builds {dataset_id!r}"): - timenet_connectors.build(dataset_id, out=tmp_path) - - -def test_build_runs_in_process_when_isolation_is_off(monkeypatch, tmp_path): - monkeypatch.setenv("TIMENET_HOME", str(tmp_path / "home")) - monkeypatch.setenv("TIMENET_ISOLATION", "off") - monkeypatch.setattr(backend_module, "run_isolated_build", _unexpected_isolation) - - version_dir = timenet_connectors.build("timenet/hello-world", out=tmp_path / "registry") - - assert version_dir == tmp_path / "registry" / "timenet" / "hello-world" / "1.0.0" - assert (version_dir / "manifest.json").is_file()