From ffb992384965fdbf61766312fdf02f24714f0b45 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Wed, 4 Nov 2026 20:04:12 +0100 Subject: [PATCH 1/3] feat(build)!: build into the local registry, publish with --publish-to timenet-build build had its own flags for where a build lands and where its parents come from: --out, --fetch-from and --build-parents. The other entry points take the default list of sources instead, so the same build could need other inputs on the command line than in Python. The build now always lands in the local registry. Missing parents and payload sources come from the default list: the remote registry, then the installed connectors. --local-registry, --remote-registry and --auto-build/--no-auto-build set that list for one command. --publish-to URL|PATH publishes the release after the build with timenet.publish(), which gets the exact release and so reads no manifest to find it. The destination is not a source of parents. An empty --publish-to is an error, not the current directory. The command shows its plan on stderr before the first step, as a tree. --dry-run prints the tree to stdout and stops. It copies, builds and publishes nothing. --quiet hides the plan. The command plans with the same internal planner and the same whole list of sources as timenet.build(), so the dry run shows the plan that a run runs. The hints and the tests name the new flags. BREAKING CHANGE: timenet-build build drops --out, --fetch-from and --build-parents. The build lands in the local registry, and missing parents come from the default list. --publish-to publishes the release afterwards. --force with a --publish-to that holds the version rebuilds into the local registry, and the publish refuses other files. Part of TNET-157. --- examples/load_tsqa.py | 2 +- .../src/timenet_connectors/builder/cli.py | 358 ++++++++-------- .../physionet/ecg_qa_cot/connector.py | 3 +- .../tests/test_build_cli.py | 31 +- .../tests/test_build_env.py | 2 +- .../tests/test_build_plan.py | 390 ++++++++++++++---- .../tests/test_end_to_end.py | 41 +- packages/timenet/src/timenet/cli/app.py | 5 +- packages/timenet/src/timenet/cli/ui.py | 8 + packages/timenet/src/timenet/client.py | 2 +- packages/timenet/tests/test_cli.py | 2 +- packages/timenet/tests/test_client.py | 2 +- packages/timenet/tests/test_ui.py | 15 + 13 files changed, 577 insertions(+), 284 deletions(-) diff --git a/examples/load_tsqa.py b/examples/load_tsqa.py index 31e9a5763..c8b7aba1b 100644 --- a/examples/load_tsqa.py +++ b/examples/load_tsqa.py @@ -2,7 +2,7 @@ Build the dataset into a local registry, then run this script:: - uv run timenet-build build "chengsenwang/tsqa" --out .timenet-registry + uv run timenet-build build "chengsenwang/tsqa" --local-registry .timenet-registry uv run python examples/load_tsqa.py Pass a dataset ID and a registry path as arguments to load another dataset. diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index dc39a036a..e6a80dbbb 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -8,29 +8,26 @@ import json from pathlib import Path +import shlex +from typing import cast import typer +from timenet import publish from timenet.builders import BuildOptions, declared +from timenet.cli.options import AUTO_BUILD, LOCAL_REGISTRY, REMOTE_REGISTRY from timenet.cli.runner import run_cli from timenet.cli.ui import console, progress_renderer -from timenet.config import settings +from timenet.client import default_sources from timenet.coordinator.build import plan_release_build, run_plan +from timenet.coordinator.plan import BuildStep, Plan, Step from timenet.errors import ( TimeNetAccessError, TimeNetFileError, TimeNetMissingParentError, - TimeNetRegistryError, ) from timenet.manifest import PayloadSource -from timenet.registry import ( - BaseRegistry, - LocalRegistry, - WritableRegistry, - copy_release, - local_registry_path, - open_registry, -) +from timenet.registry import LocalRegistry, WritableRegistry, open_registry from timenet.registry._paths import release_dir # ruff: ignore[import-private-name] from timenet.sources import InstalledConnectors from timenet.types.references import DatasetRef, is_latest, split_ref @@ -47,49 +44,13 @@ def _root(quiet: bool = typer.Option(False, "--quiet", "-q", help="Suppress stat console.quiet = quiet -def _resolve_target(out: str | None) -> WritableRegistry | Path: - """Resolve a build output to a local directory or writable remote registry. - - Args: - out: The output URL or path, or ``None`` to use ``TIMENET_LOCAL_REGISTRY``. - - Returns: - A local directory ``Path`` or a remote ``WritableRegistry``. - - Raises: - TimeNetRegistryError: If ``out`` is blank or cannot be opened as a registry. - """ # ruff: ignore[docstring-extraneous-exception] - raised by open_registry - # TODO(adrian): Remove temporary glue which maps a missing --out onto the local registry of the - # default list until the build CLI takes the flags of the defaults. - if out is None: - return settings().local_registry_dir - try: - return local_registry_path(out, label="--out") - except TimeNetRegistryError: - return open_registry(out, label="--out") - - @app.command() # ruff: ignore[too-many-arguments, too-many-positional-arguments] (a typer command: one parameter per option) def build( release: str = typer.Argument(..., metavar="DATASET_ID[@VERSION]"), - out: str | None = typer.Option( - None, - "--out", - help="Output registry: a local directory or a timenet:// / http(s):// / s3:// URL " - "(default: the local registry, $TIMENET_LOCAL_REGISTRY).", - ), force: bool = typer.Option( False, "--force", "-f", help="Build the release again, even if the local registry holds it." ), - fetch_from: str | None = typer.Option( - None, - "--fetch-from", - help="Copy missing parent releases from this registry URI (default for a remote --out: that registry).", - ), - build_parents: bool = typer.Option( - False, "--build-parents", help="Build missing parent releases from their installed connectors." - ), keep_cache: bool = typer.Option( False, "--keep-cache", help="Keep the raw download cache after building (default: remove it)." ), @@ -98,171 +59,240 @@ def build( "--isolation/--no-isolation", help="Run the build in an environment built from the connector's requirements.", ), + publish_to: str | None = typer.Option( + None, + "--publish-to", + metavar="URL|PATH", + help="After the build, publish the release and its payload sources to this registry: a URL or a local path.", + ), + dry_run: bool = typer.Option(False, "--dry-run", help="Print the plan and stop."), + local_registry: str | None = LOCAL_REGISTRY, + remote_registry: str | None = REMOTE_REGISTRY, + auto_build: bool | None = AUTO_BUILD, ) -> None: """Run a connector through the engine and write or publish its dataset. - A local ``--out`` writes a dataset-layout directory and prints its path to stdout. A remote - ``--out`` (``timenet://`` / ``http(s)://`` / ``s3://``) gets the release in two steps. First, the - release builds into the default local registry. Then a copy publishes it with its payload - sources, and the command prints the version. If the remote registry already holds the version - and ``--force`` is not set, nothing builds and nothing is copied. Either way an emoji summary goes - to stderr, so scripts can capture stdout. A built version is reused unless you give ``--force``. - A published version never changes. So ``--force`` with a remote ``--out`` that holds the version - is a usage error, and nothing runs. + The build writes the release into the local registry, a dataset-layout directory, and prints its path to + stdout. With ``--publish-to``, the release gets to that registry in two steps. First, the release + builds into the local registry. Then a copy publishes it with its payload sources, and the command prints + the version. If that registry already holds the version and ``--force`` is not set, nothing builds + and nothing is copied. ``--dry-run`` prints the plan to stdout and runs nothing. Either way an + emoji summary goes to stderr, so scripts can capture stdout. A built version is reused unless you + give ``--force``. The build runs in an environment built from the connector's ``requirements.txt``. Pass ``--no-isolation`` (or set ``TIMENET_ISOLATION=off``) to run it in the current interpreter instead, which is what you want while writing a connector. - A dataset whose card declares ``parents`` needs those exact parent releases in the local - registry. ``--fetch-from`` names a registry to copy a missing parent from. For a remote - ``--out``, that registry is the default, because it holds the parents of its releases. - ``--build-parents`` builds a missing parent from its own connector. If neither option obtains a - missing parent, the build fails before any download. The build prepares every parent before the - dataset itself. It never builds a parent over a release that the local registry already holds. - If the local registry already holds the version, the build needs only the payload sources of - the version. It prepares them the same way as a parent. + A dataset whose card declares ``parents`` needs those exact parent releases in the local registry. A + parent that the local registry lacks comes from the default list of sources, which ``--local-registry``, + ``--remote-registry`` and ``--auto-build`` set. If no source provides a missing parent, the build + fails before any download. The build prepares every parent before the dataset itself. It never + builds a parent over a release that the local registry already holds. If the local registry already holds the + version, the build needs only the payload sources of the version. It prepares them the same way + as a parent. Raises: - BadParameter: If ``dataset_id`` has no known connector. Also if ``--force`` is set and the - remote ``--out`` holds the version. Nothing runs then. + BadParameter: If ``dataset_id`` has no known connector. TimeNetVersionUnavailableError: If the connector cannot build the requested version. TimeNetMissingParentError: If a parent is missing. The hint of the error names the flags. TimeNetFileError: If a copy of a parent in the local registry is damaged, or a copied file does not match its manifest record. The hint of the error names the flags. - TimeNetAccessError: If the output is remote and gets a release without access declared + TimeNetAccessError: If ``--publish-to`` is remote and gets a release without access declared ``open``. - TimeNetVersionConflictError: If the copy to the remote ``--out`` finds other files of the + TimeNetVersionConflictError: If the copy to ``--publish-to`` finds other files of the version, or of a payload source of it, in that registry. Nothing is copied then. - """ # ruff: ignore[docstring-extraneous-exception] - the copy raises TimeNetVersionConflictError + """ # ruff: ignore[docstring-extraneous-exception] - the publish raises TimeNetVersionConflictError # known_ids() imports no connector. So an unknown id fails before any environment exists. dataset_id, version = split_ref(release) known = known_ids() if dataset_id not in known: raise typer.BadParameter(f"no connector for {dataset_id!r}; known: {', '.join(known) or '(none)'}") - if not is_latest(version): - declared(dataset_id, version) + # A connector builds only the version of its card, so "latest" asks for that version. + wanted = declared(dataset_id, None if is_latest(version) else version).release + sources = default_sources(local_registry=local_registry, remote_registry=remote_registry, auto_build=auto_build) + # The default list starts with the local registry, which the build writes into. The planner leaves it out of + # the sources of the copies and builds. + store = cast("LocalRegistry", sources[0]) + dest = None if publish_to is None else open_registry(publish_to, label="--publish-to") + has_connectors = any(isinstance(source, InstalledConnectors) for source in sources) try: + if _already_published(wanted, dest, force=force): + console.success(f"Nothing to do: '{wanted.dataset_id}' {wanted.version} is already in {dest}") + if not dry_run: + typer.echo(str(wanted.version)) + return + # A destination that requires open access holds open releases only. So for such a destination, the + # plan refuses another release before any download. + plan = plan_release_build( + str(wanted), + store=store, + parents_from=sources, + force=force, + open_access_only=dest is not None and dest.requires_open_access, + ) + if dry_run: + lines = _plan_lines(plan) + if dest is not None: + lines.append(f"then publish {wanted} to {dest}") + elif not plan.steps: + console.success(f"Nothing to do: '{wanted.dataset_id}' {wanted.version} is already in {store}") + if lines: + typer.echo("\n".join(lines)) + return + _show_plan(wanted, plan) # The parent shows each copy and build step. An isolated child shows its own downloads and shards. with progress_renderer() as progress: - result = _build_to( - dataset_id, - _resolve_target(out), - fetch_from=None if fetch_from is None else open_registry(fetch_from), - build_parents=build_parents, - force=force, - options=BuildOptions( - keep_cache=keep_cache, - progress=progress, - quiet=console.quiet, - isolation=None if isolation is None else ("on" if isolation else "off"), - ), + options = BuildOptions( + keep_cache=keep_cache, + progress=progress, + quiet=console.quiet, + isolation=None if isolation is None else ("on" if isolation else "off"), ) + run_plan(plan, store, options=options) + if dest is not None: + publish(wanted, dest=dest, source=store, progress=progress) except TimeNetMissingParentError as error: - error.hint = _missing_hint(fetch_from=fetch_from, build_parents=build_parents) + error.hint = _missing_hint(has_connectors=has_connectors) raise except TimeNetFileError as error: + flags = _registry_flags(local_registry=local_registry, remote_registry=remote_registry) error.hint = ( - f"Fetch an intact copy with --fetch-from , or rebuild it with `timenet-build build " - f"{error.release.dataset_id} --force`. A rebuild changes its files, and the releases that " - "pin the old files then fail to open." + f"Copy an intact release into the local registry with `timenet download {error.release}{flags}`, or rebuild " + f"it with `timenet-build build {error.release.dataset_id} --force{flags}`. A rebuild changes its " + "files, and the releases that pin the old files then fail to open." ) raise except TimeNetAccessError as error: - error.hint = "A remote --out holds open releases only. Build it into a local --out instead." + error.hint = _access_hint(dest, has_connectors=has_connectors) raise - typer.echo(result) + if dest is None and not plan.steps: + console.success(f"Nothing to do: '{wanted.dataset_id}' {wanted.version} is already in {store}") + # A publish prints the version, and a local build prints the release directory. + typer.echo(str(wanted.version) if dest is not None else str(release_dir(store.root, wanted))) -def _missing_hint(*, fetch_from: str | None, build_parents: bool) -> str | None: - """Suggest flags to obtain a missing release. +def _missing_hint(*, has_connectors: bool) -> str: + """Say how a build gets a parent that no source of its list provides. Args: - fetch_from: The value of ``--fetch-from``, or ``None``. - build_parents: Whether ``--build-parents`` is set. + has_connectors: Whether the list of sources holds the installed connectors. Returns: - A sentence naming unused flags, or ``None`` if both are set. + The hint, a full sentence that names ``--auto-build`` too if the list has no connectors. """ - ways = [] - if fetch_from is None: - ways.append("fetch it with --fetch-from ") - if not build_parents: - ways.append("build it with --build-parents") - if not ways: - return None - hint = " or ".join(ways) - return f"{hint[0].upper()}{hint[1:]}." - - -def _build_to( # ruff: ignore[too-many-arguments] - dataset_id: str, - target: WritableRegistry | Path, - *, - fetch_from: BaseRegistry | None, - build_parents: bool, - force: bool, - options: BuildOptions, -) -> str: - """Build a release for a local or remote output. - - Remote registries receive a copy from the local registry. - The build skips an existing release unless ``force`` is set. + ways = ( + "copy it from a registry that holds it with --remote-registry , or publish it to such a registry " + "first with `timenet publish`." + ) + if not has_connectors: + return f"Build it from its installed connector with --auto-build, {ways}" + return f"{ways[0].upper()}{ways[1:]}" + + +def _access_hint(dest: WritableRegistry | None, *, has_connectors: bool) -> str | None: + """Say how a build gets a release that is not open. Args: - dataset_id: The dataset ID. - target: The local output directory or remote registry. - fetch_from: The registry for missing parents, or ``None`` to use the remote output if present. - build_parents: Build missing parents with installed connectors. - force: Rebuild an existing local release. - options: Options for each build. + dest: The registry that ``--publish-to`` names, or ``None``. + has_connectors: Whether the list of sources holds the installed connectors. Returns: - The local version directory or remote version string. + The hint as a full sentence, or ``None`` if no flag of the command helps. + """ + if dest is not None and dest.requires_open_access: + return ( + f"{dest} serves open releases only, so --publish-to takes open releases only. Build it into the " + "local registry without --publish-to instead." + ) + if not has_connectors: + return "Build it from its installed connector with --auto-build." + return None - Raises: - BadParameter: If ``force`` is set and the remote output already holds the version. + +def _registry_flags(*, local_registry: str | None, remote_registry: str | None) -> str: + """Repeat the registry flags that the command got, for a command in a hint. + + Args: + local_registry: The value of ``--local-registry``, or ``None``. + remote_registry: The value of ``--remote-registry``, or ``None``. + + Returns: + Each given flag with its quoted value and a space before it, or an empty string. + """ + flags = (("--local-registry", local_registry), ("--remote-registry", remote_registry)) + return "".join(f" {flag} {shlex.quote(value)}" for flag, value in flags if value is not None) + + +def _already_published(release: DatasetRef, dest: WritableRegistry | None, *, force: bool) -> bool: + """Check whether the destination of the publish holds the release, so that nothing needs to run. + + Args: + release: The release to build. + dest: The publish destination, or ``None``. + force: Whether the build runs again anyway. + + Returns: + ``True`` if ``dest`` holds the release and ``force`` is false. + """ + return dest is not None and not force and dest.exists(release.dataset_id, str(release.version)) + + +def _show_plan(release: DatasetRef, plan: Plan) -> None: + """Show the steps of a plan on the console before the first step runs. + + Args: + release: The requested release. + plan: The plan. """ - release = declared(dataset_id).release - if isinstance(target, Path): - store, remote = LocalRegistry(target), None - elif target.exists(release.dataset_id, str(release.version)): - if force: - raise typer.BadParameter( - f"the remote --out holds {release}, and a published version never changes. --force builds " - "a release again only for a local --out, or for a version that the remote --out does not hold.", - param_hint="--force", - ) - console.success(f"Nothing to do: '{release.dataset_id}' {release.version} is already in the remote --out") - return str(release.version) - else: - # TODO(adrian): Remove temporary glue which maps a remote --out onto a build into the - # local registry of the default list until --publish-to replaces a remote --out. - store, remote = LocalRegistry(settings().local_registry_dir), target - # A build into the remote registry found the parents there. So without --fetch-from, the build - # copies a missing parent from the remote registry into the local one. - fetch_from = fetch_from if fetch_from is not None else remote - # A remote registry holds only open releases. So for a remote output, the build refuses another - # release before any download, as a build into that registry would. - plan = plan_release_build( - dataset_id, - store=store, - # TODO(adrian): Remove temporary glue which maps --fetch-from and --build-parents onto a - # list of sources until the flags of the defaults replace them. - parents_from=[ - *([fetch_from] if fetch_from is not None else []), - *([InstalledConnectors()] if build_parents else []), - ], - force=force, - open_access_only=remote is not None, - ) - run_plan(plan, store, options=options) - if remote is not None: - copy_release(release, store, remote, progress=options.progress) - return str(release.version) if not plan.steps: - console.success(f"Nothing to do: '{release.dataset_id}' {release.version} is already in {store}") - return str(release_dir(store.root, release)) + return + console.status("📋", f"Plan for '{release.dataset_id}' {release.version}:") + for line in _plan_lines(plan): + # Three columns, the width of the emoji and its space, put the tree under the text above it. + console.detail(f" {line}") + + +_ACTIONS = {"read": "read from", "copy": "copy from", "build": "build with"} +"""The words of each action of a step in a line of a plan.""" + +_BRANCHES = {"├── ": "│ ", "└── ": " "} +"""For each branch of a tree line: what the lines below it show in its column.""" + + +def _plan_lines(plan: Plan) -> list[str]: + """Draw a plan as a tree, with the requested release at the top. + + Below each release come the releases that it needs. Each line names the release, the action and + the source, for example ``: copy from ``, or ``: see above`` the second + time. + + Args: + plan: The plan. + + Returns: + One line for each node of the tree, without line ends. + """ + steps = {step.release: step for step in plan.steps} + needed = {release for step in plan.steps for release in step.depends_on} + lines: list[str] = [] + drawn: set[DatasetRef] = set() + + def draw(step: Step, indent: str, branch: str) -> None: + if step.release in drawn: + lines.append(f"{indent}{branch}{step.release}: see above") + return + drawn.add(step.release) + source = InstalledConnectors() if isinstance(step, BuildStep) else step.registry + lines.append(f"{indent}{branch}{step.release}: {_ACTIONS[step.action]} {source}") + below = [steps[release] for release in step.depends_on if release in steps] + for number, child in enumerate(below, start=1): + draw(child, indent + _BRANCHES.get(branch, ""), "└── " if number == len(below) else "├── ") + + for step in reversed(plan.steps): + if step.release not in needed: + draw(step, "", "") + return lines @app.command("build-prepared-release", hidden=True) diff --git a/packages/timenet-connectors/src/timenet_connectors/datasets/physionet/ecg_qa_cot/connector.py b/packages/timenet-connectors/src/timenet_connectors/datasets/physionet/ecg_qa_cot/connector.py index db156305d..fe1301f5c 100644 --- a/packages/timenet-connectors/src/timenet_connectors/datasets/physionet/ecg_qa_cot/connector.py +++ b/packages/timenet-connectors/src/timenet_connectors/datasets/physionet/ecg_qa_cot/connector.py @@ -13,7 +13,8 @@ The connector imports the recordings and their patient metadata from the pinned PTB-XL TimeF parent. The per-template answer options come from the ``Jwoo5/ecg-qa`` GitHub repo. The precomputed CoT rows (question, answer, rationale, template) come from the OpenTSLM release, the only public -source for the rationales. Build the PTB-XL parent first, or pass ``--build-parents``. +source for the rationales. If the local registry lacks the PTB-XL parent, a build copies it from the remote +registry or builds it from its connector first. The clinical context comes from the annotations of the imported PTB-XL records. It states the patient's age and sex, the recording device, and the signal-quality issues, extra beats, and pacemaker that PTB-XL diff --git a/packages/timenet-connectors/tests/test_build_cli.py b/packages/timenet-connectors/tests/test_build_cli.py index 808b0b55e..2e087451b 100644 --- a/packages/timenet-connectors/tests/test_build_cli.py +++ b/packages/timenet-connectors/tests/test_build_cli.py @@ -54,7 +54,7 @@ def test_build_runs_isolated_by_default(monkeypatch, tmp_path): calls = [] monkeypatch.setattr(backend_module, "run_isolated_build", _isolated_child(calls)) - result = runner.invoke(app, ["build", "timenet/hello-world", "--out", str(tmp_path)]) + result = runner.invoke(app, ["build", "timenet/hello-world", "--local-registry", str(tmp_path)]) assert result.exit_code == 0, result.output assert [(call["release"], call["store_root"]) for call in calls] == [(HELLO, tmp_path)] @@ -70,7 +70,14 @@ def test_build_runs_in_process_without_isolation(monkeypatch, tmp_path, how): monkeypatch.setenv("TIMENET_ISOLATION", "off") result = runner.invoke( - app, ["build", "timenet/hello-world", "--out", str(tmp_path), *(["--no-isolation"] if how == "flag" else [])] + app, + [ + "build", + "timenet/hello-world", + "--local-registry", + str(tmp_path), + *(["--no-isolation"] if how == "flag" else []), + ], ) assert result.exit_code == 0, result.output @@ -82,7 +89,7 @@ def test_build_isolates_when_the_flag_overrides_the_setting(monkeypatch, tmp_pat calls = [] monkeypatch.setattr(backend_module, "run_isolated_build", _isolated_child(calls)) - result = runner.invoke(app, ["build", "timenet/hello-world", "--out", str(tmp_path), "--isolation"]) + result = runner.invoke(app, ["build", "timenet/hello-world", "--local-registry", str(tmp_path), "--isolation"]) assert result.exit_code == 0, result.output assert len(calls) == 1 @@ -92,7 +99,9 @@ def test_build_without_isolation_shows_each_build_line_once(tmp_path): # In this process, the coordinator sends the build step, and the renderer shows it. Nothing else # prints a build line. - result = runner.invoke(app, ["build", "timenet/hello-world", "--out", str(tmp_path / "out"), "--no-isolation"]) + result = runner.invoke( + app, ["build", "timenet/hello-world", "--local-registry", str(tmp_path / "out"), "--no-isolation"] + ) assert result.exit_code == 0, result.output assert result.stderr.count("🔧 Building 'timenet/hello-world'…") == 1 @@ -104,7 +113,7 @@ def test_a_build_that_reuses_the_release_says_so_on_stderr(monkeypatch, tmp_path monkeypatch.setattr(cli_module.console, "rich", Console(stderr=True, width=1000)) calls = [] monkeypatch.setattr(backend_module, "run_isolated_build", _isolated_child(calls)) - command = ["build", "timenet/hello-world", "--out", str(tmp_path)] + command = ["build", "timenet/hello-world", "--local-registry", str(tmp_path)] assert runner.invoke(app, command).exit_code == 0 result = runner.invoke(app, command) @@ -119,11 +128,11 @@ def test_a_build_that_reuses_the_release_says_so_on_stderr(monkeypatch, tmp_path def test_build_takes_an_exact_release_and_refuses_another_version(monkeypatch, tmp_path): calls = [] monkeypatch.setattr(backend_module, "run_isolated_build", _isolated_child(calls)) - result = runner.invoke(app, ["build", "timenet/hello-world@1.0.0", "--out", str(tmp_path)]) + result = runner.invoke(app, ["build", "timenet/hello-world@1.0.0", "--local-registry", str(tmp_path)]) assert result.exit_code == 0, result.output assert [call["release"] for call in calls] == [HELLO] - result = runner.invoke(app, ["build", "timenet/hello-world@9.9.9", "--out", str(tmp_path)]) + result = runner.invoke(app, ["build", "timenet/hello-world@9.9.9", "--local-registry", str(tmp_path)]) assert isinstance(result.exception, TimeNetVersionUnavailableError) assert len(calls) == 1 @@ -137,7 +146,7 @@ def test_build_rejects_an_unknown_id_without_importing_anything(monkeypatch, tmp # a connector class by identity still sees the same object. monkeypatch.delitem(sys.modules, name) - result = runner.invoke(app, ["build", "nope/nothing", "--out", str(tmp_path)]) + result = runner.invoke(app, ["build", "nope/nothing", "--local-registry", str(tmp_path)]) assert result.exit_code != 0 assert "no connector for" in result.output @@ -147,7 +156,7 @@ def test_build_rejects_an_unknown_id_without_importing_anything(monkeypatch, tmp def test_build_rejects_an_id_that_no_card_declares_and_names_the_known_ids(tmp_path): # The module path of the connector accepts this spelling, but a dataset ID must match its card. - result = runner.invoke(app, ["build", "timenet/hello_world", "--out", str(tmp_path)]) + result = runner.invoke(app, ["build", "timenet/hello_world", "--local-registry", str(tmp_path)]) assert result.exit_code == 2 assert "no connector for 'timenet/hello_world'" in result.output @@ -162,7 +171,7 @@ def test_build_forwards_quiet_to_the_isolated_child(monkeypatch, tmp_path, quiet monkeypatch.setattr(backend_module, "run_isolated_build", _isolated_child(calls)) result = runner.invoke( - app, [*(["--quiet"] if quiet else []), "build", "timenet/hello-world", "--out", str(tmp_path)] + app, [*(["--quiet"] if quiet else []), "build", "timenet/hello-world", "--local-registry", str(tmp_path)] ) assert result.exit_code == 0, result.output @@ -180,7 +189,7 @@ def test_isolated_build_prints_the_version_dir_and_narrates_once(tmp_path): env.pop("TIMENET_ISOLATION", None) # default is "on"; make sure nothing forces it off run_cli = [sys.executable, "-c", "from timenet_connectors.builder.cli import main; main()"] result = subprocess.run( - [*run_cli, "build", "timenet/hello-world", "--out", str(registry)], + [*run_cli, "build", "timenet/hello-world", "--local-registry", str(registry)], env=env, cwd=tmp_path, # away from the repo tree so the CLI does not read the repo .env capture_output=True, diff --git a/packages/timenet-connectors/tests/test_build_env.py b/packages/timenet-connectors/tests/test_build_env.py index ff6c91594..7b26a698c 100644 --- a/packages/timenet-connectors/tests/test_build_env.py +++ b/packages/timenet-connectors/tests/test_build_env.py @@ -58,7 +58,7 @@ def test_uv_command_passes_the_requirements_file(): def test_uv_command_puts_the_child_argv_last(): - argv = ["timenet-build", "build", "x/y", "--out", "/fake/reg"] + argv = ["timenet-build", "build", "x/y", "--local-registry", "/fake/reg"] assert uv_command(_spec(), argv)[-len(argv) :] == argv diff --git a/packages/timenet-connectors/tests/test_build_plan.py b/packages/timenet-connectors/tests/test_build_plan.py index 9e7f8918b..38a958732 100644 --- a/packages/timenet-connectors/tests/test_build_plan.py +++ b/packages/timenet-connectors/tests/test_build_plan.py @@ -1,15 +1,18 @@ from pathlib import Path +import shlex import shutil -import click import numpy as np import pytest +from rich.console import Console from typer.testing import CliRunner -from timenet import builders +import timenet +from timenet import builders, client as client_module +from timenet.client import default_local_registry from timenet.connectors import BaseConnector from timenet.coordinator.build import plan_release_build, run_plan -from timenet.coordinator.plan import BuildStep, CopyStep, Step +from timenet.coordinator.plan import BuildStep, CopyStep, Plan, ReadStep, Step from timenet.dataset import Record, RegularAxis, Signal, Source, TimeFDataset from timenet.engine import open_parents, run_pipeline from timenet.errors import ( @@ -18,6 +21,8 @@ TimeNetAccessError, TimeNetFileError, TimeNetMissingParentError, + TimeNetRegistryError, + TimeNetVersionConflictError, ) from timenet.reader import TimeFReader from timenet.registry import LocalRegistry, WritableRegistry, open_local_version @@ -32,7 +37,7 @@ from timenet.values_backends import ValuesBackend from timenet_connectors.builder import backend as backend_module, cli as cli_module, env as env_module from timenet_connectors.builder.backend import ConnectorBuilder -from timenet_connectors.builder.cli import app +from timenet_connectors.builder.cli import _plan_lines, app GRAND = DatasetRef(dataset_id=make_dataset().metadata.dataset_id, version=make_dataset().metadata.dataset_version) @@ -524,7 +529,7 @@ def test_build_prepares_parents_before_the_isolated_child(tmp_path, monkeypatch) result = runner.invoke( app, - ["build", CHILD.dataset_id, "--out", str(out), "--fetch-from", str(origin.root), "--build-parents", "-f"], + ["build", CHILD.dataset_id, "--local-registry", str(out), "--remote-registry", str(origin.root), "-f"], ) assert result.exit_code == 0, result.output @@ -583,7 +588,7 @@ def child(command, env): monkeypatch.setattr(backend_module, "run_pipeline", run_pipeline) # The output holds PARENT, so the build gets only GRAND, the payload source of PARENT. - result = runner.invoke(app, ["build", PARENT.dataset_id, "--out", str(output.root), "--build-parents"]) + result = runner.invoke(app, ["build", PARENT.dataset_id, "--local-registry", str(output.root)]) assert result.exit_code == 0, result.output # The rebuild of GRAND gets the pins of PARENT, through a file that is gone afterwards. @@ -593,52 +598,76 @@ def child(command, env): assert output.exists(GRAND.dataset_id, str(GRAND.version)) -@pytest.mark.parametrize("case", ["missing", "damaged", "not-open"]) +_COPY_HINT = ( + "copy it from a registry that holds it with --remote-registry , or publish it to such a registry first " + "with `timenet publish`." +) + + +@pytest.mark.parametrize("case", ["missing", "missing-without-auto-build", "damaged", "not-open", "not-served"]) def test_the_cli_hint_names_its_flags(tmp_path, monkeypatch, case): cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) - out = LocalRegistry(tmp_path / "out") + store = LocalRegistry(tmp_path / "store") + options = [] + if case == "missing": + # No registry holds the parent, and no installed connector builds it. + del cards[PARENT.dataset_id] + if case == "missing-without-auto-build": + options = ["--no-auto-build"] if case == "damaged": - out.put_dataset(TimeFDataset(metadata=cards[PARENT.dataset_id])) - (release_dir(out.root, PARENT) / "control.duckdb").write_bytes(b"cut short") + store.put_dataset(TimeFDataset(metadata=cards[PARENT.dataset_id])) + (release_dir(store.root, PARENT) / "control.duckdb").write_bytes(b"cut short") + options = ["--remote-registry", str(tmp_path / "remote registry")] if case == "not-open": cards[CHILD.dataset_id] = cards[CHILD.dataset_id].model_copy(update=_GATED) - monkeypatch.setattr(cli_module, "_resolve_target", lambda target: _Hosted(tmp_path / "hosted")) + monkeypatch.setattr(cli_module, "open_registry", lambda url, **_: _Hosted(tmp_path / "hosted")) + options = ["--publish-to", "s3://bucket/registry"] + if case == "not-served": + # The remote registry lists the parent without serving its files, and the list has no connectors. + hosted = _Hosted(tmp_path / "hosted") + hosted.put_dataset(TimeFDataset(metadata=cards[PARENT.dataset_id].model_copy(update=_GATED))) + monkeypatch.setattr(client_module, "open_registry", lambda uri, **_: hosted) + options = ["--no-auto-build"] - # A damaged parent is never built over, even with --build-parents. - options = ["--build-parents"] if case == "damaged" else [] - result = runner.invoke(app, ["build", CHILD.dataset_id, "--out", str(out.root), *options]) + result = runner.invoke(app, ["build", CHILD.dataset_id, "--local-registry", str(store.root), *options]) + # The hint of a damaged copy repeats the registry flags of the command, quoted for the shell. + remote = str(tmp_path / "remote registry") + flags = f" --local-registry {shlex.quote(str(store.root))} --remote-registry {shlex.quote(remote)}" hints = { - "missing": ( + "missing": (TimeNetMissingParentError, f". {_COPY_HINT[0].upper()}{_COPY_HINT[1:]}"), + "missing-without-auto-build": ( TimeNetMissingParentError, - ". Fetch it with --fetch-from or build it with --build-parents.", + f". Build it from its installed connector with --auto-build, {_COPY_HINT}", ), # A rebuild of a release that others read changes the files they pin. "damaged": ( TimeNetFileError, - ". Fetch an intact copy with --fetch-from , or rebuild it with " - "`timenet-build build test/parent --force`. " + f". Copy an intact release into the local registry with `timenet download test/parent@1.0.0{flags}`, or " + f"rebuild it with `timenet-build build test/parent --force{flags}`. " "A rebuild changes its files, and the releases that pin the old files then fail to open.", ), "not-open": ( TimeNetAccessError, - ". A remote --out holds open releases only. Build it into a local --out instead.", + ". _Hosted serves open releases only, so --publish-to takes open releases only. " + "Build it into the local registry without --publish-to instead.", ), + "not-served": (TimeNetAccessError, ". Build it from its installed connector with --auto-build."), } error, hint = hints[case] - assert isinstance(result.exception, error) + assert isinstance(result.exception, error), result.output assert str(result.exception).endswith(hint) -def _remote_out(tmp_path, monkeypatch, cards) -> tuple[_Hosted, LocalRegistry, list]: - """Make the ``--out`` of the CLI a registry that is not local, and record each isolated build. +def _publish_to(tmp_path, monkeypatch, cards) -> tuple[_Hosted, LocalRegistry, list]: + """Make each ``--publish-to`` of the CLI open a registry that is not local, and record each isolated build. Returns: - The registry that ``--out`` names, the store, and the list of the builds. + The registry that ``--publish-to`` opens, the store, and the list of the builds. """ remote = _Hosted(tmp_path / "hosted") - monkeypatch.setattr(cli_module, "_resolve_target", lambda target: remote) + monkeypatch.setattr(cli_module, "open_registry", lambda url, **_: remote) monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) monkeypatch.setenv("TIMENET_LOCAL_REGISTRY", str(tmp_path / "store")) calls = [] @@ -646,91 +675,182 @@ def _remote_out(tmp_path, monkeypatch, cards) -> tuple[_Hosted, LocalRegistry, l return remote, LocalRegistry(tmp_path / "store"), calls -def test_build_to_a_remote_out_refuses_a_credentialed_release_before_any_download(tmp_path, monkeypatch): +@pytest.mark.parametrize("gated", ["child", "parent"]) +def test_publish_to_refuses_a_credentialed_release_before_any_download(tmp_path, monkeypatch, gated): cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) - cards[PARENT.dataset_id] = cards[PARENT.dataset_id].model_copy(update=_GATED) + gated_ref = CHILD if gated == "child" else PARENT + cards[gated_ref.dataset_id] = cards[gated_ref.dataset_id].model_copy(update=_GATED) origin = LocalRegistry(tmp_path / "origin") origin.put_dataset(TimeFDataset(metadata=cards[PARENT.dataset_id])) - _, local, calls = _remote_out(tmp_path, monkeypatch, cards) - - command = ["build", CHILD.dataset_id, "--out", "s3://bucket/registry", "--fetch-from", str(origin.root)] + _, store, calls = _publish_to(tmp_path, monkeypatch, cards) + command = ["build", CHILD.dataset_id, "--publish-to", "s3://bucket/registry"] - result = runner.invoke(app, command) + result = runner.invoke(app, [*command, "--remote-registry", str(origin.root), "--no-auto-build"]) - # The build into the local registry gets the access checks of the remote registry. + # The build into the store gets the access checks of the destination. assert isinstance(result.exception, TimeNetAccessError), result.output - assert result.exception.dataset == PARENT + assert result.exception.dataset == gated_ref assert calls == [] - assert not local.exists(PARENT.dataset_id, str(PARENT.version)) + assert not store.exists(PARENT.dataset_id, str(PARENT.version)) -def test_build_to_a_remote_out_builds_locally_and_copies(tmp_path, monkeypatch): +def test_publish_to_builds_into_the_store_and_publishes(tmp_path, monkeypatch): cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) origin = LocalRegistry(tmp_path / "origin") parent = make_dataset() parent._metadata = cards[PARENT.dataset_id] origin.put_dataset(parent) - remote, local, calls = _remote_out(tmp_path, monkeypatch, cards) - command = ["build", CHILD.dataset_id, "--out", "s3://bucket/registry", "--fetch-from", str(origin.root)] + remote, store, calls = _publish_to(tmp_path, monkeypatch, cards) + command = ["build", CHILD.dataset_id, "--publish-to", "s3://bucket/registry"] + command += ["--remote-registry", str(origin.root), "--no-auto-build"] result = runner.invoke(app, command) assert result.exit_code == 0, result.output - assert result.stdout.strip() == str(CHILD.version) - # The parent lands in the local registry first, and the child builds there. Then the copy takes - # the child and its payload source, the parent, to the remote registry. + assert result.stdout == f"{CHILD.version}\n" + # The parent lands in the store first, and the child builds there. Then the publish takes the + # child and its payload source, the parent, to the destination. assert calls == [(CHILD.dataset_id, False)] - for registry in (local, remote): + for registry in (store, remote): assert registry.exists(CHILD.dataset_id, str(CHILD.version)) assert registry.exists(PARENT.dataset_id, str(PARENT.version)) - # A remote registry that holds the version gets nothing, and nothing builds. - shutil.rmtree(local.root) + # A destination that holds the version gets nothing, and nothing builds. + shutil.rmtree(store.root) result = runner.invoke(app, command) - assert (result.exit_code, result.stdout.strip()) == (0, str(CHILD.version)) - assert result.stderr.splitlines() == ["✅ Nothing to do: 'test/child' 1.0.0 is already in the remote --out"] + assert (result.exit_code, result.stdout) == (0, f"{CHILD.version}\n") + assert result.stderr.splitlines() == [f"✅ Nothing to do: 'test/child' 1.0.0 is already in {remote}"] assert calls == [(CHILD.dataset_id, False)] - assert not local.root.exists() + assert not store.root.exists() -def test_build_to_a_remote_out_copies_a_missing_parent_from_that_registry(tmp_path, monkeypatch): +def test_publish_to_copies_parents_only_from_the_sources(tmp_path, monkeypatch): cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) - remote, local, calls = _remote_out(tmp_path, monkeypatch, cards) + remote, store, calls = _publish_to(tmp_path, monkeypatch, cards) parent = make_dataset() parent._metadata = cards[PARENT.dataset_id] remote.put_dataset(parent) + command = ["build", CHILD.dataset_id, "--publish-to", "s3://bucket/registry", "--no-auto-build"] - result = runner.invoke(app, ["build", CHILD.dataset_id, "--out", "s3://bucket/registry"]) + # The destination holds the parent, but the destination is no source of parents. + result = runner.invoke(app, command) - # Without --fetch-from, the build copies the parent from the remote --out, which holds it, into - # the local registry. So only the child builds. + assert isinstance(result.exception, TimeNetMissingParentError), result.output + assert "--remote-registry" in str(result.exception) + assert "`timenet publish`" in str(result.exception) + assert calls == [] + # As the remote registry of the list, the same registry provides the parent. + result = runner.invoke(app, [*command, "--remote-registry", str(remote.local_root)]) assert result.exit_code == 0, result.output assert calls == [(CHILD.dataset_id, False)] - assert local.exists(PARENT.dataset_id, str(PARENT.version)) + assert store.exists(PARENT.dataset_id, str(PARENT.version)) assert remote.exists(CHILD.dataset_id, str(CHILD.version)) +def test_publish_to_a_local_directory_publishes_there(tmp_path, monkeypatch, child_builds): + cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) + monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) + published = LocalRegistry(tmp_path / "published") + + result = runner.invoke(app, ["build", CHILD.dataset_id, "--publish-to", str(published.root)]) + + assert result.exit_code == 0, result.output + assert result.stdout == f"{CHILD.version}\n" + assert child_builds == [(PARENT.dataset_id, False), (CHILD.dataset_id, False)] + # The build lands in the store, and the publish copies the child with its payload source. + for registry in (default_local_registry(), published): + assert registry.exists(CHILD.dataset_id, str(CHILD.version)) + assert registry.exists(PARENT.dataset_id, str(PARENT.version)) + + +@pytest.mark.parametrize("value", ["", " ", "ftp://host/registry"]) +def test_publish_to_takes_a_registry_url_or_a_path(monkeypatch, child_builds, value): + # An empty value would publish into the current directory. + result = runner.invoke(app, ["build", GRAND.dataset_id, "--publish-to", value]) + + assert result.exit_code == 1, result.output + assert isinstance(result.exception, TimeNetRegistryError) + assert str(result.exception).startswith("--publish-to ") + assert child_builds == [] + + +def test_publish_to_shows_a_settings_error_with_its_own_message(monkeypatch, child_builds): + monkeypatch.setenv("TIMENET_AUTO_BUILD", "maybe") + + result = runner.invoke(app, ["build", GRAND.dataset_id, "--publish-to", "timenet://"]) + + assert isinstance(result.exception, TimeFValidationError), result.output + assert "TIMENET_AUTO_BUILD" in str(result.exception) + assert child_builds == [] + + +def test_the_requested_release_builds_with_no_auto_build(tmp_path, monkeypatch, child_builds): + cards = _install_cards(monkeypatch, child={}, parent={}) + monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) + store = tmp_path / "store" + + result = runner.invoke(app, ["build", CHILD.dataset_id, "--local-registry", str(store), "--no-auto-build"]) + + assert result.exit_code == 0, result.output + assert result.stdout == f"{release_dir(store, CHILD)}\n" + assert child_builds == [(CHILD.dataset_id, False)] + + def _files(root: Path) -> dict[Path, tuple[int, int]]: """Return the size and the modification time of each file under ``root``, by its path.""" return {path: (path.stat().st_size, path.stat().st_mtime_ns) for path in root.rglob("*") if path.is_file()} -def test_force_is_a_usage_error_if_the_remote_out_holds_the_version(tmp_path, monkeypatch): +def _change_the_next_build(monkeypatch) -> None: + """Make each later build store its dataset with one more Record, so that it writes other files.""" + put_dataset = LocalRegistry.put_dataset + + def put_one_more_record(self, dataset, **options): + extra = Signal.from_values( + np.arange(16, dtype=np.float32), spec=_SPEC, name="extra", time_axis=RegularAxis.from_rate_hz(16) + ) + dataset.add_record(record=Record(sources=(Source(name="extra", signals=(extra,)),))) + return put_dataset(self, dataset, **options) + + monkeypatch.setattr(LocalRegistry, "put_dataset", put_one_more_record) + + +def test_force_with_publish_to_rebuilds_and_the_publish_refuses_other_files(tmp_path, monkeypatch): cards = _install_cards(monkeypatch, child={}, parent={}) - _, local, calls = _remote_out(tmp_path, monkeypatch, cards) - command = ["build", CHILD.dataset_id, "--out", "s3://bucket/registry"] + remote, _, calls = _publish_to(tmp_path, monkeypatch, cards) + command = ["build", CHILD.dataset_id, "--publish-to", "s3://bucket/registry"] assert runner.invoke(app, command).exit_code == 0 - held = _files(local.root) + published = _files(remote.local_root) + _change_the_next_build(monkeypatch) # the next build writes other files + result = runner.invoke(app, [*command, "--force"]) + assert isinstance(result.exception, TimeNetVersionConflictError) + assert calls == [(CHILD.dataset_id, False), (CHILD.dataset_id, True)] + # A published version cannot change, so the destination keeps its files. + assert _files(remote.local_root) == published + + +def test_force_with_publish_to_does_nothing_for_the_same_files(tmp_path, monkeypatch): + cards = _install_cards(monkeypatch, child={}, parent={}) + remote, _, calls = _publish_to(tmp_path, monkeypatch, cards) + saved = LocalRegistry(tmp_path / "saved") + _store_from_card(CHILD.dataset_id, saved.root, force=False) + + def copy_the_saved_release(release, store_root, *, force=False, **_): + # Each build puts the same release directory into the store, so both builds write the same files. + calls.append((release.dataset_id, force)) + LocalRegistry(store_root).put_release(release_dir(saved.root, CHILD)) + + monkeypatch.setattr(backend_module, "run_isolated_build", copy_the_saved_release) + command = ["build", CHILD.dataset_id, "--publish-to", "s3://bucket/registry"] + assert runner.invoke(app, command).exit_code == 0 + published = _files(remote.local_root) result = runner.invoke(app, [*command, "--force"]) - # A published version never changes. So the command refuses before anything downloads or builds. - assert result.exit_code == 2, result.output - output = click.unstyle(result.output) - assert "Invalid value for --force" in output - # The error box wraps the message and draws a border on each line. - assert "a published version never changes" in " ".join(output.replace("│", " ").split()) - assert calls == [(CHILD.dataset_id, False)] - assert _files(local.root) == held + assert result.exit_code == 0, result.output + assert result.stdout == f"{CHILD.version}\n" + assert calls == [(CHILD.dataset_id, False), (CHILD.dataset_id, True)] + # The destination holds the same files, so the publish copied nothing. + assert _files(remote.local_root) == published def test_the_plan_drops_the_store_from_parents_from(tmp_path, monkeypatch): @@ -796,19 +916,6 @@ def test_the_requested_build_waits_for_a_payload_source_of_a_held_parent(tmp_pat ) -def test_the_hint_names_only_the_option_that_the_build_did_not_use(tmp_path, monkeypatch): - # No registry holds the parent, and no installed connector builds it. The build uses --fetch-from. - cards = _install_cards(monkeypatch, child={"parent": PARENT}) - monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) - out = tmp_path / "out" - - options = ["--fetch-from", str(tmp_path / "origin")] - error = runner.invoke(app, ["build", CHILD.dataset_id, "--out", str(out), *options]).exception - - assert isinstance(error, TimeNetMissingParentError) - assert error.hint == "Build it with --build-parents." - - def _count_reads(monkeypatch, registry) -> dict[str, int]: """Count each manifest that is read from a registry, by release.""" reads, get_manifest = {}, registry.get_manifest @@ -847,3 +954,134 @@ def test_a_plan_reads_each_manifest_once_from_each_registry(tmp_path, monkeypatc # more, from the registry as it is when the copy runs. SOURCE is in both copies. assert hosted_reads == {str(DECLARED): 1, str(SOURCE): 2, str(GRAND): 1} assert child_builds == [(CHILD.dataset_id, False)] + + +def test_plan_lines_draw_a_shared_release_once(tmp_path, monkeypatch): + _install_cards(monkeypatch, child={"parent": PARENT, "grand": GRAND}, parent={"grand": GRAND}) + hosted = LocalRegistry(tmp_path / "hosted") + plan = Plan( + ( + _build(GRAND), + CopyStep(PARENT, hosted, depends_on=(GRAND,)), + _build(CHILD, depends_on=(PARENT, GRAND)), + ) + ) + + assert _plan_lines(plan) == [ + "test/child@1.0.0: build with installed connectors", + f"├── test/parent@1.0.0: copy from {hosted}", + "│ └── timenet/hello-world@1.0.0: build with installed connectors", + "└── timenet/hello-world@1.0.0: see above", + ] + + +def test_plan_lines_name_read_steps(tmp_path): + hosted = LocalRegistry(tmp_path / "hosted") + # Neither step depends on the other, so each is a root of its own, the last step first. + plan = Plan((ReadStep(GRAND, hosted), ReadStep(PARENT, hosted))) + + assert _plan_lines(plan) == [ + f"test/parent@1.0.0: read from {hosted}", + f"timenet/hello-world@1.0.0: read from {hosted}", + ] + + +def test_dry_run_prints_the_plan_and_changes_nothing(tmp_path, monkeypatch): + cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) + remote, _, calls = _publish_to(tmp_path, monkeypatch, cards) + store = tmp_path / "store" + result = runner.invoke( + app, + [ + "build", + CHILD.dataset_id, + "--local-registry", + str(store), + "--dry-run", + "--publish-to", + "s3://bucket/registry", + ], + ) + assert result.exit_code == 0, result.output + assert result.stdout.splitlines() == [ + "test/child@1.0.0: build with installed connectors", + "└── test/parent@1.0.0: build with installed connectors", + f"then publish test/child@1.0.0 to {remote}", + ] + assert calls == [] and not store.exists() and not remote.exists(CHILD.dataset_id, "1.0.0") + + +def test_the_dry_run_lines_equal_the_plan_lines_of_plan_build(tmp_path, monkeypatch, child_builds): + cards = _install_cards(monkeypatch, child={"parent": PARENT, "grand": GRAND}, parent={}) + monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) + origin = LocalRegistry(tmp_path / "origin") + _store_from_card(PARENT.dataset_id, origin.root, force=False) + monkeypatch.setenv("TIMENET_REMOTE_REGISTRY", str(origin.root)) + + result = runner.invoke(app, ["build", CHILD.dataset_id, "--dry-run"]) + + # The command and the public API plan alike: a copy of PARENT, a build of GRAND, and the build of CHILD. + assert result.exit_code == 0, result.output + lines = _plan_lines(timenet.plan_build(CHILD.dataset_id)) + assert len(lines) == 3 + assert result.stdout.splitlines() == lines + assert child_builds == [] + + +def test_a_build_prints_the_plan_before_the_first_step(tmp_path, monkeypatch, child_builds): + cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) + monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) + # The plan names a long path, so the console must not wrap the lines around it. + monkeypatch.setattr(cli_module.console, "rich", Console(stderr=True, width=1000)) + origin = LocalRegistry(tmp_path / "origin") + _store_from_card(PARENT.dataset_id, origin.root, force=False) + + result = runner.invoke(app, ["build", CHILD.dataset_id, "--remote-registry", str(origin.root)]) + + assert result.exit_code == 0, result.output + lines = result.stderr.splitlines() + first_step = next(index for index, line in enumerate(lines) if "Building" in line or "Copying" in line) + assert lines[:first_step] == [ + "📋 Plan for 'test/child' 1.0.0:", + " test/child@1.0.0: build with installed connectors", + f" └── test/parent@1.0.0: copy from {origin}", + ] + assert child_builds == [(CHILD.dataset_id, False)] + + +@pytest.mark.parametrize("quiet", [False, True]) +def test_quiet_hides_the_plan(tmp_path, monkeypatch, child_builds, quiet): + cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) + monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) + + result = runner.invoke(app, [*(["--quiet"] if quiet else []), "build", CHILD.dataset_id]) + + assert result.exit_code == 0, result.output + if quiet: + assert result.stderr == "" + else: + assert "📋 Plan for 'test/child' 1.0.0:" in result.stderr.splitlines() + assert child_builds == [(PARENT.dataset_id, False), (CHILD.dataset_id, False)] + + +@pytest.mark.parametrize("held_in", ["store", "destination", "store-only"]) +def test_a_dry_run_of_a_held_release_says_nothing_to_do(tmp_path, monkeypatch, held_in): + cards = _install_cards(monkeypatch, child={}, parent={}) + remote, store, calls = _publish_to(tmp_path, monkeypatch, cards) + # The line names a long path, so the console must not wrap it. + monkeypatch.setattr(cli_module.console, "rich", Console(stderr=True, width=1000)) + held_by = remote if held_in == "destination" else store + _store_from_card(CHILD.dataset_id, held_by.root if held_by is store else remote.local_root, force=False) + options = [] if held_in == "store" else ["--publish-to", "s3://bucket/registry"] + + result = runner.invoke(app, ["build", CHILD.dataset_id, "--dry-run", *options]) + + assert result.exit_code == 0, result.output + if held_in == "store-only": + # The store holds the release and the destination does not, so only the publish is left to do. + assert result.stdout.splitlines() == [f"then publish test/child@1.0.0 to {remote}"] + assert result.stderr == "" + else: + assert result.stdout == "" + assert result.stderr.splitlines() == [f"✅ Nothing to do: 'test/child' 1.0.0 is already in {held_by}"] + assert calls == [] diff --git a/packages/timenet-connectors/tests/test_end_to_end.py b/packages/timenet-connectors/tests/test_end_to_end.py index 51c8e38d5..32c42ee19 100644 --- a/packages/timenet-connectors/tests/test_end_to_end.py +++ b/packages/timenet-connectors/tests/test_end_to_end.py @@ -6,12 +6,11 @@ from timenet.client import TimeNet from timenet.config import settings -from timenet.errors import TimeNetRegistryError from timenet.provenance import build_env -from timenet.registry import LocalRegistry, RemoteRegistry +from timenet.registry import LocalRegistry from timenet.sources import InstalledConnectors from timenet.testing import assert_datasets_equal -from timenet_connectors.builder.cli import _resolve_target, app as build_app +from timenet_connectors.builder.cli import app as build_app from timenet_connectors.datasets.timenet.hello_world import HelloWorldConnector from timenet_connectors.discovery import available, resolve @@ -47,7 +46,9 @@ def test_build_then_load_round_trips(converted, tmp_path): registry = tmp_path / "registry" # PRODUCE: build the demo dataset into a local registry directory. - result = runner.invoke(build_app, ["build", "timenet/hello-world", "--out", str(registry), "--no-isolation"]) + result = runner.invoke( + build_app, ["build", "timenet/hello-world", "--local-registry", str(registry), "--no-isolation"] + ) assert result.exit_code == 0, result.output assert (registry / "timenet" / "hello-world" / "1.0.0" / "manifest.json").exists() @@ -58,7 +59,7 @@ def test_build_then_load_round_trips(converted, tmp_path): def test_build_unknown_id_fails(tmp_path): - result = runner.invoke(build_app, ["build", "acme/not_a_dataset", "--out", str(tmp_path / "registry")]) + result = runner.invoke(build_app, ["build", "acme/not_a_dataset", "--local-registry", str(tmp_path / "registry")]) assert result.exit_code != 0 @@ -76,38 +77,30 @@ def test_build_honors_timenet_local_registry(monkeypatch, tmp_path): assert TimeNet().load("timenet/hello-world@1.0.0").metadata.dataset_id == "timenet/hello-world" -def test_build_out_overrides_timenet_local_registry(monkeypatch, tmp_path): +def test_local_registry_flag_overrides_timenet_local_registry(monkeypatch, tmp_path): monkeypatch.setenv("TIMENET_LOCAL_REGISTRY", str(tmp_path / "elsewhere")) - out = tmp_path / "out" + store = tmp_path / "store" - result = runner.invoke(build_app, ["build", "timenet/hello-world", "--out", str(out), "--no-isolation"]) + result = runner.invoke( + build_app, ["build", "timenet/hello-world", "--local-registry", str(store), "--no-isolation"] + ) assert result.exit_code == 0 - assert (out / "timenet" / "hello-world" / "1.0.0" / "manifest.json").exists() + assert (store / "timenet" / "hello-world" / "1.0.0" / "manifest.json").exists() assert not (tmp_path / "elsewhere").exists() -def test_resolve_target_routes_remote_url_to_registry(tmp_path): - # A remote --out publishes through a writable registry (constructed offline, no network). - assert isinstance(_resolve_target("timenet://"), RemoteRegistry) - assert isinstance(_resolve_target("https://registry.timenet.ai"), RemoteRegistry) - # A local --out writes a directory. - assert _resolve_target(str(tmp_path / "reg")) == tmp_path / "reg" - - -def test_resolve_target_rejects_an_empty_out_instead_of_the_current_directory(): - with pytest.raises(TimeNetRegistryError, match=r"^--out is blank"): - _resolve_target("") - - def test_build_cleans_cache_but_keep_flag_retains(tmp_path): cache = settings().downloads_dir / "timenet" / "hello-world" - built = runner.invoke(build_app, ["build", "timenet/hello-world", "--out", str(tmp_path / "r1"), "--no-isolation"]) + built = runner.invoke( + build_app, ["build", "timenet/hello-world", "--local-registry", str(tmp_path / "r1"), "--no-isolation"] + ) assert built.exit_code == 0 assert not cache.exists() # cleaned by default after a successful build keep = runner.invoke( - build_app, ["build", "timenet/hello-world", "--out", str(tmp_path / "r2"), "--keep-cache", "--no-isolation"] + build_app, + ["build", "timenet/hello-world", "--local-registry", str(tmp_path / "r2"), "--keep-cache", "--no-isolation"], ) assert keep.exit_code == 0 assert cache.exists() # retained with --keep-cache diff --git a/packages/timenet/src/timenet/cli/app.py b/packages/timenet/src/timenet/cli/app.py index 1267d8feb..966b327a9 100644 --- a/packages/timenet/src/timenet/cli/app.py +++ b/packages/timenet/src/timenet/cli/app.py @@ -270,10 +270,9 @@ def publish( bare_id, version = split_ref(dataset_id) wanted = bare_id if version is None else f"{bare_id}@{version}" # Repeat the path of --local-registry, so that each command of the hint uses it too. - path = shlex.quote(str(store.root)) - out, flag = ("", "") if local_registry is None else (f" --out {path}", f" --local-registry {path}") + flag = "" if local_registry is None else f" --local-registry {shlex.quote(str(store.root))}" hint = ( - f"Build it into the local registry with `timenet-build build {wanted}{out}`, " + f"Build it into the local registry with `timenet-build build {wanted}{flag}`, " f"or copy it there with `timenet download {wanted}{flag}`." ) raise TimeNetDatasetNotFoundError(f"{str(error).rstrip('.')}. {hint}") from error diff --git a/packages/timenet/src/timenet/cli/ui.py b/packages/timenet/src/timenet/cli/ui.py index b4ee73d54..d4b13da2d 100644 --- a/packages/timenet/src/timenet/cli/ui.py +++ b/packages/timenet/src/timenet/cli/ui.py @@ -44,6 +44,14 @@ def success(self, message: str) -> None: """Prints a success line. Quiet mode suppresses this line.""" self._emit("✅", message, style="green") + def detail(self, message: str) -> None: + """Prints a line without an emoji. Quiet mode suppresses this line. + + The console does not wrap the line, so a long line stays whole. + """ + if not self.quiet: + self.rich.print(message, markup=False, highlight=False, soft_wrap=True) + def warn(self, message: str) -> None: """Prints a warning line. Quiet mode does not suppress this line.""" self._emit("⚠️", message, style="yellow", force=True) diff --git a/packages/timenet/src/timenet/client.py b/packages/timenet/src/timenet/client.py index 4b2ead3c7..0dbb2f226 100644 --- a/packages/timenet/src/timenet/client.py +++ b/packages/timenet/src/timenet/client.py @@ -559,7 +559,7 @@ def _repair_hint(release: DatasetRef, found: Found | None, store: LocalRegistry) return f"Copy an intact release into the local registry with download('{release}')." if isinstance(found, FoundInConnector): # Only an installed connector provides the release. - command = f"timenet-build build {release.dataset_id} --out {shlex.quote(str(store.root))} --force" + command = f"timenet-build build {release.dataset_id} --local-registry {shlex.quote(str(store.root))} --force" return f"Build it again with `{command}`." # Only the local registry holds the release. return "No registry other than the local registry provides it, so download() cannot replace it." diff --git a/packages/timenet/tests/test_cli.py b/packages/timenet/tests/test_cli.py index a8920d3d4..c63974227 100644 --- a/packages/timenet/tests/test_cli.py +++ b/packages/timenet/tests/test_cli.py @@ -303,7 +303,7 @@ def test_publish_of_a_release_that_the_store_lacks_names_the_fix(tmp_path): assert type(result.exception) is TimeNetDatasetNotFoundError assert str(result.exception) == ( f"timenet/hello-world is not in local registry {store}. " - f"Build it into the local registry with `timenet-build build timenet/hello-world --out {store}`, " + f"Build it into the local registry with `timenet-build build timenet/hello-world --local-registry {store}`, " f"or copy it there with `timenet download timenet/hello-world --local-registry {store}`." ) # The hint keeps the version, and names no store without --local-registry. diff --git a/packages/timenet/tests/test_client.py b/packages/timenet/tests/test_client.py index 282360a66..25ea84669 100644 --- a/packages/timenet/tests/test_client.py +++ b/packages/timenet/tests/test_client.py @@ -166,7 +166,7 @@ def test_a_damaged_build_in_the_store_names_the_build_that_replaces_it(tmp_path, (store.root / "timenet/hello-world/1.0.0/control.duckdb").write_bytes(b"cut short") # No registry other than the local registry provides the release, so download() cannot replace it. - hint = f"Build it again with `timenet-build build timenet/hello-world --out {store.root} --force`." + hint = f"Build it again with `timenet-build build timenet/hello-world --local-registry {store.root} --force`." with pytest.raises(TimeNetFileError) as caught: client.open("timenet/hello-world@1.0.0") assert caught.value.hint == hint diff --git a/packages/timenet/tests/test_ui.py b/packages/timenet/tests/test_ui.py index 706e9e287..1bbfb47b8 100644 --- a/packages/timenet/tests/test_ui.py +++ b/packages/timenet/tests/test_ui.py @@ -128,3 +128,18 @@ def test_the_renderer_shows_copy_and_build_steps(monkeypatch, lines, bars): "✅ Built 'timenet/hello-world' → 1.0.0", ] assert len(bars) == 1 + + +def test_detail_prints_the_line_as_it_is(console, capsys): + # A line of a plan can name a long path. The console keeps the line whole, without an emoji. + line = "└── test/parent@1.0.0: copy from local registry " + "/directory" * 12 + console.detail(line) + captured = capsys.readouterr() + assert captured.out == "" + assert captured.err == f"{line}\n" + + +def test_quiet_suppresses_detail(console, capsys): + console.quiet = True + console.detail("test/child@1.0.0: build with installed connectors") + assert capsys.readouterr().err == "" From 5154408e53a4b9c3f5c2b20090196949ddb209b9 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Thu, 5 Nov 2026 00:32:01 +0100 Subject: [PATCH 2/3] docs: shorten the docstrings of the build and publish command Shorter docstrings for the code that this change adds or changes. Part of TNET-157. --- examples/load_tsqa.py | 8 ++-- .../src/timenet_connectors/builder/cli.py | 37 +++++++++---------- packages/timenet/src/timenet/cli/app.py | 8 ++-- packages/timenet/src/timenet/cli/ui.py | 6 +-- packages/timenet/src/timenet/client.py | 8 ++-- 5 files changed, 33 insertions(+), 34 deletions(-) diff --git a/examples/load_tsqa.py b/examples/load_tsqa.py index c8b7aba1b..594fc0393 100644 --- a/examples/load_tsqa.py +++ b/examples/load_tsqa.py @@ -1,11 +1,11 @@ -"""Print a local dataset's summary and sample signal values. +"""Load a local TimeF dataset and print its summary and a sample. -Build the dataset into a local registry, then run this script:: +Build the dataset into a local registry first, then run this script:: - uv run timenet-build build "chengsenwang/tsqa" --local-registry .timenet-registry + uv run timenet-build build chengsenwang/tsqa --local-registry .timenet-registry uv run python examples/load_tsqa.py -Pass a dataset ID and a registry path as arguments to load another dataset. +Pass another dataset ID and registry path as the first and second arguments. """ from pathlib import Path diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index e6a80dbbb..dd6e88260 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -173,13 +173,13 @@ def build( def _missing_hint(*, has_connectors: bool) -> str: - """Say how a build gets a parent that no source of its list provides. + """Explain how to obtain a missing parent release. Args: - has_connectors: Whether the list of sources holds the installed connectors. + has_connectors: Whether the source list includes installed connectors. Returns: - The hint, a full sentence that names ``--auto-build`` too if the list has no connectors. + Instructions to find the parent, including ``--auto-build`` when connectors are absent. """ ways = ( "copy it from a registry that holds it with --remote-registry , or publish it to such a registry " @@ -191,14 +191,14 @@ def _missing_hint(*, has_connectors: bool) -> str: def _access_hint(dest: WritableRegistry | None, *, has_connectors: bool) -> str | None: - """Say how a build gets a release that is not open. + """Explain how to resolve a release access error. Args: - dest: The registry that ``--publish-to`` names, or ``None``. - has_connectors: Whether the list of sources holds the installed connectors. + dest: The publish destination, or ``None``. + has_connectors: Whether the source list includes installed connectors. Returns: - The hint as a full sentence, or ``None`` if no flag of the command helps. + Instructions to use a local registry or build the release, or ``None`` if neither applies. """ if dest is not None and dest.requires_open_access: return ( @@ -211,14 +211,14 @@ def _access_hint(dest: WritableRegistry | None, *, has_connectors: bool) -> str def _registry_flags(*, local_registry: str | None, remote_registry: str | None) -> str: - """Repeat the registry flags that the command got, for a command in a hint. + """Format registry options as shell arguments. Args: - local_registry: The value of ``--local-registry``, or ``None``. - remote_registry: The value of ``--remote-registry``, or ``None``. + local_registry: The local registry path, or ``None``. + remote_registry: The remote registry URL, or ``None``. Returns: - Each given flag with its quoted value and a space before it, or an empty string. + Each supplied flag and shell-quoted value, preceded by a space, or an empty string. """ flags = (("--local-registry", local_registry), ("--remote-registry", remote_registry)) return "".join(f" {flag} {shlex.quote(value)}" for flag, value in flags if value is not None) @@ -254,24 +254,23 @@ def _show_plan(release: DatasetRef, plan: Plan) -> None: _ACTIONS = {"read": "read from", "copy": "copy from", "build": "build with"} -"""The words of each action of a step in a line of a plan.""" +"""Display text for each plan action.""" _BRANCHES = {"├── ": "│ ", "└── ": " "} -"""For each branch of a tree line: what the lines below it show in its column.""" +"""Indentation below each tree branch.""" def _plan_lines(plan: Plan) -> list[str]: - """Draw a plan as a tree, with the requested release at the top. + """Format a build plan as a tree with the requested release at the top. - Below each release come the releases that it needs. Each line names the release, the action and - the source, for example ``: copy from ``, or ``: see above`` the second - time. + Each line shows a release, its action, and its source. + Repeated releases show ``see above``. Args: - plan: The plan. + plan: The build plan. Returns: - One line for each node of the tree, without line ends. + The tree lines without newline characters. """ steps = {step.release: step for step in plan.steps} needed = {release for step in plan.steps for release in step.depends_on} diff --git a/packages/timenet/src/timenet/cli/app.py b/packages/timenet/src/timenet/cli/app.py index 966b327a9..9e34e972b 100644 --- a/packages/timenet/src/timenet/cli/app.py +++ b/packages/timenet/src/timenet/cli/app.py @@ -248,14 +248,14 @@ def publish( ), local_registry: str | None = LOCAL_REGISTRY, ) -> None: - """Copy a release and its payload sources to a registry, and print the release to stdout. + """Copy a release and its payload sources to another registry. - The source defaults to the local registry. - The copy skips identical releases and rejects releases with conflicting files. + Read from ``--source`` or the local registry, and print the release to stdout. + Skip identical releases and fail if the destination has different files for the same version. \f Raises: - TimeNetDatasetNotFoundError: If the source lacks the release. + TimeNetDatasetNotFoundError: If the source does not contain the release. """ # ruff: ignore[escape-sequence-in-docstring] - the form feed ends the help text of the command store = default_local_registry(local_registry=local_registry) origin = store if source is None else open_registry(source, label="--source") diff --git a/packages/timenet/src/timenet/cli/ui.py b/packages/timenet/src/timenet/cli/ui.py index d4b13da2d..26b913bd2 100644 --- a/packages/timenet/src/timenet/cli/ui.py +++ b/packages/timenet/src/timenet/cli/ui.py @@ -30,7 +30,7 @@ class _Console: - """Writes emoji status lines to stderr. Stdout stays reserved for machine-readable output.""" + """CLI output on stderr with a quiet mode.""" def __init__(self) -> None: self.quiet = False @@ -45,9 +45,9 @@ def success(self, message: str) -> None: self._emit("✅", message, style="green") def detail(self, message: str) -> None: - """Prints a line without an emoji. Quiet mode suppresses this line. + """Print a plain text line without wrapping. - The console does not wrap the line, so a long line stays whole. + Quiet mode hides the line. """ if not self.quiet: self.rich.print(message, markup=False, highlight=False, soft_wrap=True) diff --git a/packages/timenet/src/timenet/client.py b/packages/timenet/src/timenet/client.py index 0dbb2f226..751399033 100644 --- a/packages/timenet/src/timenet/client.py +++ b/packages/timenet/src/timenet/client.py @@ -544,15 +544,15 @@ def _copy_source(sources: Sequence[ReleaseSource], store: LocalRegistry, release def _repair_hint(release: DatasetRef, found: Found | None, store: LocalRegistry) -> str: - """Suggest how to replace a damaged release in the local registry. + """Suggest how to replace a damaged local release. Args: release: The damaged release. - found: The answer of the available source, or ``None`` if none exists. - store: The local registry. + found: The answer of a source for the release, or ``None``. + store: The local registry that holds the damaged release. Returns: - A message with a download or build command, or the reason replacement is unavailable. + A download or rebuild hint, or an explanation that no replacement is available. """ if isinstance(found, FoundInRegistry) and not is_same_registry(found.registry, store): # A registry other than the local registry provides the release. From f17c8aea314c56b858c491e99dc88a178cef077b Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Thu, 5 Nov 2026 11:02:28 +0100 Subject: [PATCH 3/3] test: trim the tests of the build and publish command Drop tests that cannot fail or repeat other tests, and sharpen the rest. Part of TNET-157. --- .../tests/test_build_plan.py | 77 +++---------------- .../tests/test_end_to_end.py | 19 +---- packages/timenet/tests/test_ui.py | 8 +- 3 files changed, 15 insertions(+), 89 deletions(-) diff --git a/packages/timenet-connectors/tests/test_build_plan.py b/packages/timenet-connectors/tests/test_build_plan.py index 38a958732..c3f363a21 100644 --- a/packages/timenet-connectors/tests/test_build_plan.py +++ b/packages/timenet-connectors/tests/test_build_plan.py @@ -7,7 +7,6 @@ from rich.console import Console from typer.testing import CliRunner -import timenet from timenet import builders, client as client_module from timenet.client import default_local_registry from timenet.connectors import BaseConnector @@ -748,6 +747,8 @@ def test_publish_to_copies_parents_only_from_the_sources(tmp_path, monkeypatch): def test_publish_to_a_local_directory_publishes_there(tmp_path, monkeypatch, child_builds): cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) + # A local directory takes releases that are not open, so the plan does not refuse this one. + cards[CHILD.dataset_id] = cards[CHILD.dataset_id].model_copy(update=_GATED) monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) published = LocalRegistry(tmp_path / "published") @@ -762,10 +763,9 @@ def test_publish_to_a_local_directory_publishes_there(tmp_path, monkeypatch, chi assert registry.exists(PARENT.dataset_id, str(PARENT.version)) -@pytest.mark.parametrize("value", ["", " ", "ftp://host/registry"]) -def test_publish_to_takes_a_registry_url_or_a_path(monkeypatch, child_builds, value): - # An empty value would publish into the current directory. - result = runner.invoke(app, ["build", GRAND.dataset_id, "--publish-to", value]) +def test_a_blank_publish_to_is_a_usage_error_before_any_build(monkeypatch, child_builds): + # A blank value names no registry. Unchecked, it would publish into the current directory. + result = runner.invoke(app, ["build", GRAND.dataset_id, "--publish-to", " "]) assert result.exit_code == 1, result.output assert isinstance(result.exception, TimeNetRegistryError) @@ -773,28 +773,6 @@ def test_publish_to_takes_a_registry_url_or_a_path(monkeypatch, child_builds, va assert child_builds == [] -def test_publish_to_shows_a_settings_error_with_its_own_message(monkeypatch, child_builds): - monkeypatch.setenv("TIMENET_AUTO_BUILD", "maybe") - - result = runner.invoke(app, ["build", GRAND.dataset_id, "--publish-to", "timenet://"]) - - assert isinstance(result.exception, TimeFValidationError), result.output - assert "TIMENET_AUTO_BUILD" in str(result.exception) - assert child_builds == [] - - -def test_the_requested_release_builds_with_no_auto_build(tmp_path, monkeypatch, child_builds): - cards = _install_cards(monkeypatch, child={}, parent={}) - monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) - store = tmp_path / "store" - - result = runner.invoke(app, ["build", CHILD.dataset_id, "--local-registry", str(store), "--no-auto-build"]) - - assert result.exit_code == 0, result.output - assert result.stdout == f"{release_dir(store, CHILD)}\n" - assert child_builds == [(CHILD.dataset_id, False)] - - def _files(root: Path) -> dict[Path, tuple[int, int]]: """Return the size and the modification time of each file under ``root``, by its path.""" return {path: (path.stat().st_size, path.stat().st_mtime_ns) for path in root.rglob("*") if path.is_file()} @@ -988,44 +966,15 @@ def test_plan_lines_name_read_steps(tmp_path): def test_dry_run_prints_the_plan_and_changes_nothing(tmp_path, monkeypatch): cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) - remote, _, calls = _publish_to(tmp_path, monkeypatch, cards) - store = tmp_path / "store" - result = runner.invoke( - app, - [ - "build", - CHILD.dataset_id, - "--local-registry", - str(store), - "--dry-run", - "--publish-to", - "s3://bucket/registry", - ], - ) + remote, store, calls = _publish_to(tmp_path, monkeypatch, cards) + result = runner.invoke(app, ["build", CHILD.dataset_id, "--dry-run", "--publish-to", "s3://bucket/registry"]) assert result.exit_code == 0, result.output assert result.stdout.splitlines() == [ "test/child@1.0.0: build with installed connectors", "└── test/parent@1.0.0: build with installed connectors", f"then publish test/child@1.0.0 to {remote}", ] - assert calls == [] and not store.exists() and not remote.exists(CHILD.dataset_id, "1.0.0") - - -def test_the_dry_run_lines_equal_the_plan_lines_of_plan_build(tmp_path, monkeypatch, child_builds): - cards = _install_cards(monkeypatch, child={"parent": PARENT, "grand": GRAND}, parent={}) - monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) - origin = LocalRegistry(tmp_path / "origin") - _store_from_card(PARENT.dataset_id, origin.root, force=False) - monkeypatch.setenv("TIMENET_REMOTE_REGISTRY", str(origin.root)) - - result = runner.invoke(app, ["build", CHILD.dataset_id, "--dry-run"]) - - # The command and the public API plan alike: a copy of PARENT, a build of GRAND, and the build of CHILD. - assert result.exit_code == 0, result.output - lines = _plan_lines(timenet.plan_build(CHILD.dataset_id)) - assert len(lines) == 3 - assert result.stdout.splitlines() == lines - assert child_builds == [] + assert calls == [] and not store.root.exists() and not remote.exists(CHILD.dataset_id, "1.0.0") def test_a_build_prints_the_plan_before_the_first_step(tmp_path, monkeypatch, child_builds): @@ -1049,18 +998,14 @@ def test_a_build_prints_the_plan_before_the_first_step(tmp_path, monkeypatch, ch assert child_builds == [(CHILD.dataset_id, False)] -@pytest.mark.parametrize("quiet", [False, True]) -def test_quiet_hides_the_plan(tmp_path, monkeypatch, child_builds, quiet): +def test_quiet_hides_the_plan(tmp_path, monkeypatch, child_builds): cards = _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) - result = runner.invoke(app, [*(["--quiet"] if quiet else []), "build", CHILD.dataset_id]) + result = runner.invoke(app, ["--quiet", "build", CHILD.dataset_id]) assert result.exit_code == 0, result.output - if quiet: - assert result.stderr == "" - else: - assert "📋 Plan for 'test/child' 1.0.0:" in result.stderr.splitlines() + assert result.stderr == "" assert child_builds == [(PARENT.dataset_id, False), (CHILD.dataset_id, False)] diff --git a/packages/timenet-connectors/tests/test_end_to_end.py b/packages/timenet-connectors/tests/test_end_to_end.py index 32c42ee19..7066215fa 100644 --- a/packages/timenet-connectors/tests/test_end_to_end.py +++ b/packages/timenet-connectors/tests/test_end_to_end.py @@ -51,6 +51,8 @@ def test_build_then_load_round_trips(converted, tmp_path): ) assert result.exit_code == 0, result.output assert (registry / "timenet" / "hello-world" / "1.0.0" / "manifest.json").exists() + # The flag overrides TIMENET_LOCAL_REGISTRY, which the test setup points at another directory. + assert not settings().local_registry_dir.exists() # CONSUME: load it back through the SDK and compare to the dataset the build wrote. restored = TimeNet(sources=[str(registry)]).load("timenet/hello-world") @@ -58,11 +60,6 @@ def test_build_then_load_round_trips(converted, tmp_path): assert_datasets_equal(original, restored) -def test_build_unknown_id_fails(tmp_path): - result = runner.invoke(build_app, ["build", "acme/not_a_dataset", "--local-registry", str(tmp_path / "registry")]) - assert result.exit_code != 0 - - # ---- where a bare `build` writes ----------------------------------------------------------------- @@ -77,18 +74,6 @@ def test_build_honors_timenet_local_registry(monkeypatch, tmp_path): assert TimeNet().load("timenet/hello-world@1.0.0").metadata.dataset_id == "timenet/hello-world" -def test_local_registry_flag_overrides_timenet_local_registry(monkeypatch, tmp_path): - monkeypatch.setenv("TIMENET_LOCAL_REGISTRY", str(tmp_path / "elsewhere")) - store = tmp_path / "store" - - result = runner.invoke( - build_app, ["build", "timenet/hello-world", "--local-registry", str(store), "--no-isolation"] - ) - assert result.exit_code == 0 - assert (store / "timenet" / "hello-world" / "1.0.0" / "manifest.json").exists() - assert not (tmp_path / "elsewhere").exists() - - def test_build_cleans_cache_but_keep_flag_retains(tmp_path): cache = settings().downloads_dir / "timenet" / "hello-world" diff --git a/packages/timenet/tests/test_ui.py b/packages/timenet/tests/test_ui.py index 1bbfb47b8..1cadcfa90 100644 --- a/packages/timenet/tests/test_ui.py +++ b/packages/timenet/tests/test_ui.py @@ -55,11 +55,13 @@ def test_quiet_suppresses_status_but_not_errors(console, capsys): console.quiet = True console.status("📥", "downloading") console.success("done") + console.detail("a plan line") console.warn("heads up") console.error("boom") captured = capsys.readouterr() assert "downloading" not in captured.err assert "done" not in captured.err + assert "a plan line" not in captured.err assert "heads up" in captured.err # warnings/errors ignore quiet assert "boom" in captured.err @@ -137,9 +139,3 @@ def test_detail_prints_the_line_as_it_is(console, capsys): captured = capsys.readouterr() assert captured.out == "" assert captured.err == f"{line}\n" - - -def test_quiet_suppresses_detail(console, capsys): - console.quiet = True - console.detail("test/child@1.0.0: build with installed connectors") - assert capsys.readouterr().err == ""