From 0406776892a824a93c84c79007d1e42d28ec16e3 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Tue, 3 Nov 2026 23:48:20 +0100 Subject: [PATCH 1/8] feat(build): build one prepared release in the built-in backend ConnectorBuilder now implements build_prepared(). It builds exactly one release into a local store that already holds each parent of the release, and each payload source of such a parent. It does not plan. If the card of the connector declares another version than the release, it raises TimeFValidationError before anything runs. TIMENET_ISOLATION decides where the build runs, as for build(). With isolation on, run_isolated_build() in env.py runs the hidden command timenet-build build-release org/name@version --store PATH in the environment of the connector. It forwards --force, --keep-cache and the pins in a file. If the options have no progress callback, it also forwards --quiet. The child runs with TIMENET_ISOLATION=off. If the child fails, run_isolated_build() raises TimeNetBuildError with the tail of the stderr of the child. With isolation off, build_prepared() calls run_pipeline() with the values_backend of the connector and builds into the root of the store. It sends each download as a BytesMoved event and each finished shard as a ShardWritten event to the progress callback of the options. Without a progress callback, it installs no download sink. The build-release command calls build_prepared() of ConnectorBuilder. Unless --quiet is set, it shows the build lines, the shards and the downloads on the console. It prints nothing on stdout. A test makes sure that the command does not plan: with a parent missing from the store, it fails instead of building the parent. The bar display of the downloads moves into _download_bars(), so that the old download display and the new renderer share it. Nothing outside the tests calls build_prepared() yet. The build command, build() and run_isolated() work as before. Part of TNET-157. --- .../src/timenet_connectors/builder/backend.py | 128 +++++++++++++++++- .../src/timenet_connectors/builder/cli.py | 87 +++++++++++- .../src/timenet_connectors/builder/env.py | 51 ++++++- .../tests/test_build_backend.py | 115 +++++++++++++++- .../tests/test_build_cli.py | 108 ++++++++++++++- .../tests/test_build_env.py | 36 ++++- packages/timenet/src/timenet/progress.py | 32 ++++- 7 files changed, 540 insertions(+), 17 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py index 02b1251a7..15a751b54 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py @@ -1,14 +1,23 @@ """The build backend the SDK reaches through the ``timenet.builders`` entry point.""" from collections.abc import Mapping +from dataclasses import dataclass from pathlib import Path +from timenet.builders import BuildOptions +from timenet.config import settings +from timenet.engine import run_pipeline +from timenet.errors import TimeFValidationError, TimeNetDatasetNotFoundError from timenet.manifest import PayloadSource -from timenet.registry import BaseRegistry +from timenet.progress import BytesMoved, ProgressCallback, ShardWritten +from timenet.registry import BaseRegistry, LocalRegistry from timenet.types import DatasetMetadata from timenet.types.references import DatasetRef +from timenet.writer.progress import ProgressStage, WriteProgressEvent from timenet_connectors.builder.cli import build_one -from timenet_connectors.discovery import connector_dir, has_connector, known_ids +from timenet_connectors.builder.env import run_isolated_build +from timenet_connectors.discovery import connector_dir, has_connector, known_ids, resolve +from timenet_connectors.download import DownloadProgress, progress_sink class ConnectorBuilder: @@ -101,3 +110,118 @@ def build( # ruff: ignore[too-many-arguments, no-self-use] (protocol) pinned_by=pinned_by, ) ) + + def build_prepared( + self, + release: DatasetRef, + store: LocalRegistry, + *, + force: bool, + pinned_by: Mapping[DatasetRef, PayloadSource], + options: BuildOptions, + ) -> None: + """Build exactly this release into the store, in an isolated environment unless isolation is off. + + Each parent of the release, and each payload source of such a parent, must already be in + ``store``, because this method does not plan. By default, the hidden ``timenet-build build-release`` command builds + the release in an environment built from the requirements of the connector. That child runs + with ``TIMENET_ISOLATION=off``, so it builds in its own interpreter and starts no other child. + ``TIMENET_ISOLATION=off`` builds in this interpreter, with the ``values_backend`` of the + connector. + + Args: + release: The exact dataset ID and version to build. + store: The local registry to build into. + force: Build the release again, even if ``store`` already holds it. + pinned_by: The releases that already read files of this release, with their pins. The + build publishes the release only if it reproduces each pinned file. + options: The options of the build. An in-process build sends a + :class:`~timenet.progress.BytesMoved` event for the downloads and a + :class:`~timenet.progress.ShardWritten` event for each finished shard to + ``options.progress``. An isolated child prints its own progress, unless + ``options.progress`` is ``None``. + + Raises: + TimeNetDatasetNotFoundError: If no connector exists for the dataset. + TimeFValidationError: If the card of the connector declares another version than + ``release``. Nothing runs then. + TimeNetBuildError: If the isolated child fails. + """ # ruff: ignore[docstring-extraneous-exception] - TimeNetBuildError is raised by run_isolated_build + card = self.card(release.dataset_id) + if card is None: + raise TimeNetDatasetNotFoundError(f"no connector builds {release.dataset_id!r}") + if card.dataset_version != release.version: + raise TimeFValidationError( + f"connector for {release.dataset_id!r} builds version {card.dataset_version}, " + f"not the requested {release.version}" + ) + if settings().isolation == "on": + # Do not call resolve() here. It imports the connector, which needs the dependencies that + # only the environment of the child has. + run_isolated_build( + release, + store.root, + force=force, + keep_cache=options.keep_cache, + quiet=options.progress is None, + pinned_by=pinned_by, + ) + return + _build_in_process(release, store.root, force=force, pinned_by=pinned_by, options=options) + + +def _build_in_process( + release: DatasetRef, + root: Path, + *, + force: bool, + pinned_by: Mapping[DatasetRef, PayloadSource], + options: BuildOptions, +) -> None: + """Build the release in this interpreter, with the ``values_backend`` of its connector. + + Args: + release: The release to build. Its version is the version that the card declares. + root: The directory of the local registry to build into. + force: Build the release again, even if ``root`` already holds it. + pinned_by: The releases that already read files of this release, with their pins. + options: The options of the build. + """ + connector = resolve(release.dataset_id)() + relay = None if options.progress is None else _ProgressRelay(release, options.progress) + # A connector reports its downloads through the ambient progress sink. Without a sink, a + # downloader skips the work that it does only to report progress. + with progress_sink(None if relay is None else relay.download): + run_pipeline( + connector, + root, + progress_cb=None if relay is None else relay.write, + force=force, + keep_cache=options.keep_cache, + pinned_by=pinned_by, + ) + + +@dataclass(frozen=True) +class _ProgressRelay: + """Sends the download and writer progress of an in-process build as progress events of the build.""" + + release: DatasetRef + report: ProgressCallback + + def download(self, event: DownloadProgress) -> None: + """Send the progress of a download as a :class:`~timenet.progress.BytesMoved` event. + + Args: + event: The progress of the download. + """ + self.report(BytesMoved(event.url, event.downloaded, event.total)) + + def write(self, event: WriteProgressEvent) -> None: + """Send each finished shard as a :class:`~timenet.progress.ShardWritten` event. + + Args: + event: The progress of the writer. + """ + if event.stage is ProgressStage.SHARD_FINALIZED: + self.report(ShardWritten(self.release, event.completed)) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index f026c56f6..425eeaaa9 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -6,8 +6,9 @@ ``CONNECTOR`` and a ``dataset.yaml`` card. You do not register it here. """ -from collections.abc import Generator, Mapping +from collections.abc import Callable, Generator, Mapping from contextlib import contextmanager, nullcontext +from functools import partial import json from pathlib import Path import sys @@ -23,7 +24,7 @@ ) import typer -from timenet.builders import BuilderBackend, declared, find_card, installed_builder +from timenet.builders import BuilderBackend, BuildOptions, declared, find_card, installed_builder from timenet.cache import human_bytes from timenet.cli.runner import run_cli from timenet.cli.ui import console @@ -38,6 +39,7 @@ TimeNetRegistryError, ) from timenet.manifest import PayloadSource +from timenet.progress import BytesMoved, ProgressEvent, ShardWritten, StepFinished, StepStarted from timenet.registry import ( BaseRegistry, LocalRegistry, @@ -186,6 +188,36 @@ def build( typer.echo(result) +@app.command("build-release", hidden=True) +def build_release_command( + release: str, + store: Path = typer.Option(..., "--store", help="The local registry to build into."), + force: bool = typer.Option(False, "--force", help="Build the release again, even if the local registry holds it."), + keep_cache: bool = typer.Option(False, "--keep-cache", help="Keep the raw download cache after building."), + pinned_by: Path | None = typer.Option( + None, "--pinned-by", help="A JSON file of the releases that read files of this release, with their pins." + ), +) -> None: + """Build exactly one release into a local store that holds its parents and payload sources. + + An isolated build runs this command in the environment of the connector. The command does not + plan, because the parent process already put each parent and payload source into the store. It + prints the status lines and the progress of the build to stderr, and nothing to stdout. + """ + # backend.py imports this module, so a module-level import here would be circular. + from timenet_connectors.builder.backend import ConnectorBuilder # ruff: ignore[import-outside-top-level] + + reference = DatasetRef.parse(release) + pins = {} if pinned_by is None else _read_pins(pinned_by) + with _renderer() as progress: + if progress is not None: + progress(StepStarted(reference, "build")) + options = BuildOptions(keep_cache=keep_cache, progress=progress) + ConnectorBuilder().build_prepared(reference, LocalRegistry(store), force=force, pinned_by=pins, options=options) + if progress is not None: + progress(StepFinished(reference, "build")) + + def build_one( # ruff: ignore[too-many-arguments] dataset_id: str, output: str | Path, @@ -433,7 +465,54 @@ def _download_progress() -> Generator[Progress | None]: with progress_sink(_text_download_reporter()): yield None return - progress = Progress( + progress = _download_bars() + with progress, progress_sink(_bar_reporter(progress)): + yield progress + + +@contextmanager +def _renderer() -> Generator[Callable[[ProgressEvent], None] | None]: + """Show the progress events of a build on the console. + + Downloads show as live progress bars in an interactive terminal, else as throttled text lines. + + Yields: + The callback that shows each event, or ``None`` if the console is quiet. + """ + if console.quiet: + yield None + return + if not sys.stderr.isatty(): + yield partial(_render, _text_download_reporter()) + return + with _download_bars() as progress: + yield partial(_render, _bar_reporter(progress)) + + +def _render(report_download: ProgressCallback, event: ProgressEvent) -> None: + """Show one progress event of a build on the console. + + Args: + report_download: Shows the progress of one download. + event: The event to show. + """ + if isinstance(event, BytesMoved): + report_download(DownloadProgress(event.url, event.done, event.total)) + elif isinstance(event, ShardWritten): + console.status("💾", f"wrote shard {event.count}") + elif isinstance(event, StepStarted) and event.action == "build": + console.status("🔧", f"Building '{event.release.dataset_id}'…") + elif isinstance(event, StepFinished) and event.action == "build": + console.success(f"Built '{event.release.dataset_id}' → {event.release.version}") + + +def _download_bars() -> Progress: + """Make the live display that shows each download as a progress bar. + + Returns: + The display. It shows only inside a ``with`` block. + """ + return Progress( TextColumn("[bold blue]{task.fields[name]}", justify="right"), BarColumn(bar_width=None), "[progress.percentage]{task.percentage:>3.1f}%", @@ -445,8 +524,6 @@ def _download_progress() -> Generator[Progress | None]: TimeRemainingColumn(), console=console.rich, ) - with progress, progress_sink(_bar_reporter(progress)): - yield progress def _bar_reporter(progress: Progress) -> ProgressCallback: diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/env.py b/packages/timenet-connectors/src/timenet_connectors/builder/env.py index 23c1e7390..bb7744558 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/env.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/env.py @@ -2,7 +2,7 @@ The parent resolves the connector's ``requirements.txt`` without importing the connector (see :func:`timenet_connectors.discovery.requirements_for`). It layers that over a base pinned to the -parent's own ``timenet`` and ``timenet-connectors``, then runs ``timenet-build build`` in that +parent's own ``timenet`` and ``timenet-connectors``, then runs ``timenet-build`` in that environment. uv owns resolution and caching. A repeat build with an unchanged requirement set is a cache hit. """ @@ -153,6 +153,55 @@ def run_isolated( # ruff: ignore[too-many-arguments] return lines[-1] +def run_isolated_build( # ruff: ignore[too-many-arguments] + release: DatasetRef, + store_root: Path, + *, + force: bool = False, + keep_cache: bool = False, + quiet: bool = False, + pinned_by: Mapping[DatasetRef, PayloadSource] | None = None, +) -> None: + """Build one release into a local store, in the environment of its connector. + + The child runs the hidden ``timenet-build build-release`` command. It builds only this release + and does not plan: each parent and payload source of the release must already be in the store. + The child's stderr is streamed live and also captured, so a failed build's message carries the + child's own error. + + Args: + release: The exact dataset ID and version to build. + store_root: The directory of the local registry to build into. + force: Build the release again, even if the store already holds it. + keep_cache: Keep the raw download cache after building. + quiet: Suppress the child's status output. + pinned_by: The releases that already read files of this release, with their pins. The child + receives them in a file, and publishes the release only if it reproduces each pinned + file. + + Raises: + TimeNetBuildError: If the child failed. + LookupError: If no connector exists for the dataset ID. + """ # ruff: ignore[docstring-extraneous-exception] (raised by env_spec, not directly here) + # --quiet belongs to timenet-build, not to build-release, so it goes before the subcommand. + argv = ["timenet-build", *(["--quiet"] if quiet else []), "build-release", str(release)] + argv += ["--store", str(store_root), *(["--force"] if force else []), *(["--keep-cache"] if keep_cache else [])] + spec = env_spec(release.dataset_id) + # TIMENET_ISOLATION=off is the recursion guard: the child is this same CLI. + child_env = {**os.environ, "TIMENET_ISOLATION": "off"} + with _pins_file(pinned_by) as pins: + if pins is not None: + argv += ["--pinned-by", str(pins)] + command = uv_command(spec, argv) + _, stderr, returncode = _run_build(command, child_env) + if returncode != 0: + tail = "\n".join(stderr.splitlines()[-15:]).strip() + detail = f"\n{tail}" if tail else " The child produced no error output." + raise TimeNetBuildError( + f"building {release} failed with exit code {returncode}. Command: {shlex.join(command)}.{detail}" + ) + + @contextmanager def _pins_file(pinned_by: Mapping[DatasetRef, PayloadSource] | None) -> Generator[Path | None]: """Write the pins for an isolated child to a temporary JSON file, keyed by the reading release. diff --git a/packages/timenet-connectors/tests/test_build_backend.py b/packages/timenet-connectors/tests/test_build_backend.py index e17d31ab8..c0afd5826 100644 --- a/packages/timenet-connectors/tests/test_build_backend.py +++ b/packages/timenet-connectors/tests/test_build_backend.py @@ -4,24 +4,39 @@ import pytest -from timenet.builders import installed_builder -from timenet.errors import ConnectorCardWarning, TimeNetInvalidCardError +from timenet.builders import BuildOptions, installed_builder +from timenet.errors import ConnectorCardWarning, TimeFValidationError, TimeNetInvalidCardError from timenet.manifest import Manifest, PayloadSource from timenet.manifest.files import FilePart +from timenet.progress import BytesMoved, ShardWritten from timenet.reader import TimeFReader -from timenet.registry import open_local_version +from timenet.registry import LocalRegistry, open_local_version from timenet.types.references import DatasetRef from timenet.values_backends import ValuesBackend +from timenet.writer.progress import ProgressStage, WriteProgressEvent from timenet_connectors import datasets as datasets_module from timenet_connectors.builder import backend as backend_module, cli as cli_module from timenet_connectors.builder.backend import ConnectorBuilder from timenet_connectors.discovery import known_ids +from timenet_connectors.download.progress import DownloadProgress, current_sink, progress_sink, report_progress + + +HELLO = DatasetRef.parse("timenet/hello-world@1.0.0") def _unexpected_isolation(*args, **kwargs): raise AssertionError(f"run_isolated should not have run: {args} {kwargs}") +def _unexpected_pipeline(*args, **kwargs): + raise AssertionError(f"run_pipeline should not have run: {args} {kwargs}") + + +def _pins(): + pinned = FilePart(path="control.duckdb", checksum="sha256:" + "0" * 64, size=1) + return {DatasetRef.parse("test/reader@1.0.0"): PayloadSource(release=HELLO, files=(pinned,))} + + def test_knows_accepts_a_known_id(): assert ConnectorBuilder().knows("timenet/hello-world") @@ -140,3 +155,97 @@ def test_build_in_process_round_trips_the_connector_default_zarr_backend(tmp_pat dataset = reader.read() assert len(dataset.records) == 3 assert len(dataset.records[0].signals[0].to_numpy()) == 16 + + +def test_build_prepared_builds_in_process_into_the_store(tmp_path, monkeypatch): + monkeypatch.setenv("TIMENET_HOME", str(tmp_path / "home")) + monkeypatch.setenv("TIMENET_ISOLATION", "off") + monkeypatch.setattr(backend_module, "run_isolated_build", _unexpected_isolation) + store = LocalRegistry(tmp_path / "store") + + ConnectorBuilder().build_prepared(HELLO, store, force=False, pinned_by={}, options=BuildOptions()) + + assert store.exists("timenet/hello-world", "1.0.0") + + +@pytest.mark.parametrize(("progress", "quiet"), [(None, True), (print, False)]) +def test_build_prepared_runs_an_isolated_child_with_the_release_and_the_pins(tmp_path, monkeypatch, progress, quiet): + monkeypatch.delenv("TIMENET_ISOLATION", raising=False) + monkeypatch.setattr(backend_module, "run_pipeline", _unexpected_pipeline) + calls = [] + monkeypatch.setattr( + backend_module, "run_isolated_build", lambda release, root, **options: calls.append((release, root, options)) + ) + + ConnectorBuilder().build_prepared( + HELLO, + LocalRegistry(tmp_path), + force=True, + pinned_by=_pins(), + options=BuildOptions(keep_cache=True, progress=progress), + ) + + assert calls == [(HELLO, tmp_path, {"force": True, "keep_cache": True, "quiet": quiet, "pinned_by": _pins()})] + + +@pytest.mark.parametrize("isolation", ["on", "off"]) +def test_build_prepared_rejects_another_version(tmp_path, monkeypatch, isolation): + monkeypatch.setenv("TIMENET_ISOLATION", isolation) + monkeypatch.setattr(backend_module, "run_isolated_build", _unexpected_isolation) + monkeypatch.setattr(backend_module, "run_pipeline", _unexpected_pipeline) + release = DatasetRef.parse("timenet/hello-world@9.9.9") + + with pytest.raises(TimeFValidationError, match=r"builds version 1\.0\.0, not the requested 9\.9\.9"): + ConnectorBuilder().build_prepared( + release, LocalRegistry(tmp_path), force=False, pinned_by={}, options=BuildOptions() + ) + + +def test_build_prepared_reports_downloads_and_shards(tmp_path, monkeypatch): + class Connector: + pass + + monkeypatch.setenv("TIMENET_ISOLATION", "off") + monkeypatch.setattr(backend_module, "resolve", lambda dataset_id: Connector) + received = {} + + def pipeline(connector, root, *, progress_cb, **options): + received.update(root=root, **options) + report_progress(DownloadProgress("https://host/records.zip", 5, 10)) + progress_cb(WriteProgressEvent(ProgressStage.TIME_SERIES, 3, None)) + progress_cb(WriteProgressEvent(ProgressStage.SHARD_FINALIZED, 1, None)) + return root / "timenet/hello-world/1.0.0" + + monkeypatch.setattr(backend_module, "run_pipeline", pipeline) + events = [] + + ConnectorBuilder().build_prepared( + HELLO, + LocalRegistry(tmp_path), + force=True, + pinned_by=_pins(), + options=BuildOptions(keep_cache=True, progress=events.append), + ) + + assert events == [BytesMoved("https://host/records.zip", 5, 10), ShardWritten(HELLO, 1)] + expected = {"root": tmp_path, "force": True, "keep_cache": True, "pinned_by": _pins()} + assert received == expected + + +def test_build_prepared_without_progress_installs_no_sink(tmp_path, monkeypatch): + monkeypatch.setenv("TIMENET_ISOLATION", "off") + seen = {} + + def pipeline(connector, root, *, progress_cb, **options): + seen.update(progress_cb=progress_cb, sink=current_sink()) + return root / "timenet/hello-world/1.0.0" + + monkeypatch.setattr(backend_module, "run_pipeline", pipeline) + + # An outer sink must not reach the build either: the build installs its own, empty one. + with progress_sink(print): + ConnectorBuilder().build_prepared( + HELLO, LocalRegistry(tmp_path), force=False, pinned_by={}, options=BuildOptions(progress=None) + ) + + assert seen == {"progress_cb": None, "sink": None} diff --git a/packages/timenet-connectors/tests/test_build_cli.py b/packages/timenet-connectors/tests/test_build_cli.py index 93e4b8fc1..74ab8d66c 100644 --- a/packages/timenet-connectors/tests/test_build_cli.py +++ b/packages/timenet-connectors/tests/test_build_cli.py @@ -1,3 +1,4 @@ +import json import os from pathlib import Path import subprocess @@ -6,11 +7,18 @@ import pytest from typer.testing import CliRunner -from timenet.errors import TimeNetVersionUnavailableError -from timenet.manifest import Manifest +from timenet.connectors import BaseConnector +from timenet.errors import TimeNetDatasetNotFoundError, TimeNetVersionUnavailableError +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_connectors.builder import cli as cli_module +from timenet.testing import make_dataset +from timenet.types import DatasetMetadata, Version +from timenet.types.reference_keys import ParentAlias +from timenet.types.references import DatasetRef +from timenet_connectors.builder import backend as backend_module, cli as cli_module +from timenet_connectors.builder.backend import ConnectorBuilder from timenet_connectors.builder.cli import app @@ -183,3 +191,97 @@ def test_isolated_build_prints_the_version_dir_and_narrates_once(tmp_path): # The isolated child is the sole narrator: each status line appears exactly once. assert result.stderr.count("🔧 Building 'timenet/hello-world'…") == 1 assert result.stderr.count("✅ Built 'timenet/hello-world' → 1.0.0") == 1 + + +_PARENT = DatasetRef.parse("test/parent@1.0.0") + + +def _demo_card(dataset_id: str, **update) -> DatasetMetadata: + """Return the card of the demo dataset under ``dataset_id``, at version 1.0.0.""" + return make_dataset().metadata.model_copy( + update={"dataset_id": dataset_id, "dataset_version": Version(1, 0, 0), **update} + ) + + +class _Parent(BaseConnector[str]): + """Builds the demo dataset as test/parent.""" + + def metadata(self): + return _demo_card(_PARENT.dataset_id) + + def download(self, cache_dir): + return [] + + def convert(self, raw_refs): + dataset = make_dataset() + dataset._metadata = self.metadata() + return dataset + + +class _Child(_Parent): + """Builds the demo dataset as test/child, which declares test/parent as its parent.""" + + def metadata(self): + return _demo_card("test/child", parents={ParentAlias("parent"): _PARENT}) + + def compose(self, raw_refs, context): + return self.convert(raw_refs) + + +def test_the_build_release_command_does_not_plan(monkeypatch, tmp_path): + # The installed connectors build test/child and its parent test/parent, in this process. + connectors = {"test/child": _Child, _PARENT.dataset_id: _Parent} + monkeypatch.setattr(ConnectorBuilder, "dataset_ids", lambda self: list(connectors)) + monkeypatch.setattr(ConnectorBuilder, "card", lambda self, dataset_id: connectors[dataset_id]().metadata()) + monkeypatch.setattr(backend_module, "resolve", connectors.__getitem__) + monkeypatch.setenv("TIMENET_ISOLATION", "off") + monkeypatch.setenv("TIMENET_HOME", str(tmp_path / "home")) + monkeypatch.setattr(cli_module.console, "quiet", False) + + # The store lacks the parent. A planner would build it first. The command must fail on the + # missing parent instead. + result = runner.invoke(app, ["build-release", "test/child@1.0.0", "--store", str(tmp_path / "store")]) + + assert result.exit_code == 1 + assert isinstance(result.exception, TimeNetDatasetNotFoundError), result.exception + assert str(result.exception).startswith(f"complete release {_PARENT} is missing") + assert not (tmp_path / "store" / _PARENT.dataset_id).exists() + + +def test_the_build_release_command_builds_a_release_without_parents(monkeypatch, tmp_path): + monkeypatch.setenv("TIMENET_ISOLATION", "off") + monkeypatch.setenv("TIMENET_HOME", str(tmp_path / "home")) + monkeypatch.setattr(cli_module.console, "quiet", False) + + result = runner.invoke(app, ["build-release", "timenet/hello-world@1.0.0", "--store", str(tmp_path / "store")]) + + assert result.exit_code == 0, result.output + assert (tmp_path / "store/timenet/hello-world/1.0.0/manifest.json").is_file() + assert result.output.count("🔧 Building 'timenet/hello-world'…") == 1 + assert result.output.count("✅ Built 'timenet/hello-world' → 1.0.0") == 1 + assert result.stdout == "" + + +@pytest.mark.parametrize("quiet", [False, True]) +def test_the_build_release_command_passes_the_pins_and_the_options(monkeypatch, tmp_path, quiet): + release = DatasetRef.parse("timenet/hello-world@1.0.0") + pinned = PayloadSource(release=release, files=(FilePart(path="a.bin", checksum="sha256:" + "0" * 64, size=1),)) + pins_file = tmp_path / "pins.json" + pins_file.write_text(json.dumps({"test/reader@1.0.0": pinned.model_dump(mode="json")}), encoding="utf-8") + calls = [] + monkeypatch.setattr( + ConnectorBuilder, + "build_prepared", + lambda self, release, store, **kwargs: calls.append((release, store, kwargs)), + ) + monkeypatch.setattr(cli_module.console, "quiet", False) + options = ["--store", str(tmp_path), "--force", "--keep-cache", "--pinned-by", str(pins_file)] + + result = runner.invoke(app, [*(["--quiet"] if quiet else []), "build-release", str(release), *options]) + + assert result.exit_code == 0, result.output + ((built, store, kwargs),) = calls + assert (built, store.root, kwargs["force"]) == (release, tmp_path, True) + assert kwargs["pinned_by"] == {DatasetRef.parse("test/reader@1.0.0"): pinned} + assert kwargs["options"].keep_cache is True + assert (kwargs["options"].progress is None) is quiet diff --git a/packages/timenet-connectors/tests/test_build_env.py b/packages/timenet-connectors/tests/test_build_env.py index 014ed0d69..2823b899c 100644 --- a/packages/timenet-connectors/tests/test_build_env.py +++ b/packages/timenet-connectors/tests/test_build_env.py @@ -7,8 +7,11 @@ import pytest from timenet.errors import TimeNetBuildError +from timenet.manifest import PayloadSource +from timenet.manifest.files import FilePart +from timenet.types.references import DatasetRef from timenet_connectors.builder import env as env_module -from timenet_connectors.builder.env import EnvSpec, env_spec, run_isolated, uv_command +from timenet_connectors.builder.env import EnvSpec, env_spec, run_isolated, run_isolated_build, uv_command def _spec(requirements=None, base=("--with", "timenet==0.1.0")): @@ -228,3 +231,34 @@ def _missing(): with pytest.raises(TimeNetBuildError, match="uv"): uv_command(_spec(), ["timenet-build"]) + + +def test_run_isolated_build_runs_build_release_with_the_store_and_the_pins(monkeypatch, tmp_path): + release = DatasetRef.parse("timenet/hello-world@1.0.0") + pinned = PayloadSource(release=release, files=(FilePart(path="a.bin", checksum="sha256:" + "0" * 64, size=1),)) + captured = {} + + def run(command, env): + # The pins file exists only while the child runs. + pins = json.loads(Path(command[command.index("--pinned-by") + 1]).read_text(encoding="utf-8")) + captured.update(argv=command[command.index("timenet-build") :], env=env, pins=pins) + return "", "", 0 + + monkeypatch.setattr(env_module, "_run_build", run) + pins = {DatasetRef.parse("test/reader@1.0.0"): pinned} + + run_isolated_build(release, tmp_path / "store", force=True, keep_cache=True, quiet=True, pinned_by=pins) + + argv = captured["argv"] + assert argv[:4] == ["timenet-build", "--quiet", "build-release", "timenet/hello-world@1.0.0"] + assert argv[argv.index("--store") + 1] == str(tmp_path / "store") + assert {"--force", "--keep-cache"} <= set(argv) + assert captured["pins"] == {"test/reader@1.0.0": pinned.model_dump(mode="json")} + assert captured["env"]["TIMENET_ISOLATION"] == "off" + + +def test_run_isolated_build_raises_with_the_tail_of_the_child_stderr(monkeypatch, tmp_path): + monkeypatch.setattr(env_module, "_run_build", lambda command, env: ("", "boom: the disk is full", 1)) + + with pytest.raises(TimeNetBuildError, match=r"timenet/hello-world@1\.0\.0.*exit code 1(.|\n)*the disk is full"): + run_isolated_build(DatasetRef.parse("timenet/hello-world@1.0.0"), tmp_path) diff --git a/packages/timenet/src/timenet/progress.py b/packages/timenet/src/timenet/progress.py index 04a399754..de8dba9f3 100644 --- a/packages/timenet/src/timenet/progress.py +++ b/packages/timenet/src/timenet/progress.py @@ -37,8 +37,36 @@ class StepFinished: action: Literal["copy", "build"] -ProgressEvent = StepStarted | StepFinished -"""An event that marks the start or successful end of a copy or build.""" +@dataclass(frozen=True) +class BytesMoved: + """Bytes of a transfer moved. + + Attributes: + url: Where the bytes come from or go to. + done: The number of bytes moved so far. + total: The number of bytes of the whole transfer, or ``None`` if it is not known. + """ + + url: str + done: int + total: int | None + + +@dataclass(frozen=True) +class ShardWritten: + """A build wrote shards of a release. + + Attributes: + release: The release that the build writes. + count: The number of shards written so far. + """ + + release: DatasetRef + count: int + + +ProgressEvent = StepStarted | StepFinished | BytesMoved | ShardWritten +"""One progress event.""" ProgressCallback = Callable[[ProgressEvent], None] """A callback that receives a progress event and returns nothing.""" From bbae352329321e3257efbc0852102a2a0545adad Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 08:19:21 +0100 Subject: [PATCH 2/8] fix(build): silence the child only for BuildOptions.quiet ConnectorBuilder.build_prepared() now passes options.quiet to the isolated child. Before, it passed whether options.progress was None. So an SDK build, which sends no progress callback, ran the child with --quiet, and the user saw nothing of a long download. The hidden build-release command passes its own --quiet on in the options too. Another version than the card declares now raises TimeNetVersionUnavailableError, like build_release() does. It is a TimeFValidationError too, so a handler of the old error still catches it. The docstring of build_prepared() wraps near 100 columns again. Part of TNET-157. --- .../src/timenet_connectors/builder/backend.py | 27 +++++++++---------- .../src/timenet_connectors/builder/cli.py | 2 +- .../tests/test_build_backend.py | 18 ++++++++++--- .../tests/test_build_cli.py | 1 + 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py index 15a751b54..ce0b372e6 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py @@ -7,7 +7,7 @@ from timenet.builders import BuildOptions from timenet.config import settings from timenet.engine import run_pipeline -from timenet.errors import TimeFValidationError, TimeNetDatasetNotFoundError +from timenet.errors import TimeNetDatasetNotFoundError, TimeNetVersionUnavailableError from timenet.manifest import PayloadSource from timenet.progress import BytesMoved, ProgressCallback, ShardWritten from timenet.registry import BaseRegistry, LocalRegistry @@ -120,14 +120,14 @@ def build_prepared( pinned_by: Mapping[DatasetRef, PayloadSource], options: BuildOptions, ) -> None: - """Build exactly this release into the store, in an isolated environment unless isolation is off. + """Build exactly this release into the store, in an isolated environment by default. Each parent of the release, and each payload source of such a parent, must already be in - ``store``, because this method does not plan. By default, the hidden ``timenet-build build-release`` command builds - the release in an environment built from the requirements of the connector. That child runs - with ``TIMENET_ISOLATION=off``, so it builds in its own interpreter and starts no other child. - ``TIMENET_ISOLATION=off`` builds in this interpreter, with the ``values_backend`` of the - connector. + ``store``, because this method does not plan. By default, the hidden + ``timenet-build build-release`` command builds the release in an environment built from the + requirements of the connector. That child runs with ``TIMENET_ISOLATION=off``, so it builds + in its own interpreter and starts no other child. ``TIMENET_ISOLATION=off`` builds in this + interpreter, with the ``values_backend`` of the connector. Args: release: The exact dataset ID and version to build. @@ -139,22 +139,19 @@ def build_prepared( :class:`~timenet.progress.BytesMoved` event for the downloads and a :class:`~timenet.progress.ShardWritten` event for each finished shard to ``options.progress``. An isolated child prints its own progress, unless - ``options.progress`` is ``None``. + ``options.quiet`` is set. Raises: TimeNetDatasetNotFoundError: If no connector exists for the dataset. - TimeFValidationError: If the card of the connector declares another version than - ``release``. Nothing runs then. + TimeNetVersionUnavailableError: If the card of the connector declares another version + than ``release``. Nothing runs then. It is also a ``TimeFValidationError``. TimeNetBuildError: If the isolated child fails. """ # ruff: ignore[docstring-extraneous-exception] - TimeNetBuildError is raised by run_isolated_build card = self.card(release.dataset_id) if card is None: raise TimeNetDatasetNotFoundError(f"no connector builds {release.dataset_id!r}") if card.dataset_version != release.version: - raise TimeFValidationError( - f"connector for {release.dataset_id!r} builds version {card.dataset_version}, " - f"not the requested {release.version}" - ) + raise TimeNetVersionUnavailableError(release, card.dataset_version) if settings().isolation == "on": # Do not call resolve() here. It imports the connector, which needs the dependencies that # only the environment of the child has. @@ -163,7 +160,7 @@ def build_prepared( store.root, force=force, keep_cache=options.keep_cache, - quiet=options.progress is None, + quiet=options.quiet, pinned_by=pinned_by, ) return diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index 425eeaaa9..530517713 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -212,7 +212,7 @@ def build_release_command( with _renderer() as progress: if progress is not None: progress(StepStarted(reference, "build")) - options = BuildOptions(keep_cache=keep_cache, progress=progress) + options = BuildOptions(keep_cache=keep_cache, progress=progress, quiet=console.quiet) ConnectorBuilder().build_prepared(reference, LocalRegistry(store), force=force, pinned_by=pins, options=options) if progress is not None: progress(StepFinished(reference, "build")) diff --git a/packages/timenet-connectors/tests/test_build_backend.py b/packages/timenet-connectors/tests/test_build_backend.py index c0afd5826..f19b82041 100644 --- a/packages/timenet-connectors/tests/test_build_backend.py +++ b/packages/timenet-connectors/tests/test_build_backend.py @@ -5,7 +5,12 @@ import pytest from timenet.builders import BuildOptions, installed_builder -from timenet.errors import ConnectorCardWarning, TimeFValidationError, TimeNetInvalidCardError +from timenet.errors import ( + ConnectorCardWarning, + TimeFValidationError, + TimeNetInvalidCardError, + TimeNetVersionUnavailableError, +) from timenet.manifest import Manifest, PayloadSource from timenet.manifest.files import FilePart from timenet.progress import BytesMoved, ShardWritten @@ -168,7 +173,8 @@ def test_build_prepared_builds_in_process_into_the_store(tmp_path, monkeypatch): assert store.exists("timenet/hello-world", "1.0.0") -@pytest.mark.parametrize(("progress", "quiet"), [(None, True), (print, False)]) +# Only BuildOptions.quiet silences the child. An SDK build without a progress callback still prints. +@pytest.mark.parametrize(("progress", "quiet"), [(None, False), (print, False), (None, True)]) def test_build_prepared_runs_an_isolated_child_with_the_release_and_the_pins(tmp_path, monkeypatch, progress, quiet): monkeypatch.delenv("TIMENET_ISOLATION", raising=False) monkeypatch.setattr(backend_module, "run_pipeline", _unexpected_pipeline) @@ -182,7 +188,7 @@ def test_build_prepared_runs_an_isolated_child_with_the_release_and_the_pins(tmp LocalRegistry(tmp_path), force=True, pinned_by=_pins(), - options=BuildOptions(keep_cache=True, progress=progress), + options=BuildOptions(keep_cache=True, progress=progress, quiet=quiet), ) assert calls == [(HELLO, tmp_path, {"force": True, "keep_cache": True, "quiet": quiet, "pinned_by": _pins()})] @@ -195,11 +201,15 @@ def test_build_prepared_rejects_another_version(tmp_path, monkeypatch, isolation monkeypatch.setattr(backend_module, "run_pipeline", _unexpected_pipeline) release = DatasetRef.parse("timenet/hello-world@9.9.9") - with pytest.raises(TimeFValidationError, match=r"builds version 1\.0\.0, not the requested 9\.9\.9"): + with pytest.raises(TimeNetVersionUnavailableError) as caught: ConnectorBuilder().build_prepared( release, LocalRegistry(tmp_path), force=False, pinned_by={}, options=BuildOptions() ) + # It is a TimeFValidationError too, so a handler of the old error still catches it. + assert isinstance(caught.value, TimeFValidationError) + assert (caught.value.release, str(caught.value.declared)) == (release, "1.0.0") + def test_build_prepared_reports_downloads_and_shards(tmp_path, monkeypatch): class Connector: diff --git a/packages/timenet-connectors/tests/test_build_cli.py b/packages/timenet-connectors/tests/test_build_cli.py index 74ab8d66c..45a06df98 100644 --- a/packages/timenet-connectors/tests/test_build_cli.py +++ b/packages/timenet-connectors/tests/test_build_cli.py @@ -285,3 +285,4 @@ def test_the_build_release_command_passes_the_pins_and_the_options(monkeypatch, assert kwargs["pinned_by"] == {DatasetRef.parse("test/reader@1.0.0"): pinned} assert kwargs["options"].keep_cache is True assert (kwargs["options"].progress is None) is quiet + assert kwargs["options"].quiet is quiet From 67e5fa865d5d7589201a2058a911405401ae34da Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 14:15:11 +0100 Subject: [PATCH 3/8] docs(build): say what build_prepared() does, not who calls it The docstring now says what the method builds and when it needs an isolated environment. Part of TNET-157. --- .../src/timenet_connectors/builder/backend.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py index ce0b372e6..e336e765b 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py @@ -120,14 +120,12 @@ def build_prepared( pinned_by: Mapping[DatasetRef, PayloadSource], options: BuildOptions, ) -> None: - """Build exactly this release into the store, in an isolated environment by default. - - Each parent of the release, and each payload source of such a parent, must already be in - ``store``, because this method does not plan. By default, the hidden - ``timenet-build build-release`` command builds the release in an environment built from the - requirements of the connector. That child runs with ``TIMENET_ISOLATION=off``, so it builds - in its own interpreter and starts no other child. ``TIMENET_ISOLATION=off`` builds in this - interpreter, with the ``values_backend`` of the connector. + """Build exactly this release into the store. + + The store must already hold each parent of the release and their payload + sources. This method does not plan. The build runs in a separate environment + with the requirements of the connector, unless ``TIMENET_ISOLATION=off``, + which builds in this interpreter. Args: release: The exact dataset ID and version to build. From 537a8dd10bec39b7c6bdf025aebc1911e4fd02b3 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 14:27:08 +0100 Subject: [PATCH 4/8] refactor(build): name the child command build-prepared-release and explain the card-only read The hidden child command now has the same name as build_prepared(), as the review asked. The comment in build_prepared() now says why the parent reads only the card: the connector and its dependencies exist only in the environment of the child. Part of TNET-157. --- .../src/timenet_connectors/builder/backend.py | 3 +-- .../src/timenet_connectors/builder/cli.py | 2 +- .../src/timenet_connectors/builder/env.py | 6 +++--- .../timenet-connectors/tests/test_build_cli.py | 14 ++++++++------ .../timenet-connectors/tests/test_build_env.py | 2 +- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py index e336e765b..8b35c3864 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py @@ -151,8 +151,7 @@ def build_prepared( if card.dataset_version != release.version: raise TimeNetVersionUnavailableError(release, card.dataset_version) if settings().isolation == "on": - # Do not call resolve() here. It imports the connector, which needs the dependencies that - # only the environment of the child has. + # The parent reads only the card: the connector and its dependencies exist only in the child. run_isolated_build( release, store.root, diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index 530517713..f13a586fc 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -188,7 +188,7 @@ def build( typer.echo(result) -@app.command("build-release", hidden=True) +@app.command("build-prepared-release", hidden=True) def build_release_command( release: str, store: Path = typer.Option(..., "--store", help="The local registry to build into."), diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/env.py b/packages/timenet-connectors/src/timenet_connectors/builder/env.py index bb7744558..d3e0907c8 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/env.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/env.py @@ -164,7 +164,7 @@ def run_isolated_build( # ruff: ignore[too-many-arguments] ) -> None: """Build one release into a local store, in the environment of its connector. - The child runs the hidden ``timenet-build build-release`` command. It builds only this release + The child runs the hidden ``timenet-build build-prepared-release`` command. It builds only this release and does not plan: each parent and payload source of the release must already be in the store. The child's stderr is streamed live and also captured, so a failed build's message carries the child's own error. @@ -183,8 +183,8 @@ def run_isolated_build( # ruff: ignore[too-many-arguments] TimeNetBuildError: If the child failed. LookupError: If no connector exists for the dataset ID. """ # ruff: ignore[docstring-extraneous-exception] (raised by env_spec, not directly here) - # --quiet belongs to timenet-build, not to build-release, so it goes before the subcommand. - argv = ["timenet-build", *(["--quiet"] if quiet else []), "build-release", str(release)] + # --quiet belongs to timenet-build, not to build-prepared-release, so it goes before the subcommand. + argv = ["timenet-build", *(["--quiet"] if quiet else []), "build-prepared-release", str(release)] argv += ["--store", str(store_root), *(["--force"] if force else []), *(["--keep-cache"] if keep_cache else [])] spec = env_spec(release.dataset_id) # TIMENET_ISOLATION=off is the recursion guard: the child is this same CLI. diff --git a/packages/timenet-connectors/tests/test_build_cli.py b/packages/timenet-connectors/tests/test_build_cli.py index 45a06df98..ef000ab66 100644 --- a/packages/timenet-connectors/tests/test_build_cli.py +++ b/packages/timenet-connectors/tests/test_build_cli.py @@ -228,7 +228,7 @@ def compose(self, raw_refs, context): return self.convert(raw_refs) -def test_the_build_release_command_does_not_plan(monkeypatch, tmp_path): +def test_the_build_prepared_release_command_does_not_plan(monkeypatch, tmp_path): # The installed connectors build test/child and its parent test/parent, in this process. connectors = {"test/child": _Child, _PARENT.dataset_id: _Parent} monkeypatch.setattr(ConnectorBuilder, "dataset_ids", lambda self: list(connectors)) @@ -240,7 +240,7 @@ def test_the_build_release_command_does_not_plan(monkeypatch, tmp_path): # The store lacks the parent. A planner would build it first. The command must fail on the # missing parent instead. - result = runner.invoke(app, ["build-release", "test/child@1.0.0", "--store", str(tmp_path / "store")]) + result = runner.invoke(app, ["build-prepared-release", "test/child@1.0.0", "--store", str(tmp_path / "store")]) assert result.exit_code == 1 assert isinstance(result.exception, TimeNetDatasetNotFoundError), result.exception @@ -248,12 +248,14 @@ def test_the_build_release_command_does_not_plan(monkeypatch, tmp_path): assert not (tmp_path / "store" / _PARENT.dataset_id).exists() -def test_the_build_release_command_builds_a_release_without_parents(monkeypatch, tmp_path): +def test_the_build_prepared_release_command_builds_a_release_without_parents(monkeypatch, tmp_path): monkeypatch.setenv("TIMENET_ISOLATION", "off") monkeypatch.setenv("TIMENET_HOME", str(tmp_path / "home")) monkeypatch.setattr(cli_module.console, "quiet", False) - result = runner.invoke(app, ["build-release", "timenet/hello-world@1.0.0", "--store", str(tmp_path / "store")]) + result = runner.invoke( + app, ["build-prepared-release", "timenet/hello-world@1.0.0", "--store", str(tmp_path / "store")] + ) assert result.exit_code == 0, result.output assert (tmp_path / "store/timenet/hello-world/1.0.0/manifest.json").is_file() @@ -263,7 +265,7 @@ def test_the_build_release_command_builds_a_release_without_parents(monkeypatch, @pytest.mark.parametrize("quiet", [False, True]) -def test_the_build_release_command_passes_the_pins_and_the_options(monkeypatch, tmp_path, quiet): +def test_the_build_prepared_release_command_passes_the_pins_and_the_options(monkeypatch, tmp_path, quiet): release = DatasetRef.parse("timenet/hello-world@1.0.0") pinned = PayloadSource(release=release, files=(FilePart(path="a.bin", checksum="sha256:" + "0" * 64, size=1),)) pins_file = tmp_path / "pins.json" @@ -277,7 +279,7 @@ def test_the_build_release_command_passes_the_pins_and_the_options(monkeypatch, monkeypatch.setattr(cli_module.console, "quiet", False) options = ["--store", str(tmp_path), "--force", "--keep-cache", "--pinned-by", str(pins_file)] - result = runner.invoke(app, [*(["--quiet"] if quiet else []), "build-release", str(release), *options]) + result = runner.invoke(app, [*(["--quiet"] if quiet else []), "build-prepared-release", str(release), *options]) assert result.exit_code == 0, result.output ((built, store, kwargs),) = calls diff --git a/packages/timenet-connectors/tests/test_build_env.py b/packages/timenet-connectors/tests/test_build_env.py index 2823b899c..68a1737a0 100644 --- a/packages/timenet-connectors/tests/test_build_env.py +++ b/packages/timenet-connectors/tests/test_build_env.py @@ -250,7 +250,7 @@ def run(command, env): run_isolated_build(release, tmp_path / "store", force=True, keep_cache=True, quiet=True, pinned_by=pins) argv = captured["argv"] - assert argv[:4] == ["timenet-build", "--quiet", "build-release", "timenet/hello-world@1.0.0"] + assert argv[:4] == ["timenet-build", "--quiet", "build-prepared-release", "timenet/hello-world@1.0.0"] assert argv[argv.index("--store") + 1] == str(tmp_path / "store") assert {"--force", "--keep-cache"} <= set(argv) assert captured["pins"] == {"test/reader@1.0.0": pinned.model_dump(mode="json")} From e751c3ad1725d34f0f2c0f95267b52bc1912d124 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 18:04:03 +0100 Subject: [PATCH 5/8] docs: shorten the docstrings and docs of the prepared build Say only what a caller of the prepared build needs to know. Part of TNET-157. --- .../src/timenet_connectors/builder/backend.py | 24 +++++++++---------- .../src/timenet_connectors/builder/cli.py | 4 +--- .../src/timenet_connectors/builder/env.py | 15 +++++------- 3 files changed, 18 insertions(+), 25 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py index 8b35c3864..a10baac01 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py @@ -122,28 +122,26 @@ def build_prepared( ) -> None: """Build exactly this release into the store. - The store must already hold each parent of the release and their payload - sources. This method does not plan. The build runs in a separate environment - with the requirements of the connector, unless ``TIMENET_ISOLATION=off``, - which builds in this interpreter. + The store must already hold each parent of the release with its payload sources. The build + runs in an environment built from the requirements of the connector, or in this interpreter + if ``TIMENET_ISOLATION=off``. Args: release: The exact dataset ID and version to build. store: The local registry to build into. force: Build the release again, even if ``store`` already holds it. pinned_by: The releases that already read files of this release, with their pins. The - build publishes the release only if it reproduces each pinned file. - options: The options of the build. An in-process build sends a - :class:`~timenet.progress.BytesMoved` event for the downloads and a - :class:`~timenet.progress.ShardWritten` event for each finished shard to - ``options.progress``. An isolated child prints its own progress, unless - ``options.quiet`` is set. + build publishes the release only if it writes each pinned file again. + options: The options of the build. A build in this interpreter sends + :class:`~timenet.progress.BytesMoved` and :class:`~timenet.progress.ShardWritten` + events to ``options.progress``. An isolated build prints its own progress instead, + unless ``options.quiet`` is set. Raises: TimeNetDatasetNotFoundError: If no connector exists for the dataset. TimeNetVersionUnavailableError: If the card of the connector declares another version - than ``release``. Nothing runs then. It is also a ``TimeFValidationError``. - TimeNetBuildError: If the isolated child fails. + than ``release``. Nothing runs then. + TimeNetBuildError: If the isolated build fails. """ # ruff: ignore[docstring-extraneous-exception] - TimeNetBuildError is raised by run_isolated_build card = self.card(release.dataset_id) if card is None: @@ -172,7 +170,7 @@ def _build_in_process( pinned_by: Mapping[DatasetRef, PayloadSource], options: BuildOptions, ) -> None: - """Build the release in this interpreter, with the ``values_backend`` of its connector. + """Build the release in this interpreter. Args: release: The release to build. Its version is the version that the card declares. diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index f13a586fc..595b8806e 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -200,9 +200,7 @@ def build_release_command( ) -> None: """Build exactly one release into a local store that holds its parents and payload sources. - An isolated build runs this command in the environment of the connector. The command does not - plan, because the parent process already put each parent and payload source into the store. It - prints the status lines and the progress of the build to stderr, and nothing to stdout. + The command prints status lines and progress to stderr, and nothing to stdout. """ # backend.py imports this module, so a module-level import here would be circular. from timenet_connectors.builder.backend import ConnectorBuilder # ruff: ignore[import-outside-top-level] diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/env.py b/packages/timenet-connectors/src/timenet_connectors/builder/env.py index d3e0907c8..b8d0cbbbb 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/env.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/env.py @@ -164,23 +164,20 @@ def run_isolated_build( # ruff: ignore[too-many-arguments] ) -> None: """Build one release into a local store, in the environment of its connector. - The child runs the hidden ``timenet-build build-prepared-release`` command. It builds only this release - and does not plan: each parent and payload source of the release must already be in the store. - The child's stderr is streamed live and also captured, so a failed build's message carries the - child's own error. + The store must already hold each parent of the release with its payload sources. The error of a + failed build includes the last lines that the build wrote to stderr. Args: release: The exact dataset ID and version to build. store_root: The directory of the local registry to build into. force: Build the release again, even if the store already holds it. keep_cache: Keep the raw download cache after building. - quiet: Suppress the child's status output. - pinned_by: The releases that already read files of this release, with their pins. The child - receives them in a file, and publishes the release only if it reproduces each pinned - file. + quiet: Print no status output from the build. + pinned_by: The releases that already read files of this release, with their pins. The build + publishes the release only if it writes each pinned file again. Raises: - TimeNetBuildError: If the child failed. + TimeNetBuildError: If the build fails. LookupError: If no connector exists for the dataset ID. """ # ruff: ignore[docstring-extraneous-exception] (raised by env_spec, not directly here) # --quiet belongs to timenet-build, not to build-prepared-release, so it goes before the subcommand. From f06f63aba6427aefb2e1bbe06525ccdc07d0e021 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 22:48:36 +0100 Subject: [PATCH 6/8] refactor(build): share the child run and leave step lines to the parent The build-prepared-release child no longer sends StepStarted and StepFinished. The coordinator in the parent sends them around build_prepared(), so the child showed each step line a second time. The renderer loses its two step branches, and the command function is now build_prepared_release_command, after its command name. run_isolated() and run_isolated_build() now run the child through one private helper. run_isolated_build() takes force, keep_cache, quiet and pinned_by without defaults. Tests that repeated another test go away: the second stderr-tail test, the in-process backend build that the CLI test covers, a progress case that the child never sees, and the isolation case of the version check, which runs before isolation matters. The child-command test no longer patches the planner, because its missing-parent failure proves that the child does not plan. Part of TNET-157. --- .../src/timenet_connectors/builder/cli.py | 12 +-- .../src/timenet_connectors/builder/env.py | 91 ++++++++++++------- .../tests/test_build_backend.py | 22 +---- .../tests/test_build_cli.py | 2 - .../tests/test_build_env.py | 7 -- 5 files changed, 67 insertions(+), 67 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index 595b8806e..d9fbadf3f 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -39,7 +39,7 @@ TimeNetRegistryError, ) from timenet.manifest import PayloadSource -from timenet.progress import BytesMoved, ProgressEvent, ShardWritten, StepFinished, StepStarted +from timenet.progress import BytesMoved, ProgressEvent, ShardWritten from timenet.registry import ( BaseRegistry, LocalRegistry, @@ -189,7 +189,7 @@ def build( @app.command("build-prepared-release", hidden=True) -def build_release_command( +def build_prepared_release_command( release: str, store: Path = typer.Option(..., "--store", help="The local registry to build into."), force: bool = typer.Option(False, "--force", help="Build the release again, even if the local registry holds it."), @@ -208,12 +208,8 @@ def build_release_command( reference = DatasetRef.parse(release) pins = {} if pinned_by is None else _read_pins(pinned_by) with _renderer() as progress: - if progress is not None: - progress(StepStarted(reference, "build")) options = BuildOptions(keep_cache=keep_cache, progress=progress, quiet=console.quiet) ConnectorBuilder().build_prepared(reference, LocalRegistry(store), force=force, pinned_by=pins, options=options) - if progress is not None: - progress(StepFinished(reference, "build")) def build_one( # ruff: ignore[too-many-arguments] @@ -498,10 +494,6 @@ def _render(report_download: ProgressCallback, event: ProgressEvent) -> None: report_download(DownloadProgress(event.url, event.done, event.total)) elif isinstance(event, ShardWritten): console.status("💾", f"wrote shard {event.count}") - elif isinstance(event, StepStarted) and event.action == "build": - console.status("🔧", f"Building '{event.release.dataset_id}'…") - elif isinstance(event, StepFinished) and event.action == "build": - console.success(f"Built '{event.release.dataset_id}' → {event.release.version}") def _download_bars() -> Progress: diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/env.py b/packages/timenet-connectors/src/timenet_connectors/builder/env.py index b8d0cbbbb..ebbd3d2a8 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/env.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/env.py @@ -126,27 +126,16 @@ def run_isolated( # ruff: ignore[too-many-arguments] Raises: TimeNetBuildError: If the build fails or prints no result. LookupError: If no connector exists for the id. - """ # ruff: ignore[docstring-extraneous-exception] (raised by env_spec, not directly here) - # --quiet belongs to timenet-build, not to build, so it goes before the subcommand. - argv = ["timenet-build", *(["--quiet"] if quiet else []), "build", dataset_id, "--out", str(out)] - if force: - argv.append("--force") - if keep_cache: - argv.append("--keep-cache") - spec = env_spec(dataset_id) - # TIMENET_ISOLATION=off is the recursion guard: the child is this same CLI. - child_env = {**os.environ, "TIMENET_ISOLATION": "off"} - with _pins_file(pinned_by) as pins: - if pins is not None: - argv += ["--pinned-by", str(pins)] - command = uv_command(spec, argv) - stdout, stderr, returncode = _run_build(command, child_env) - if returncode != 0: - tail = "\n".join(stderr.splitlines()[-15:]).strip() - detail = f"\n{tail}" if tail else " The child produced no error output." - raise TimeNetBuildError( - f"building {dataset_id!r} failed with exit code {returncode}. Command: {shlex.join(command)}.{detail}" - ) + """ # ruff: ignore[docstring-extraneous-exception] (raised by _run_in_env, not directly here) + stdout = _run_in_env( + dataset_id, + ["build", dataset_id, "--out", str(out)], + force=force, + keep_cache=keep_cache, + quiet=quiet, + pinned_by=pinned_by, + subject=repr(dataset_id), + ) lines = [line.strip() for line in stdout.splitlines() if line.strip()] if not lines: raise TimeNetBuildError(f"building {dataset_id!r} printed no version") @@ -157,10 +146,10 @@ def run_isolated_build( # ruff: ignore[too-many-arguments] release: DatasetRef, store_root: Path, *, - force: bool = False, - keep_cache: bool = False, - quiet: bool = False, - pinned_by: Mapping[DatasetRef, PayloadSource] | None = None, + force: bool, + keep_cache: bool, + quiet: bool, + pinned_by: Mapping[DatasetRef, PayloadSource], ) -> None: """Build one release into a local store, in the environment of its connector. @@ -179,24 +168,64 @@ def run_isolated_build( # ruff: ignore[too-many-arguments] Raises: TimeNetBuildError: If the build fails. LookupError: If no connector exists for the dataset ID. + """ # ruff: ignore[docstring-extraneous-exception] (raised by _run_in_env, not directly here) + _run_in_env( + release.dataset_id, + ["build-prepared-release", str(release), "--store", str(store_root)], + force=force, + keep_cache=keep_cache, + quiet=quiet, + pinned_by=pinned_by, + subject=str(release), + ) + + +def _run_in_env( # ruff: ignore[too-many-arguments] + dataset_id: str, + subcommand: Sequence[str], + *, + force: bool, + keep_cache: bool, + quiet: bool, + pinned_by: Mapping[DatasetRef, PayloadSource] | None, + subject: str, +) -> str: + """Run a ``timenet-build`` subcommand in the environment of a connector. + + Args: + dataset_id: The dataset ID whose connector gives the environment. + subcommand: The subcommand with its arguments. The flags of the build go after it. + force: Add ``--force``. + keep_cache: Add ``--keep-cache``. + quiet: Add ``--quiet``. + pinned_by: The pins to give the child in a file, or ``None``. + subject: What the error of a failed build names. + + Returns: + What the child printed on stdout. + + Raises: + TimeNetBuildError: If the child fails. + LookupError: If no connector exists for the dataset ID. """ # ruff: ignore[docstring-extraneous-exception] (raised by env_spec, not directly here) - # --quiet belongs to timenet-build, not to build-prepared-release, so it goes before the subcommand. - argv = ["timenet-build", *(["--quiet"] if quiet else []), "build-prepared-release", str(release)] - argv += ["--store", str(store_root), *(["--force"] if force else []), *(["--keep-cache"] if keep_cache else [])] - spec = env_spec(release.dataset_id) + # --quiet belongs to timenet-build, not to the subcommand, so it goes first. + argv = ["timenet-build", *(["--quiet"] if quiet else []), *subcommand] + argv += [*(["--force"] if force else []), *(["--keep-cache"] if keep_cache else [])] + spec = env_spec(dataset_id) # TIMENET_ISOLATION=off is the recursion guard: the child is this same CLI. child_env = {**os.environ, "TIMENET_ISOLATION": "off"} with _pins_file(pinned_by) as pins: if pins is not None: argv += ["--pinned-by", str(pins)] command = uv_command(spec, argv) - _, stderr, returncode = _run_build(command, child_env) + stdout, stderr, returncode = _run_build(command, child_env) if returncode != 0: tail = "\n".join(stderr.splitlines()[-15:]).strip() detail = f"\n{tail}" if tail else " The child produced no error output." raise TimeNetBuildError( - f"building {release} failed with exit code {returncode}. Command: {shlex.join(command)}.{detail}" + f"building {subject} failed with exit code {returncode}. Command: {shlex.join(command)}.{detail}" ) + return stdout @contextmanager diff --git a/packages/timenet-connectors/tests/test_build_backend.py b/packages/timenet-connectors/tests/test_build_backend.py index f19b82041..29e13c5b0 100644 --- a/packages/timenet-connectors/tests/test_build_backend.py +++ b/packages/timenet-connectors/tests/test_build_backend.py @@ -162,20 +162,9 @@ def test_build_in_process_round_trips_the_connector_default_zarr_backend(tmp_pat assert len(dataset.records[0].signals[0].to_numpy()) == 16 -def test_build_prepared_builds_in_process_into_the_store(tmp_path, monkeypatch): - monkeypatch.setenv("TIMENET_HOME", str(tmp_path / "home")) - monkeypatch.setenv("TIMENET_ISOLATION", "off") - monkeypatch.setattr(backend_module, "run_isolated_build", _unexpected_isolation) - store = LocalRegistry(tmp_path / "store") - - ConnectorBuilder().build_prepared(HELLO, store, force=False, pinned_by={}, options=BuildOptions()) - - assert store.exists("timenet/hello-world", "1.0.0") - - # Only BuildOptions.quiet silences the child. An SDK build without a progress callback still prints. -@pytest.mark.parametrize(("progress", "quiet"), [(None, False), (print, False), (None, True)]) -def test_build_prepared_runs_an_isolated_child_with_the_release_and_the_pins(tmp_path, monkeypatch, progress, quiet): +@pytest.mark.parametrize("quiet", [False, True]) +def test_build_prepared_runs_an_isolated_child_with_the_release_and_the_pins(tmp_path, monkeypatch, quiet): monkeypatch.delenv("TIMENET_ISOLATION", raising=False) monkeypatch.setattr(backend_module, "run_pipeline", _unexpected_pipeline) calls = [] @@ -188,15 +177,14 @@ def test_build_prepared_runs_an_isolated_child_with_the_release_and_the_pins(tmp LocalRegistry(tmp_path), force=True, pinned_by=_pins(), - options=BuildOptions(keep_cache=True, progress=progress, quiet=quiet), + options=BuildOptions(keep_cache=True, quiet=quiet), ) assert calls == [(HELLO, tmp_path, {"force": True, "keep_cache": True, "quiet": quiet, "pinned_by": _pins()})] -@pytest.mark.parametrize("isolation", ["on", "off"]) -def test_build_prepared_rejects_another_version(tmp_path, monkeypatch, isolation): - monkeypatch.setenv("TIMENET_ISOLATION", isolation) +def test_build_prepared_rejects_another_version(tmp_path, monkeypatch): + monkeypatch.setenv("TIMENET_ISOLATION", "on") monkeypatch.setattr(backend_module, "run_isolated_build", _unexpected_isolation) monkeypatch.setattr(backend_module, "run_pipeline", _unexpected_pipeline) release = DatasetRef.parse("timenet/hello-world@9.9.9") diff --git a/packages/timenet-connectors/tests/test_build_cli.py b/packages/timenet-connectors/tests/test_build_cli.py index ef000ab66..15cfb60d2 100644 --- a/packages/timenet-connectors/tests/test_build_cli.py +++ b/packages/timenet-connectors/tests/test_build_cli.py @@ -259,8 +259,6 @@ def test_the_build_prepared_release_command_builds_a_release_without_parents(mon assert result.exit_code == 0, result.output assert (tmp_path / "store/timenet/hello-world/1.0.0/manifest.json").is_file() - assert result.output.count("🔧 Building 'timenet/hello-world'…") == 1 - assert result.output.count("✅ Built 'timenet/hello-world' → 1.0.0") == 1 assert result.stdout == "" diff --git a/packages/timenet-connectors/tests/test_build_env.py b/packages/timenet-connectors/tests/test_build_env.py index 68a1737a0..5c0061440 100644 --- a/packages/timenet-connectors/tests/test_build_env.py +++ b/packages/timenet-connectors/tests/test_build_env.py @@ -255,10 +255,3 @@ def run(command, env): assert {"--force", "--keep-cache"} <= set(argv) assert captured["pins"] == {"test/reader@1.0.0": pinned.model_dump(mode="json")} assert captured["env"]["TIMENET_ISOLATION"] == "off" - - -def test_run_isolated_build_raises_with_the_tail_of_the_child_stderr(monkeypatch, tmp_path): - monkeypatch.setattr(env_module, "_run_build", lambda command, env: ("", "boom: the disk is full", 1)) - - with pytest.raises(TimeNetBuildError, match=r"timenet/hello-world@1\.0\.0.*exit code 1(.|\n)*the disk is full"): - run_isolated_build(DatasetRef.parse("timenet/hello-world@1.0.0"), tmp_path) From c1d07e873815e04085b1d8658d14040cff3402bc Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Thu, 5 Nov 2026 00:14:20 +0100 Subject: [PATCH 7/8] docs: shorten the docstrings of the prepared build Shorter docstrings for the code that this change adds or changes. Part of TNET-157. --- .../src/timenet_connectors/builder/backend.py | 42 ++++++++--------- .../src/timenet_connectors/builder/cli.py | 29 ++++++------ .../src/timenet_connectors/builder/env.py | 45 +++++++++---------- .../src/timenet_connectors/download/s3.py | 4 +- packages/timenet/src/timenet/progress.py | 12 ++--- 5 files changed, 62 insertions(+), 70 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py index a10baac01..aad953d4b 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/backend.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/backend.py @@ -120,27 +120,21 @@ def build_prepared( pinned_by: Mapping[DatasetRef, PayloadSource], options: BuildOptions, ) -> None: - """Build exactly this release into the store. + """Build a release in a local registry. - The store must already hold each parent of the release with its payload sources. The build - runs in an environment built from the requirements of the connector, or in this interpreter - if ``TIMENET_ISOLATION=off``. + The registry must already contain all parent releases and their payload sources. + Set ``TIMENET_ISOLATION=off`` to build in this interpreter. Args: release: The exact dataset ID and version to build. store: The local registry to build into. - force: Build the release again, even if ``store`` already holds it. - pinned_by: The releases that already read files of this release, with their pins. The - build publishes the release only if it writes each pinned file again. - options: The options of the build. A build in this interpreter sends - :class:`~timenet.progress.BytesMoved` and :class:`~timenet.progress.ShardWritten` - events to ``options.progress``. An isolated build prints its own progress instead, - unless ``options.quiet`` is set. + force: Rebuild even if ``store`` already contains the release. + pinned_by: Releases that depend on this release, with file pins that the build must preserve. + options: Cache and output settings, with a progress callback for builds in this interpreter. Raises: TimeNetDatasetNotFoundError: If no connector exists for the dataset. - TimeNetVersionUnavailableError: If the card of the connector declares another version - than ``release``. Nothing runs then. + TimeNetVersionUnavailableError: If the requested version differs from the connector's card. TimeNetBuildError: If the isolated build fails. """ # ruff: ignore[docstring-extraneous-exception] - TimeNetBuildError is raised by run_isolated_build card = self.card(release.dataset_id) @@ -170,14 +164,14 @@ def _build_in_process( pinned_by: Mapping[DatasetRef, PayloadSource], options: BuildOptions, ) -> None: - """Build the release in this interpreter. + """Build a release in the current interpreter. Args: - release: The release to build. Its version is the version that the card declares. - root: The directory of the local registry to build into. - force: Build the release again, even if ``root`` already holds it. - pinned_by: The releases that already read files of this release, with their pins. - options: The options of the build. + release: The dataset ID and version declared in the connector's card. + root: The local registry directory to build into. + force: Rebuild even if ``root`` already contains the release. + pinned_by: Releases that depend on this release, with file pins that the build must preserve. + options: Cache settings and a progress callback. """ connector = resolve(release.dataset_id)() relay = None if options.progress is None else _ProgressRelay(release, options.progress) @@ -196,24 +190,24 @@ def _build_in_process( @dataclass(frozen=True) class _ProgressRelay: - """Sends the download and writer progress of an in-process build as progress events of the build.""" + """Convert download and write updates into build progress events.""" release: DatasetRef report: ProgressCallback def download(self, event: DownloadProgress) -> None: - """Send the progress of a download as a :class:`~timenet.progress.BytesMoved` event. + """Report downloaded bytes as a :class:`~timenet.progress.BytesMoved` event. Args: - event: The progress of the download. + event: The download progress update. """ self.report(BytesMoved(event.url, event.downloaded, event.total)) def write(self, event: WriteProgressEvent) -> None: - """Send each finished shard as a :class:`~timenet.progress.ShardWritten` event. + """Report a completed shard as a :class:`~timenet.progress.ShardWritten` event. Args: - event: The progress of the writer. + event: The writer progress update. """ if event.stage is ProgressStage.SHARD_FINALIZED: self.report(ShardWritten(self.release, event.completed)) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index d9fbadf3f..3e6f49490 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -198,9 +198,10 @@ def build_prepared_release_command( None, "--pinned-by", help="A JSON file of the releases that read files of this release, with their pins." ), ) -> None: - """Build exactly one release into a local store that holds its parents and payload sources. + """Build a release in a local registry. - The command prints status lines and progress to stderr, and nothing to stdout. + The registry must already contain all parent releases and their payload sources. + Status and progress go to stderr, and nothing goes to stdout. """ # backend.py imports this module, so a module-level import here would be circular. from timenet_connectors.builder.backend import ConnectorBuilder # ruff: ignore[import-outside-top-level] @@ -441,15 +442,13 @@ def _report_progress(event: WriteProgressEvent) -> None: @contextmanager def _download_progress() -> Generator[Progress | None]: - """Install the download-progress sink for a build and render it. + """Install a download-progress sink for the block and display what it reports. - In an interactive terminal, downloads show as live progress bars, one row per file, updating in - parallel. When output goes through a pipe or you set ``--quiet``, this falls back to throttled - text lines (or silence), so redirected logs stay readable. + Interactive terminals show live bars, and other outputs show periodic text updates. + Quiet mode hides progress. Yields: - The live progress display when rendering bars, else ``None``. This installs the sink for - the duration of the block. + The live progress display for the block, or ``None`` for text output or quiet mode. """ if console.quiet: with progress_sink(None): @@ -466,12 +465,12 @@ def _download_progress() -> Generator[Progress | None]: @contextmanager def _renderer() -> Generator[Callable[[ProgressEvent], None] | None]: - """Show the progress events of a build on the console. + """Provide a callback that displays build progress. - Downloads show as live progress bars in an interactive terminal, else as throttled text lines. + Interactive terminals show download bars, and other outputs show periodic text updates. Yields: - The callback that shows each event, or ``None`` if the console is quiet. + The progress callback for the block, or ``None`` in quiet mode. """ if console.quiet: yield None @@ -484,10 +483,10 @@ def _renderer() -> Generator[Callable[[ProgressEvent], None] | None]: def _render(report_download: ProgressCallback, event: ProgressEvent) -> None: - """Show one progress event of a build on the console. + """Display a download or completed shard update. Args: - report_download: Shows the progress of one download. + report_download: The callback that displays download progress. event: The event to show. """ if isinstance(event, BytesMoved): @@ -497,10 +496,10 @@ def _render(report_download: ProgressCallback, event: ProgressEvent) -> None: def _download_bars() -> Progress: - """Make the live display that shows each download as a progress bar. + """Create a live display of download progress bars. Returns: - The display. It shows only inside a ``with`` block. + The progress display to activate with a ``with`` block. """ return Progress( TextColumn("[bold blue]{task.fields[name]}", justify="right"), diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/env.py b/packages/timenet-connectors/src/timenet_connectors/builder/env.py index ebbd3d2a8..e18936337 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/env.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/env.py @@ -108,24 +108,24 @@ def run_isolated( # ruff: ignore[too-many-arguments] quiet: bool = False, pinned_by: Mapping[DatasetRef, PayloadSource] | None = None, ) -> str: - """Build a dataset in a separate environment with its connector's dependencies. + """Build a dataset in a separate environment. - The build streams status and errors to stderr. + The build forwards stderr as it runs and includes its last lines in build errors. Args: - dataset_id: The dataset id. - out: The output directory or a ``timenet://``, ``http(s)://``, or ``s3://`` registry URL. + dataset_id: The dataset ID. + out: A local directory or a ``timenet://``, ``http(s)://``, or ``s3://`` URL. force: Rebuild an existing version. keep_cache: Keep the raw download cache after building. - quiet: Suppress the build's status output. - pinned_by: Releases that read this dataset's files, with file pins the build must match. + quiet: Hide the build's status output. + pinned_by: File pins to preserve, grouped by dependent releases, or ``None``. Returns: - The version directory for a local output, or the published version for a remote output. + The local version directory or the published version for a remote target. Raises: TimeNetBuildError: If the build fails or prints no result. - LookupError: If no connector exists for the id. + LookupError: If no connector exists for the dataset ID. """ # ruff: ignore[docstring-extraneous-exception] (raised by _run_in_env, not directly here) stdout = _run_in_env( dataset_id, @@ -151,19 +151,18 @@ def run_isolated_build( # ruff: ignore[too-many-arguments] quiet: bool, pinned_by: Mapping[DatasetRef, PayloadSource], ) -> None: - """Build one release into a local store, in the environment of its connector. + """Build a specific release into a local registry in its connector's environment. - The store must already hold each parent of the release with its payload sources. The error of a - failed build includes the last lines that the build wrote to stderr. + The registry must already contain all parent releases and their payload sources. + Build errors include the last lines of stderr. Args: release: The exact dataset ID and version to build. - store_root: The directory of the local registry to build into. - force: Build the release again, even if the store already holds it. + store_root: The local registry directory to build into. + force: Rebuild even if the registry already contains the release. keep_cache: Keep the raw download cache after building. - quiet: Print no status output from the build. - pinned_by: The releases that already read files of this release, with their pins. The build - publishes the release only if it writes each pinned file again. + quiet: Hide the build's status output. + pinned_by: Releases that depend on this release, with file pins that the build must preserve. Raises: TimeNetBuildError: If the build fails. @@ -190,22 +189,22 @@ def _run_in_env( # ruff: ignore[too-many-arguments] pinned_by: Mapping[DatasetRef, PayloadSource] | None, subject: str, ) -> str: - """Run a ``timenet-build`` subcommand in the environment of a connector. + """Run a ``timenet-build`` subcommand in a connector's environment. Args: - dataset_id: The dataset ID whose connector gives the environment. - subcommand: The subcommand with its arguments. The flags of the build go after it. + dataset_id: The dataset ID that selects the connector's environment. + subcommand: The subcommand and its arguments, before the build flags. force: Add ``--force``. keep_cache: Add ``--keep-cache``. quiet: Add ``--quiet``. - pinned_by: The pins to give the child in a file, or ``None``. - subject: What the error of a failed build names. + pinned_by: The file pins to pass to the build, or ``None``. + subject: The name to include in build errors. Returns: - What the child printed on stdout. + The command's stdout. Raises: - TimeNetBuildError: If the child fails. + TimeNetBuildError: If the command fails. LookupError: If no connector exists for the dataset ID. """ # ruff: ignore[docstring-extraneous-exception] (raised by env_spec, not directly here) # --quiet belongs to timenet-build, not to the subcommand, so it goes first. diff --git a/packages/timenet-connectors/src/timenet_connectors/download/s3.py b/packages/timenet-connectors/src/timenet_connectors/download/s3.py index c23601a41..b7338a80e 100644 --- a/packages/timenet-connectors/src/timenet_connectors/download/s3.py +++ b/packages/timenet-connectors/src/timenet_connectors/download/s3.py @@ -71,8 +71,8 @@ def download_s3_object(s3_url: str, dest: Path, *, sha256: str | None = None) -> sha256: An optional SHA-256 hash in hexadecimal that the downloaded file must match. Raises: - TimeFValidationError: If the URL uses a scheme other than ``s3://`` or lacks a bucket or key. - TimeFFormatError: If the downloaded file does not match the supplied SHA-256 hash. + TimeFValidationError: If the URL is not an ``s3://`` URL with a bucket and key. + TimeFFormatError: If the downloaded file does not match ``sha256``. """ parsed = urlparse(s3_url) bucket, key = parsed.netloc, parsed.path.lstrip("/") diff --git a/packages/timenet/src/timenet/progress.py b/packages/timenet/src/timenet/progress.py index de8dba9f3..87fb8a9b6 100644 --- a/packages/timenet/src/timenet/progress.py +++ b/packages/timenet/src/timenet/progress.py @@ -39,12 +39,12 @@ class StepFinished: @dataclass(frozen=True) class BytesMoved: - """Bytes of a transfer moved. + """The byte count of a transfer in progress. Attributes: - url: Where the bytes come from or go to. + url: The source or destination URL. done: The number of bytes moved so far. - total: The number of bytes of the whole transfer, or ``None`` if it is not known. + total: The transfer size in bytes, or ``None`` if unknown. """ url: str @@ -54,10 +54,10 @@ class BytesMoved: @dataclass(frozen=True) class ShardWritten: - """A build wrote shards of a release. + """The number of completed shards in a release build. Attributes: - release: The release that the build writes. + release: The release being built. count: The number of shards written so far. """ @@ -66,7 +66,7 @@ class ShardWritten: ProgressEvent = StepStarted | StepFinished | BytesMoved | ShardWritten -"""One progress event.""" +"""A step, transfer, or shard progress event.""" ProgressCallback = Callable[[ProgressEvent], None] """A callback that receives a progress event and returns nothing.""" From 38a10b6fa053283b9a25ff5bed65ef7deeca96f9 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Thu, 5 Nov 2026 14:36:58 +0100 Subject: [PATCH 8/8] refactor(cli)!: share one progress renderer and name DownloadCallback The build CLI drew its own download bars and status lines, and turned each BytesMoved event back into a DownloadProgress event to reuse its old reporters. timenet.cli.ui.progress_renderer() now shows every progress event: step lines, a bar for each transfer on a terminal, and throttled lines elsewhere. The build CLI uses it, and the conversion back goes. Only the old in-process build still turns the downloads of a connector into BytesMoved events. timenet_connectors.download.ProgressCallback had the name of the callback type in timenet.progress, but takes another event. It is now DownloadCallback. BREAKING CHANGE: timenet_connectors.download.ProgressCallback is now DownloadCallback. Part of TNET-157. --- .../src/timenet_connectors/builder/cli.py | 158 ++---------------- .../timenet_connectors/download/__init__.py | 4 +- .../timenet_connectors/download/progress.py | 9 +- .../src/timenet_connectors/download/s3.py | 4 +- .../tests/test_build_progress.py | 50 +----- packages/timenet/src/timenet/cli/ui.py | 129 ++++++++++++++ packages/timenet/tests/test_ui.py | 102 ++++++++++- 7 files changed, 264 insertions(+), 192 deletions(-) diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index 3e6f49490..ce1e73d0a 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -6,28 +6,16 @@ ``CONNECTOR`` and a ``dataset.yaml`` card. You do not register it here. """ -from collections.abc import Callable, Generator, Mapping +from collections.abc import Generator, Mapping from contextlib import contextmanager, nullcontext -from functools import partial import json from pathlib import Path -import sys - -from rich.progress import ( - BarColumn, - DownloadColumn, - Progress, - TaskID, - TextColumn, - TimeRemainingColumn, - TransferSpeedColumn, -) + import typer from timenet.builders import BuilderBackend, BuildOptions, declared, find_card, installed_builder -from timenet.cache import human_bytes from timenet.cli.runner import run_cli -from timenet.cli.ui import console +from timenet.cli.ui import console, progress_renderer from timenet.config import settings from timenet.coordinator.plan import Step, prepare_parents from timenet.engine import publish_pipeline, run_pipeline @@ -39,7 +27,7 @@ TimeNetRegistryError, ) from timenet.manifest import PayloadSource -from timenet.progress import BytesMoved, ProgressEvent, ShardWritten +from timenet.progress import BytesMoved from timenet.registry import ( BaseRegistry, LocalRegistry, @@ -52,7 +40,7 @@ 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, ProgressCallback, progress_sink +from timenet_connectors.download import DownloadProgress, progress_sink app = typer.Typer(help="Build TimeNet datasets from connectors.", no_args_is_help=True) @@ -208,7 +196,7 @@ def build_prepared_release_command( reference = DatasetRef.parse(release) pins = {} if pinned_by is None else _read_pins(pinned_by) - with _renderer() as progress: + with progress_renderer() as progress: options = BuildOptions(keep_cache=keep_cache, progress=progress, quiet=console.quiet) ConnectorBuilder().build_prepared(reference, LocalRegistry(store), force=force, pinned_by=pins, options=options) @@ -441,131 +429,19 @@ def _report_progress(event: WriteProgressEvent) -> None: @contextmanager -def _download_progress() -> Generator[Progress | None]: - """Install a download-progress sink for the block and display what it reports. - - Interactive terminals show live bars, and other outputs show periodic text updates. - Quiet mode hides progress. - - Yields: - The live progress display for the block, or ``None`` for text output or quiet mode. - """ - if console.quiet: - with progress_sink(None): - yield None - return - if not sys.stderr.isatty(): - with progress_sink(_text_download_reporter()): - yield None - return - progress = _download_bars() - with progress, progress_sink(_bar_reporter(progress)): - yield progress - - -@contextmanager -def _renderer() -> Generator[Callable[[ProgressEvent], None] | None]: - """Provide a callback that displays build progress. - - Interactive terminals show download bars, and other outputs show periodic text updates. - - Yields: - The progress callback for the block, or ``None`` in quiet mode. - """ - if console.quiet: - yield None - return - if not sys.stderr.isatty(): - yield partial(_render, _text_download_reporter()) - return - with _download_bars() as progress: - yield partial(_render, _bar_reporter(progress)) - - -def _render(report_download: ProgressCallback, event: ProgressEvent) -> None: - """Display a download or completed shard update. - - Args: - report_download: The callback that displays download progress. - event: The event to show. - """ - if isinstance(event, BytesMoved): - report_download(DownloadProgress(event.url, event.done, event.total)) - elif isinstance(event, ShardWritten): - console.status("💾", f"wrote shard {event.count}") - - -def _download_bars() -> Progress: - """Create a live display of download progress bars. - - Returns: - The progress display to activate with a ``with`` block. - """ - return Progress( - TextColumn("[bold blue]{task.fields[name]}", justify="right"), - BarColumn(bar_width=None), - "[progress.percentage]{task.percentage:>3.1f}%", - "•", - DownloadColumn(), - "•", - TransferSpeedColumn(), - "•", - TimeRemainingColumn(), - console=console.rich, - ) - - -def _bar_reporter(progress: Progress) -> ProgressCallback: - """Build a download-progress sink that maps each URL to a live progress-bar task. - - The first event for a URL adds a task (keyed by URL so concurrent downloads stay on their own - row). Later events update its position. - - Args: - progress: The display to add and update per-file tasks on. - - Returns: - A callback for :func:`~timenet_connectors.download.progress_sink`. - """ - tasks: dict[str, TaskID] = {} - - def report(event: DownloadProgress) -> None: - task_id = tasks.get(event.url) - if task_id is None: - task_id = progress.add_task("download", name=event.url.rsplit("/", 1)[-1], total=event.total) - tasks[event.url] = task_id - progress.update(task_id, completed=event.downloaded, total=event.total) - - return report - - -_UNKNOWN_TOTAL_STEP_BYTES = 50 * 1024 * 1024 # progress cadence when the download size is unknown - - -def _text_download_reporter() -> ProgressCallback: - """Build a download-progress sink that prints throttled status lines to the console. +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 - Reports each file at roughly 20% steps, or every 50 MB when the total size is unknown. Keys each - report per URL so concurrent downloads do not interleave into a flood of lines. + def report(event: DownloadProgress) -> None: + render(BytesMoved(event.url, event.downloaded, event.total)) - Returns: - A callback for :func:`~timenet_connectors.download.progress_sink`. - """ - last: dict[str, int] = {} - - def report(event: DownloadProgress) -> None: - # ~20% steps when the size is known, else one line every 50 MB. - step = event.downloaded * 5 // event.total if event.total else event.downloaded // _UNKNOWN_TOTAL_STEP_BYTES - if last.get(event.url) == step: - return - last[event.url] = step - name = event.url.rsplit("/", 1)[-1] - if event.total: - console.status("⬇️", f"{name} {event.downloaded * 100 // event.total}% ({human_bytes(event.total)})") - else: - console.status("⬇️", f"{name} {human_bytes(event.downloaded)}") - - return report + with progress_sink(report): + yield def main() -> None: diff --git a/packages/timenet-connectors/src/timenet_connectors/download/__init__.py b/packages/timenet-connectors/src/timenet_connectors/download/__init__.py index 33b6431ed..cecc74d03 100644 --- a/packages/timenet-connectors/src/timenet_connectors/download/__init__.py +++ b/packages/timenet-connectors/src/timenet_connectors/download/__init__.py @@ -7,13 +7,13 @@ """ from timenet_connectors.download.fetch import Artifact, download_files, ensure_archive, find_dir_containing -from timenet_connectors.download.progress import DownloadProgress, ProgressCallback, progress_sink +from timenet_connectors.download.progress import DownloadCallback, DownloadProgress, progress_sink __all__ = [ "Artifact", + "DownloadCallback", "DownloadProgress", - "ProgressCallback", "download_files", "ensure_archive", "find_dir_containing", diff --git a/packages/timenet-connectors/src/timenet_connectors/download/progress.py b/packages/timenet-connectors/src/timenet_connectors/download/progress.py index a2798b541..df3a9e2bd 100644 --- a/packages/timenet-connectors/src/timenet_connectors/download/progress.py +++ b/packages/timenet-connectors/src/timenet_connectors/download/progress.py @@ -23,13 +23,14 @@ class DownloadProgress: total: int | None -ProgressCallback = Callable[[DownloadProgress], None] +DownloadCallback = Callable[[DownloadProgress], None] +"""A callback that receives download progress events.""" -_sink: ContextVar[ProgressCallback | None] = ContextVar("download_progress_sink", default=None) +_sink: ContextVar[DownloadCallback | None] = ContextVar("download_progress_sink", default=None) @contextmanager -def progress_sink(callback: ProgressCallback | None) -> Generator[None]: +def progress_sink(callback: DownloadCallback | None) -> Generator[None]: """Install ``callback`` as the download-progress sink for the duration of the block. Args: @@ -56,7 +57,7 @@ def report_progress(event: DownloadProgress) -> None: sink(event) -def current_sink() -> ProgressCallback | None: +def current_sink() -> DownloadCallback | None: """Return the installed download-progress sink, or ``None`` if none is installed. A downloader captures this on its own thread before handing a callback to a library that reports diff --git a/packages/timenet-connectors/src/timenet_connectors/download/s3.py b/packages/timenet-connectors/src/timenet_connectors/download/s3.py index b7338a80e..51e35892a 100644 --- a/packages/timenet-connectors/src/timenet_connectors/download/s3.py +++ b/packages/timenet-connectors/src/timenet_connectors/download/s3.py @@ -15,7 +15,7 @@ from urllib.parse import urlparse from timenet.errors import TimeFFormatError, TimeFValidationError, TimeNetBuildError -from timenet_connectors.download.progress import DownloadProgress, ProgressCallback, current_sink +from timenet_connectors.download.progress import DownloadCallback, DownloadProgress, current_sink _LOG = logging.getLogger(__name__) @@ -90,7 +90,7 @@ def download_s3_object(s3_url: str, dest: Path, *, sha256: str | None = None) -> total = client.head_object(Bucket=bucket, Key=key)["ContentLength"] transferred = 0 - def _on_bytes(count: int, report: ProgressCallback = sink) -> None: + def _on_bytes(count: int, report: DownloadCallback = sink) -> None: nonlocal transferred transferred += count report(DownloadProgress(s3_url, transferred, total)) diff --git a/packages/timenet-connectors/tests/test_build_progress.py b/packages/timenet-connectors/tests/test_build_progress.py index 8441afa4f..5df32d217 100644 --- a/packages/timenet-connectors/tests/test_build_progress.py +++ b/packages/timenet-connectors/tests/test_build_progress.py @@ -1,54 +1,20 @@ """Tests for the timenet-build download-progress rendering.""" +import io import sys -from rich.progress import Progress +from rich.console import Console from timenet.cli.ui import console -from timenet_connectors.builder.cli import _bar_reporter, _download_progress +from timenet_connectors.builder.cli import _download_progress from timenet_connectors.download.progress import DownloadProgress, report_progress -def test_bar_reporter_tracks_one_task_per_url(): - progress = Progress() - report = _bar_reporter(progress) - report(DownloadProgress("https://host/records.zip", 10, 100)) - report(DownloadProgress("https://host/records.zip", 60, 100)) # same URL -> same row, advanced - report(DownloadProgress("https://host/labels.csv", 5, 20)) # new URL -> its own row - - tasks = {task.fields["name"]: task for task in progress.tasks} - assert set(tasks) == {"records.zip", "labels.csv"} - assert (tasks["records.zip"].completed, tasks["records.zip"].total) == (60, 100) - assert (tasks["labels.csv"].completed, tasks["labels.csv"].total) == (5, 20) - - -def test_bar_reporter_handles_unknown_total(): - progress = Progress() - _bar_reporter(progress)(DownloadProgress("https://host/stream.bin", 4096, None)) - (task,) = progress.tasks - assert task.total is None # indeterminate bar, still tracked - assert task.completed == 4096 - - -def test_download_progress_is_silent_when_quiet(monkeypatch): - monkeypatch.setattr(console, "quiet", True) - with _download_progress() as progress: - assert progress is None # no display installed - - -def test_download_progress_falls_back_to_text_off_a_tty(monkeypatch): +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() as progress: - assert progress is None # throttled text lines, no bars - - -def test_download_progress_renders_bars_on_a_tty(monkeypatch): - monkeypatch.setattr(console, "quiet", False) - monkeypatch.setattr(sys.stderr, "isatty", lambda: True, raising=False) - with _download_progress() as progress: - assert isinstance(progress, Progress) + with _download_progress(): report_progress(DownloadProgress("https://host/a.zip", 3, 9)) # routed through the ambient sink - (task,) = progress.tasks - assert task.fields["name"] == "a.zip" - assert task.completed == 3 + assert captured.getvalue().splitlines() == ["⬇️ a.zip 33% (9 B)"] diff --git a/packages/timenet/src/timenet/cli/ui.py b/packages/timenet/src/timenet/cli/ui.py index b6f73bb6c..f2c8b0ffa 100644 --- a/packages/timenet/src/timenet/cli/ui.py +++ b/packages/timenet/src/timenet/cli/ui.py @@ -9,7 +9,24 @@ This avoids corruption of the display area. """ +from collections.abc import Callable, Generator +from contextlib import contextmanager +from functools import partial +import sys + from rich.console import Console +from rich.progress import ( + BarColumn, + DownloadColumn, + Progress, + TaskID, + TextColumn, + TimeRemainingColumn, + TransferSpeedColumn, +) + +from timenet.cache import human_bytes +from timenet.progress import BytesMoved, ProgressCallback, ProgressEvent, ShardWritten, StepStarted class _Console: @@ -42,3 +59,115 @@ def _emit(self, emoji: str, message: str, *, style: str | None = None, force: bo console = _Console() + + +@contextmanager +def progress_renderer() -> Generator[ProgressCallback | None]: + """Show progress events on the console for the block. + + Transfers appear as bars in a terminal or periodic status lines elsewhere. + Steps and completed shards appear as status lines. + + Yields: + The callback for the events, or ``None`` if the console is quiet. + """ + if console.quiet: + yield None + return + if not sys.stderr.isatty(): + yield partial(_render, _text_download_reporter()) + return + with _download_bars() as progress: + yield partial(_render, _bar_reporter(progress)) + + +_STEP_LINES = {"copy": ("📥", "Copying", "Copied"), "build": ("🔧", "Building", "Built")} +"""The start emoji, start verb, and completion verb for each action.""" + + +def _render(report_download: Callable[[BytesMoved], None], event: ProgressEvent) -> None: + """Show one progress event on the console. + + Args: + report_download: The callback that shows transfer progress. + event: The event to show. + """ + if isinstance(event, BytesMoved): + report_download(event) + elif isinstance(event, ShardWritten): + console.status("💾", f"wrote shard {event.count}") + else: + emoji, doing, done = _STEP_LINES[event.action] + if isinstance(event, StepStarted): + console.status(emoji, f"{doing} '{event.release.dataset_id}'…") + else: + console.success(f"{done} '{event.release.dataset_id}' → {event.release.version}") + + +def _download_bars() -> Progress: + """Create a live display of download progress bars. + + Returns: + The progress display to activate with a ``with`` block. + """ + return Progress( + TextColumn("[bold blue]{task.fields[name]}", justify="right"), + BarColumn(bar_width=None), + "[progress.percentage]{task.percentage:>3.1f}%", + "•", + DownloadColumn(), + "•", + TransferSpeedColumn(), + "•", + TimeRemainingColumn(), + console=console.rich, + ) + + +def _bar_reporter(progress: Progress) -> Callable[[BytesMoved], None]: + """Create a callback that displays a progress bar for each transfer URL. + + Args: + progress: The display that holds the bars. + + Returns: + A callback that adds or updates a bar for each event. + """ + tasks: dict[str, TaskID] = {} + + def report(event: BytesMoved) -> None: + task_id = tasks.get(event.url) + if task_id is None: + task_id = progress.add_task("download", name=event.url.rsplit("/", 1)[-1], total=event.total) + tasks[event.url] = task_id + progress.update(task_id, completed=event.done, total=event.total) + + return report + + +_UNKNOWN_TOTAL_STEP_BYTES = 50 * 1024 * 1024 # progress cadence when the download size is unknown + + +def _text_download_reporter() -> Callable[[BytesMoved], None]: + """Build a callback that shows throttled status lines for each transfer. + + It shows each transfer at about every 20%, or every 50 MB when the size is unknown. + + Returns: + The callback. + """ + last: dict[str, int] = {} + + def report(event: BytesMoved) -> None: + # ~20% steps when the size is known, else one line every 50 MB. + step = event.done * 5 // event.total if event.total else event.done // _UNKNOWN_TOTAL_STEP_BYTES + if last.get(event.url) == step: + return + last[event.url] = step + name = event.url.rsplit("/", 1)[-1] + if event.total: + console.status("⬇️", f"{name} {event.done * 100 // event.total}% ({human_bytes(event.total)})") + else: + console.status("⬇️", f"{name} {human_bytes(event.done)}") + + return report diff --git a/packages/timenet/tests/test_ui.py b/packages/timenet/tests/test_ui.py index 937e2b1d4..706e9e287 100644 --- a/packages/timenet/tests/test_ui.py +++ b/packages/timenet/tests/test_ui.py @@ -1,6 +1,17 @@ +import io +import sys + import pytest +from rich.console import Console +from rich.progress import Progress + +from timenet.cli import ui +from timenet.cli.ui import _bar_reporter, _Console, progress_renderer +from timenet.progress import BytesMoved, ShardWritten, StepFinished, StepStarted +from timenet.types.references import DatasetRef -from timenet.cli.ui import _Console + +HELLO = DatasetRef.parse("timenet/hello-world@1.0.0") @pytest.fixture @@ -8,6 +19,29 @@ def console(): return _Console() +@pytest.fixture +def lines(monkeypatch): + """Capture the status lines of the shared console, which is not quiet.""" + captured = io.StringIO() + monkeypatch.setattr(ui.console, "rich", Console(file=captured, width=200)) + monkeypatch.setattr(ui.console, "quiet", False) + return captured + + +@pytest.fixture +def bars(monkeypatch): + """Record each live display of download bars that the renderer opens.""" + opened = [] + make = ui._download_bars + + def record(): + opened.append(make()) + return opened[-1] + + monkeypatch.setattr(ui, "_download_bars", record) + return opened + + def test_status_and_success_go_to_stderr(console, capsys): console.status("📥", "downloading") console.success("built hello_world") @@ -28,3 +62,69 @@ def test_quiet_suppresses_status_but_not_errors(console, capsys): assert "done" not in captured.err assert "heads up" in captured.err # warnings/errors ignore quiet assert "boom" in captured.err + + +def test_bar_reporter_tracks_one_task_per_url(): + progress = Progress() + report = _bar_reporter(progress) + report(BytesMoved("https://host/records.zip", 10, 100)) + report(BytesMoved("https://host/records.zip", 60, 100)) # same URL -> same row, advanced + report(BytesMoved("https://host/labels.csv", 5, 20)) # new URL -> its own row + + tasks = {task.fields["name"]: task for task in progress.tasks} + assert set(tasks) == {"records.zip", "labels.csv"} + assert (tasks["records.zip"].completed, tasks["records.zip"].total) == (60, 100) + assert (tasks["labels.csv"].completed, tasks["labels.csv"].total) == (5, 20) + + +def test_bar_reporter_handles_unknown_total(): + progress = Progress() + _bar_reporter(progress)(BytesMoved("https://host/stream.bin", 4096, None)) + (task,) = progress.tasks + assert task.total is None # indeterminate bar, still tracked + assert task.completed == 4096 + + +def test_the_renderer_is_silent_when_quiet(monkeypatch, bars): + monkeypatch.setattr(ui.console, "quiet", True) + with progress_renderer() as progress: + assert progress is None + assert bars == [] + + +def test_the_renderer_falls_back_to_text_off_a_tty(monkeypatch, lines, bars): + monkeypatch.setattr(sys.stderr, "isatty", lambda: False, raising=False) + with progress_renderer() as progress: + assert progress is not None + progress(BytesMoved("https://host/a.zip", 3, 9)) + progress(ShardWritten(HELLO, 2)) + assert lines.getvalue().splitlines() == ["⬇️ a.zip 33% (9 B)", "💾 wrote shard 2"] + assert bars == [] + + +def test_the_renderer_shows_downloads_as_bars_on_a_tty(monkeypatch, bars): + monkeypatch.setattr(ui.console, "quiet", False) + monkeypatch.setattr(sys.stderr, "isatty", lambda: True, raising=False) + with progress_renderer() as progress: + assert progress is not None + progress(BytesMoved("https://host/a.zip", 3, 9)) + ((task,),) = [display.tasks for display in bars] + assert (task.fields["name"], task.completed) == ("a.zip", 3) + + +def test_the_renderer_shows_copy_and_build_steps(monkeypatch, lines, bars): + monkeypatch.setattr(sys.stderr, "isatty", lambda: True, raising=False) + with progress_renderer() as progress: + assert progress is not None + for action in ("copy", "build"): + progress(StepStarted(HELLO, action)) + progress(StepFinished(HELLO, action)) + + # The live display ends with an empty line. + assert [line for line in lines.getvalue().splitlines() if line] == [ + "📥 Copying 'timenet/hello-world'…", + "✅ Copied 'timenet/hello-world' → 1.0.0", + "🔧 Building 'timenet/hello-world'…", + "✅ Built 'timenet/hello-world' → 1.0.0", + ] + assert len(bars) == 1