diff --git a/examples/load_tsqa.py b/examples/load_tsqa.py index 31e9a5763..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" --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. +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 dc39a036a..dd6e88260 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,239 @@ 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: + """Explain how to obtain a missing parent release. Args: - fetch_from: The value of ``--fetch-from``, or ``None``. - build_parents: Whether ``--build-parents`` is set. + has_connectors: Whether the source list includes installed connectors. Returns: - A sentence naming unused flags, or ``None`` if both are set. + Instructions to find the parent, including ``--auto-build`` when connectors are absent. """ - 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: + """Explain how to resolve a release access error. 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 publish destination, or ``None``. + has_connectors: Whether the source list includes installed connectors. Returns: - The local version directory or remote version string. + 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 ( + 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: + """Format registry options as shell arguments. + + Args: + local_registry: The local registry path, or ``None``. + remote_registry: The remote registry URL, or ``None``. + + Returns: + 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) + + +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"} +"""Display text for each plan action.""" + +_BRANCHES = {"├── ": "│ ", "└── ": " "} +"""Indentation below each tree branch.""" + + +def _plan_lines(plan: Plan) -> list[str]: + """Format a build plan as a tree with the requested release at the top. + + Each line shows a release, its action, and its source. + Repeated releases show ``see above``. + + Args: + plan: The build plan. + + Returns: + 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} + 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..c3f363a21 100644 --- a/packages/timenet-connectors/tests/test_build_plan.py +++ b/packages/timenet-connectors/tests/test_build_plan.py @@ -1,15 +1,17 @@ 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 +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 +20,8 @@ TimeNetAccessError, TimeNetFileError, TimeNetMissingParentError, + TimeNetRegistryError, + TimeNetVersionConflictError, ) from timenet.reader import TimeFReader from timenet.registry import LocalRegistry, WritableRegistry, open_local_version @@ -32,7 +36,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 +528,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 +587,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 +597,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 +674,161 @@ 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) + _, store, calls = _publish_to(tmp_path, monkeypatch, cards) + command = ["build", CHILD.dataset_id, "--publish-to", "s3://bucket/registry"] - command = ["build", CHILD.dataset_id, "--out", "s3://bucket/registry", "--fetch-from", str(origin.root)] + result = runner.invoke(app, [*command, "--remote-registry", str(origin.root), "--no-auto-build"]) - result = runner.invoke(app, command) - - # 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={}) + # 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") + + 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)) + + +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) + assert str(result.exception).startswith("--publish-to ") + assert child_builds == [] + + 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 +894,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 +932,101 @@ 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, 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.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): + 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)] + + +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", "build", CHILD.dataset_id]) + + assert result.exit_code == 0, result.output + assert result.stderr == "" + 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..7066215fa 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,9 +46,13 @@ 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() + # 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") @@ -57,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", "--out", str(tmp_path / "registry")]) - assert result.exit_code != 0 - - # ---- where a bare `build` writes ----------------------------------------------------------------- @@ -76,38 +74,18 @@ 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): - monkeypatch.setenv("TIMENET_LOCAL_REGISTRY", str(tmp_path / "elsewhere")) - out = tmp_path / "out" - - result = runner.invoke(build_app, ["build", "timenet/hello-world", "--out", str(out), "--no-isolation"]) - assert result.exit_code == 0 - assert (out / "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..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") @@ -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..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 @@ -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: + """Print a plain text line without wrapping. + + Quiet mode hides the line. + """ + 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..751399033 100644 --- a/packages/timenet/src/timenet/client.py +++ b/packages/timenet/src/timenet/client.py @@ -544,22 +544,22 @@ 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. 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..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 @@ -128,3 +130,12 @@ 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"