From 623c93d48984b9dd665c72a01e24bce8d2054e69 Mon Sep 17 00:00:00 2001 From: Adrian Blumer Date: Mon, 9 Nov 2026 10:43:58 +0100 Subject: [PATCH] feat(hints)!: give every error one hint for the SDK and one for the CLI The remedy of an error lived in many places. Some messages carried it, and the client and timenet-build each had their own hint helpers. The timenet CLI showed the advice of the SDK, such as "add InstalledConnectors()". Some hints named a command that fails in that situation. download(), timenet.build() and timenet.publish() gave no hint for a missing parent. An error message now states facts only. TimeNetError has a hint, and str() puts it after the message. The new module timenet._hints picks the hint from the facts of the call: the sources, the local registry and the request. A HintWording gives the parts of the hint that name calls or commands. A context variable holds the current wording. Its default is SdkWording, which names SDK calls. Each command of both CLIs sets CliWording with use_wording(), and CliWording names commands with the registry flags that the command got. So the client, timenet.build() and publish() compute each hint once, in the words of the front end. A command that calls no SDK function, such as timenet local clear, adds the hint itself. The hint code picks the remedy by the class of the error, and a subclass without a remedy of its own gets the remedy of its nearest base class. A lookup that fails with a registry, format or card error gives no hint. Any other error of the lookup surfaces, so a bug in it does not hide. A hint names only a call or a command that works. So it looks at the local registry, the installed connectors and the registries that the call already has, and at the versions that they provide. It opens no other registry: - A release that no source provides names a registry to add. It names the installed connectors only if a connector builds the release, and a connector package if none is installed. - A publish of a release that the local registry lacks names the build if an installed connector builds the release, else the download. It asks no registry, so it does not check that a registry holds the release. - A damaged copy in the local registry names a download from a registry, else a forced build, else a registry to add. A failed transfer gets no hint. - A download that only a connector can serve, for example of "latest" when a connector declares a newer version than any registry, names the build and the exact version that a registry holds. - A version conflict with the local registry names the copy to delete. A conflict on publish names a new version. A conflict that timenet-build build --publish-to finds before the build names --force, which compares a fresh build with the published release. - A read or a download of "latest" that cannot reach a registry names the version in the local registry. - A stored manifest that is not valid names the folder to delete, because a download or a build reads that manifest first. - A provenance record in the local registry that is not valid names the forced download and the forced build. Both replace the release and its record. The info and the clear commands of timenet local give this hint. - An isolated build that cannot make its environment names --no-isolation or TIMENET_ISOLATION=off. - A list or a search whose sources have no catalog names the read of an exact release: open('@') in the SDK and timenet info @ in the CLI. Four errors mark some of these situations. TimeNetUnreachableError, a subclass of TimeNetRegistryError, marks a registry that a request cannot reach. TimeNetManifestError marks a stored manifest that is not valid, and its path names the file. TimeNetOriginError marks a provenance record that is not valid. Both are subclasses of TimeFFormatError. TimeNetIsolationError, a subclass of TimeNetBuildError, marks an environment that an isolated build cannot make. LocalRegistry.release_at() finds the release whose folder holds a path. TimeNetVersionConflictError gives the location of the destination registry in registry_location, so a hint can compare it with the local registry. Its difference is a ReleaseDifference, new in timenet.errors: FILES or CARD. The difference is now a required argument, and it comes before needed_by. Manifest.difference_from() compares two manifests of a release and gives this value. same_release() is gone. A registry flag that names no registry (--local-registry, --remote-registry, --publish-to, --dest, --source) is now a usage error. It exits with code 2 and names the flag. A bad TIMENET_* variable still exits with code 1 and names the variable. timenet-build build --publish-to now compares the release in the local registry with the release in the destination. For another release, it raises TimeNetVersionConflictError before the build. If the local registry lacks the release, the version in the destination is enough, and the command says "Nothing to do". With --force, the command builds the release again, and the publish compares the files. The root help of timenet says what the tool does. The help of timenet-build build no longer shows its Raises section. Every command has the same help for --local-registry: the local registry, where downloads and builds land and which a lookup checks first. BREAKING CHANGE: error messages no longer contain remedies. The remedy is in error.hint, which str() shows after the message. Part of TNET-157. --- .../src/timenet_connectors/builder/cli.py | 166 ++--- .../src/timenet_connectors/builder/env.py | 18 +- .../tests/test_build_plan.py | 143 ++-- packages/timenet/src/timenet/_hints.py | 622 ++++++++++++++++++ packages/timenet/src/timenet/builders.py | 5 + packages/timenet/src/timenet/cli/app.py | 158 ++--- packages/timenet/src/timenet/cli/hints.py | 73 ++ packages/timenet/src/timenet/cli/options.py | 4 +- packages/timenet/src/timenet/client.py | 241 ++----- packages/timenet/src/timenet/config.py | 8 +- .../timenet/src/timenet/coordinator/plan.py | 3 +- packages/timenet/src/timenet/errors.py | 144 ++-- .../timenet/src/timenet/manifest/__init__.py | 3 +- .../timenet/src/timenet/manifest/manifest.py | 42 +- packages/timenet/src/timenet/producer.py | 83 ++- .../timenet/src/timenet/registry/_copy.py | 20 +- .../timenet/src/timenet/registry/local.py | 39 +- .../src/timenet/registry/remote/_http.py | 6 +- packages/timenet/src/timenet/registry/s3.py | 6 +- packages/timenet/tests/registry/test_copy.py | 15 +- .../timenet/tests/registry/test_factory.py | 5 +- .../timenet/tests/registry/test_remote.py | 4 - packages/timenet/tests/registry/test_s3.py | 15 + packages/timenet/tests/test_cli.py | 244 +++++-- packages/timenet/tests/test_client.py | 199 ++++-- packages/timenet/tests/test_config.py | 21 +- packages/timenet/tests/test_errors.py | 36 +- packages/timenet/tests/test_hints.py | 104 +++ packages/timenet/tests/test_producer.py | 31 +- 29 files changed, 1746 insertions(+), 712 deletions(-) create mode 100644 packages/timenet/src/timenet/_hints.py create mode 100644 packages/timenet/src/timenet/cli/hints.py create mode 100644 packages/timenet/tests/test_hints.py diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py index 8be73fadb..b18703c55 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/cli.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/cli.py @@ -8,26 +8,28 @@ import json from pathlib import Path -import shlex -from typing import cast import typer from timenet import publish +from timenet._hints import ( # ruff: ignore[import-private-name] + CliWording, + Facts, + Operation, + RequestedRelease, + explain, + use_wording, +) from timenet.builders import BuildOptions, declared +from timenet.cli.hints import local_registry_from_flag, registry_flag, sources_from_flags 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.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, -) +from timenet.errors import TimeNetVersionConflictError from timenet.manifest import PayloadSource -from timenet.registry import LocalRegistry, WritableRegistry, open_registry +from timenet.registry import LocalRegistry, WritableRegistry 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 @@ -57,7 +59,8 @@ def build( isolation: bool | None = typer.Option( None, "--isolation/--no-isolation", - help="Run the build in an environment built from the connector's requirements.", + help="Run the build in an environment built from the connector's requirements " + "(default: $TIMENET_ISOLATION, else on).", ), publish_to: str | None = typer.Option( None, @@ -91,9 +94,10 @@ def build( 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. + \f Raises: - BadParameter: If ``dataset_id`` has no known connector. + BadParameter: If ``dataset_id`` has no known connector, or a registry flag names no registry. 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 @@ -102,35 +106,36 @@ def build( ``open``. 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 publish raises TimeNetVersionConflictError + """ # ruff: ignore[docstring-extraneous-exception, escape-sequence-in-docstring] - raised by the build and the publish; the form feed ends the help text # 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)'}") - # 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. - local = 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), - target_registry=local, - parents_from=sources, - force=force, - open_access_only=dest is not None and dest.requires_open_access, - ) + # The build writes into the local registry, which starts the list. The planner leaves it out of the sources of + # the copies and builds. + local = local_registry_from_flag(local_registry) + sources = sources_from_flags(local, remote_registry, auto_build) + dest = None if publish_to is None else registry_flag(publish_to, "--publish-to") + facts = Facts(sources, local, request=Operation.BUILD, dest=dest, wanted=RequestedRelease(dataset_id, version)) + with use_wording(CliWording(local_registry, remote_registry)): + with explain(facts): + # 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 + if _already_published(wanted, local, 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), + target_registry=local, + 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: @@ -149,93 +154,44 @@ def build( quiet=console.quiet, isolation=None if isolation is None else ("on" if isolation else "off"), ) - run_plan(plan, local, options=options) + with explain(facts): + run_plan(plan, local, options=options) if dest is not None: + # The publish gives its own hint, the hint of a publish. publish(wanted, dest=dest, source=local, progress=progress) - except TimeNetMissingParentError as error: - 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"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 = _access_hint(dest, has_connectors=has_connectors) - raise if dest is None and not plan.steps: console.success(f"Nothing to do: '{wanted.dataset_id}' {wanted.version} is already in {local}") # 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(local.root, wanted))) -def _missing_hint(*, has_connectors: bool) -> str: - """Explain how to obtain a missing parent release. - - Args: - has_connectors: Whether the source list includes installed connectors. - - Returns: - 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 " - "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: - dest: The publish destination, or ``None``. - has_connectors: Whether the source list includes installed connectors. - - Returns: - 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 - - -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: +def _already_published( + release: DatasetRef, local_registry: LocalRegistry, 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. + local_registry: The local registry to build into. dest: The publish destination, or ``None``. - force: Whether the build runs again anyway. + force: Whether the build runs again anyway, so that the publish compares its files with ``dest``. Returns: ``True`` if ``dest`` holds the release and ``force`` is false. + + Raises: + TimeNetVersionConflictError: If the local registry holds the release and ``dest`` holds it with other + files or another card. Nothing is built then. """ - return dest is not None and not force and dest.exists(release.dataset_id, str(release.version)) + published = None if dest is None or force else dest.find_manifest(release) + if dest is None or published is None: + return False + held = local_registry.find_manifest(release) + # If the local registry holds the release, the destination must hold the same one. + difference = None if held is None else held.difference_from(published) + if difference is not None: + raise TimeNetVersionConflictError(release, str(dest), dest.location, str(local_registry), difference) + return True def _show_plan(release: DatasetRef, plan: Plan) -> None: diff --git a/packages/timenet-connectors/src/timenet_connectors/builder/env.py b/packages/timenet-connectors/src/timenet_connectors/builder/env.py index f8af26ef7..c325c035b 100644 --- a/packages/timenet-connectors/src/timenet_connectors/builder/env.py +++ b/packages/timenet-connectors/src/timenet_connectors/builder/env.py @@ -23,7 +23,7 @@ from uv import find_uv_bin -from timenet.errors import TimeNetBuildError +from timenet.errors import TimeNetBuildError, TimeNetIsolationError from timenet.manifest import PayloadSource from timenet.types.references import DatasetRef from timenet_connectors.discovery import requirements_for @@ -58,7 +58,7 @@ def env_spec(dataset_id: str) -> EnvSpec: Raises: LookupError: If no connector exists for the id. - TimeNetBuildError: If a base distribution is not installed. + TimeNetIsolationError: If a base distribution is not installed. """ # ruff: ignore[docstring-extraneous-exception] (raised by requirements_for and _base_args, not directly here) base = _base_args() # The parent cannot read the connector's backend before the environment exists, so the @@ -83,15 +83,12 @@ def uv_command(spec: EnvSpec, argv: Sequence[str]) -> list[str]: The full argument vector. Raises: - TimeNetBuildError: If the uv executable is missing. + TimeNetIsolationError: If the uv executable is missing. """ try: uv = find_uv_bin() except FileNotFoundError as exc: - raise TimeNetBuildError( - "cannot run an isolated build: no uv executable found. Install uv, or build in this " - "interpreter with TIMENET_ISOLATION=off." - ) from exc + raise TimeNetIsolationError("cannot run an isolated build: no uv executable found.") from exc command = [uv, "run", "--no-project", "--python", spec.python, *spec.base] if spec.requirements is not None: command += ["--with-requirements", str(spec.requirements)] @@ -206,16 +203,15 @@ def _base_args() -> tuple[str, ...]: Alternating flag/value arguments for uv. Raises: - TimeNetBuildError: If a base distribution is not installed in this interpreter. + TimeNetIsolationError: If a base distribution is not installed in this interpreter. """ args: list[str] = [] for name in BASE_DISTRIBUTIONS: try: dist = Distribution.from_name(name) except PackageNotFoundError as exc: - raise TimeNetBuildError( - f"cannot build a build environment: {name} is not installed in {sys.executable}. " - "Reinstall it, or build in this interpreter with TIMENET_ISOLATION=off." + raise TimeNetIsolationError( + f"cannot make a build environment: {name} is not installed in {sys.executable}." ) from exc source = _local_source_path(dist) if source is not None: diff --git a/packages/timenet-connectors/tests/test_build_plan.py b/packages/timenet-connectors/tests/test_build_plan.py index cfc5b3c3a..453662bd8 100644 --- a/packages/timenet-connectors/tests/test_build_plan.py +++ b/packages/timenet-connectors/tests/test_build_plan.py @@ -1,13 +1,17 @@ +import json 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 +import timenet from timenet import builders, client as client_module +from timenet.cli import hints as hints_module from timenet.client import default_local_registry from timenet.connectors import BaseConnector from timenet.coordinator.build import plan_release_build, run_plan @@ -15,12 +19,13 @@ from timenet.dataset import Record, RegularAxis, Signal, Source, TimeFDataset from timenet.engine import open_parents, run_pipeline from timenet.errors import ( + ReleaseDifference, TimeFFormatError, TimeFValidationError, TimeNetAccessError, TimeNetFileError, + TimeNetIsolationError, TimeNetMissingParentError, - TimeNetRegistryError, TimeNetVersionConflictError, ) from timenet.reader import TimeFReader @@ -614,30 +619,25 @@ def child(command, env): assert local_registry.exists(GRAND.dataset_id, str(GRAND.version)) -_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"]) +@pytest.mark.parametrize("case", ["missing", "missing-buildable", "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)) local_registry = LocalRegistry(tmp_path / "store") options = [] + if case in {"missing", "missing-buildable"}: + # No registry holds the parent, and the list has no connectors. + options = ["--no-auto-build"] if case == "missing": - # No registry holds the parent, and no installed connector builds it. + # No installed connector builds the parent. del cards[PARENT.dataset_id] - if case == "missing-without-auto-build": - options = ["--no-auto-build"] if case == "damaged": local_registry.put_dataset(TimeFDataset(metadata=cards[PARENT.dataset_id])) (release_dir(local_registry.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, "open_registry", lambda url, **_: _Hosted(tmp_path / "hosted")) + monkeypatch.setattr(hints_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. @@ -652,28 +652,25 @@ def test_the_cli_hint_names_its_flags(tmp_path, monkeypatch, case): remote = str(tmp_path / "remote registry") flags = f" --local-registry {shlex.quote(str(local_registry.root))} --remote-registry {shlex.quote(remote)}" hints = { - "missing": (TimeNetMissingParentError, f". {_COPY_HINT[0].upper()}{_COPY_HINT[1:]}"), - "missing-without-auto-build": ( + # No installed connector builds the parent. + "missing": (TimeNetMissingParentError, "Pass a registry that holds it with --remote-registry ."), + "missing-buildable": ( TimeNetMissingParentError, - 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, - 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.", + "Pass a registry that holds it with --remote-registry , or build it from its installed connector " + "with --auto-build.", ), + # No registry provides the parent, so the hint names no download. + "damaged": (TimeNetFileError, f"Build it again with `timenet-build build test/parent{flags} --force`."), "not-open": ( TimeNetAccessError, - ". _Hosted serves open releases only, so --publish-to takes open releases only. " + "_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."), + "not-served": (TimeNetAccessError, "Build it locally from its installed connector with --auto-build."), } error, hint = hints[case] assert isinstance(result.exception, error), result.output - assert str(result.exception).endswith(hint) + assert result.exception.hint == hint def _publish_to(tmp_path, monkeypatch, cards) -> tuple[_Hosted, LocalRegistry, list]: @@ -683,7 +680,10 @@ def _publish_to(tmp_path, monkeypatch, cards) -> tuple[_Hosted, LocalRegistry, l The registry that ``--publish-to`` opens, the local registry, and the list of the builds. """ remote = _Hosted(tmp_path / "hosted") - monkeypatch.setattr(cli_module, "open_registry", lambda url, **_: remote) + opened = hints_module.open_registry + monkeypatch.setattr( + hints_module, "open_registry", lambda url, **kw: remote if url == "s3://bucket/registry" else opened(url, **kw) + ) monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) monkeypatch.setenv("TIMENET_LOCAL_REGISTRY", str(tmp_path / "store")) calls = [] @@ -730,13 +730,6 @@ def test_publish_to_builds_into_the_local_registry_and_publishes(tmp_path, monke for registry in (local_registry, remote): assert registry.exists(CHILD.dataset_id, str(CHILD.version)) assert registry.exists(PARENT.dataset_id, str(PARENT.version)) - # A destination that holds the version gets nothing, and nothing builds. - shutil.rmtree(local_registry.root) - result = runner.invoke(app, command) - 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_registry.root.exists() def test_publish_to_copies_parents_only_from_the_sources(tmp_path, monkeypatch): @@ -752,7 +745,6 @@ def test_publish_to_copies_parents_only_from_the_sources(tmp_path, monkeypatch): 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)]) @@ -784,12 +776,53 @@ def test_a_blank_publish_to_is_a_usage_error_before_any_build(monkeypatch, child # 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 ") + # A usage error that names the flag. + assert result.exit_code == 2, result.output + assert "Invalidvalue:--publish-to" in "".join(click.unstyle(result.output).split()) assert child_builds == [] +def test_publish_to_a_destination_that_holds_another_card_fails_before_any_build(tmp_path, monkeypatch): + cards = _install_cards(monkeypatch, child={}, parent={}) + remote, local_registry, calls = _publish_to(tmp_path, monkeypatch, cards) + # The destination holds the files of the local registry, under another card. + published = release_dir(remote.local_root, CHILD) + shutil.copytree(_store_from_card(CHILD.dataset_id, local_registry.root, force=False), published) + manifest = json.loads((published / "manifest.json").read_text()) + manifest["metadata"]["description"] = "Another card." + (published / "manifest.json").write_text(json.dumps(manifest)) + + result = runner.invoke(app, ["build", CHILD.dataset_id, "--publish-to", "s3://bucket/registry"]) + + assert isinstance(result.exception, TimeNetVersionConflictError), result.output + assert result.exception.difference is ReleaseDifference.CARD + assert result.exception.hint == ( + "Build it again with `--force` to compare a fresh build with the published release, or give a changed release " + "a new version." + ) + assert "Nothing to do" not in click.unstyle(result.output) + assert calls == [] + + +def test_publish_to_a_destination_that_holds_the_version_builds_nothing_without_force(tmp_path, monkeypatch): + cards = _install_cards(monkeypatch, child={}, parent={}) + remote, local_registry, 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)) + # The destination holds the version, and the local registry does not. + shutil.copytree( + _store_from_card(CHILD.dataset_id, tmp_path / "elsewhere", force=False), release_dir(remote.local_root, CHILD) + ) + + result = runner.invoke(app, ["build", CHILD.dataset_id, "--publish-to", "s3://bucket/registry"]) + + assert result.exit_code == 0, result.output + assert click.unstyle(result.stdout) == f"{CHILD.version}\n" + assert click.unstyle(result.stderr).splitlines() == [f"✅ Nothing to do: 'test/child' 1.0.0 is already in {remote}"] + assert calls == [] + assert not local_registry.exists(CHILD.dataset_id, str(CHILD.version)) + + def _files(root: Path) -> dict[Path, tuple[int, int]]: """Return the size and the modification time of each file under ``root``, by its path.""" return {path: (path.stat().st_size, path.stat().st_mtime_ns) for path in root.rglob("*") if path.is_file()} @@ -818,6 +851,8 @@ def test_force_with_publish_to_rebuilds_and_the_publish_refuses_other_files(tmp_ _change_the_next_build(monkeypatch) # the next build writes other files result = runner.invoke(app, [*command, "--force"]) assert isinstance(result.exception, TimeNetVersionConflictError) + # The conflict comes from the publish, so it gets the hint of a publish. + assert result.exception.hint == "A published version cannot change, so a changed release needs a new version." 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 @@ -848,6 +883,30 @@ def copy_the_saved_release(release, store_root, *, force=False, **_): assert _files(remote.local_root) == published +@pytest.mark.parametrize( + ("entry", "hint"), + [ + ("cli", "Build in this interpreter with --no-isolation."), + ("sdk", "Build in this interpreter with TIMENET_ISOLATION=off."), + ], +) +def test_an_environment_that_cannot_be_made_names_the_build_in_this_interpreter(tmp_path, monkeypatch, entry, hint): + cards = _install_cards(monkeypatch, parent={}) + monkeypatch.setattr(cli_module, "known_ids", lambda: list(cards)) + monkeypatch.setattr(env_module, "BASE_DISTRIBUTIONS", ("timenet-not-installed",)) + local_registry = tmp_path / "store" + + if entry == "cli": + error = runner.invoke(app, ["build", PARENT.dataset_id, "--local-registry", str(local_registry)]).exception + else: + with pytest.raises(TimeNetIsolationError) as caught: + timenet.build(PARENT.dataset_id, local_registry=local_registry) + error = caught.value + + assert isinstance(error, TimeNetIsolationError) + assert error.hint == hint + + def test_the_plan_drops_the_target_registry_from_parents_from(tmp_path, monkeypatch): _install_cards(monkeypatch, child={"parent": PARENT}, parent={}) target_registry = LocalRegistry(tmp_path / "store") @@ -1030,14 +1089,17 @@ def test_quiet_hides_the_plan(tmp_path, monkeypatch, child_builds): assert child_builds == [(PARENT.dataset_id, False), (CHILD.dataset_id, False)] -@pytest.mark.parametrize("held_in", ["local-registry", "destination", "local-registry-only"]) +@pytest.mark.parametrize("held_in", ["local-registry", "both", "local-registry-only", "destination-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, local_registry, 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 local_registry - _store_from_card(CHILD.dataset_id, held_by.root if held_by is local_registry else remote.local_root, force=False) + directory = _store_from_card(CHILD.dataset_id, local_registry.root, force=False) + if held_in in {"both", "destination-only"}: + shutil.copytree(directory, release_dir(remote.local_root, CHILD)) + if held_in == "destination-only": + shutil.rmtree(local_registry.root) options = [] if held_in == "local-registry" else ["--publish-to", "s3://bucket/registry"] result = runner.invoke(app, ["build", CHILD.dataset_id, "--dry-run", *options]) @@ -1048,6 +1110,7 @@ def test_a_dry_run_of_a_held_release_says_nothing_to_do(tmp_path, monkeypatch, h assert result.stdout.splitlines() == [f"then publish test/child@1.0.0 to {remote}"] assert result.stderr == "" else: + held_by = local_registry if held_in == "local-registry" else remote 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/src/timenet/_hints.py b/packages/timenet/src/timenet/_hints.py new file mode 100644 index 000000000..de053a2b9 --- /dev/null +++ b/packages/timenet/src/timenet/_hints.py @@ -0,0 +1,622 @@ +"""Suggest SDK calls or CLI commands to resolve TimeNet errors.""" + +from abc import ABC, abstractmethod +from collections.abc import Callable, Generator, Sequence +from contextlib import contextmanager +from contextvars import ContextVar +from dataclasses import dataclass +from enum import Enum, auto +import shlex +from typing import Any, TypeAlias + +from typing_extensions import override + +from timenet import builders +from timenet._fs import same_directory +from timenet.config import settings +from timenet.errors import ( + TimeFFormatError, + TimeNetAccessError, + TimeNetDatasetNotFoundError, + TimeNetError, + TimeNetFileError, + TimeNetInvalidCardError, + TimeNetIsolationError, + TimeNetManifestError, + TimeNetMissingParentError, + TimeNetNoCatalogError, + TimeNetNotBuiltError, + TimeNetOriginError, + TimeNetRegistryError, + TimeNetUnreachableError, + TimeNetVersionConflictError, + TimeNetVersionUnavailableError, +) +from timenet.registry import BaseRegistry, LocalRegistry, WritableRegistry +from timenet.registry._paths import release_dir +from timenet.registry.local import is_same_registry +from timenet.sources import Found, InstalledConnectors, ReleaseSource, resolve_release +from timenet.types.references import LATEST_VERSION, DatasetRef, split_ref + + +_PACKAGE = "a connector package that builds it, such as timenet-connectors" + + +class Operation(Enum): + """What a call does.""" + + READ = auto() + DOWNLOAD = auto() + BUILD = auto() + PUBLISH = auto() + CLEAR = auto() + + +@dataclass(frozen=True) +class RequestedRelease: + """A requested dataset ID, with an optional version.""" + + dataset_id: str + """The dataset ID.""" + version: str | None = None + """The requested version or ``"latest"``, or ``None`` without a version.""" + + @classmethod + def parse(cls, value: "str | DatasetRef") -> "RequestedRelease": + """Read a dataset ID with an optional ``@version``, or an exact release. + + Args: + value: The dataset ID with an optional ``@version``, or the release. + + Returns: + The requested release. + + Raises: + TimeFValidationError: If ``value`` is not a valid reference. + """ # ruff: ignore[docstring-extraneous-exception] (raised by split_ref) + if isinstance(value, DatasetRef): + return cls(value.dataset_id, str(value.version)) + return cls(*split_ref(value)) + + def __str__(self) -> str: + return self.dataset_id if self.version is None else f"{self.dataset_id}@{self.version}" + + +@dataclass(frozen=True) +class Facts: + """Details of an operation for choosing an error hint.""" + + sources: Sequence[ReleaseSource] + """The sources of the call in order, with the local registry.""" + local_registry: LocalRegistry | None + """The local registry that the call uses, or ``None`` if it uses none.""" + request: Operation = Operation.READ + """What the call does.""" + dest: WritableRegistry | None = None + """The registry that the call publishes to, or ``None``.""" + wanted: RequestedRelease | None = None + """The requested release, or ``None``.""" + + +class HintWording(ABC): + """The parts of a hint that name SDK calls or CLI commands.""" + + @abstractmethod + def build(self, dataset_id: str, facts: Facts, *, force: bool = False) -> str: + """Return the call or the command that builds a dataset into the local registry of the call. + + Args: + dataset_id: The dataset ID, optionally with ``@version``. + facts: Details of the operation. + force: Build the release again. + + Returns: + The call or the command. + """ + + @abstractmethod + def download(self, release: DatasetRef | str, facts: Facts, *, force: bool = False) -> str: + """Return the call or the command that copies a release into the local registry. + + Args: + release: The release, or a dataset ID with an optional ``@version``. + facts: Details of the operation. + force: Copy the release again. + + Returns: + The call or the command. + """ + + @abstractmethod + def add_registry(self) -> str: + """Return the start of a sentence that adds a registry that holds a release.""" + + @abstractmethod + def or_build_from_connectors(self, facts: Facts) -> str: + """Return the end of a sentence that builds a release from its installed connector. + + Args: + facts: Details of the operation. + + Returns: + The end of the sentence, from its comma. + """ + + @abstractmethod + def build_locally_from_connectors(self, facts: Facts) -> str: + """Return the sentence that builds a release locally from its installed connector. + + Args: + facts: Details of the operation. + + Returns: + The sentence. + """ + + @abstractmethod + def copy_intact(self, release: DatasetRef, facts: Facts) -> str: + """Return the sentence that copies an intact release from a registry that the call does not have. + + Args: + release: The release. + facts: Details of the operation. + + Returns: + The sentence. + """ + + @abstractmethod + def read_exact_release(self) -> str: + """Return the call or the command that reads a release by its exact ID and version.""" + + @abstractmethod + def leave_unreachable(self) -> str: + """Return the part of a sentence that replaces a registry that the call cannot reach.""" + + @abstractmethod + def run_in_this_interpreter(self) -> str: + """Return the flag or the variable that builds in this interpreter.""" + + @abstractmethod + def publish_to_open_only(self, dest: WritableRegistry) -> str | None: + """Return the hint for a publish target that serves open releases only, or ``None``. + + Args: + dest: The registry that the call publishes to. + + Returns: + The hint, or ``None`` if the front end has no such target. + """ + + +class SdkWording(HintWording): + """The parts of a hint for the SDK: calls and keywords.""" + + @override + def build(self, dataset_id: str, facts: Facts, *, force: bool = False) -> str: + local_registry = facts.local_registry + keywords = "" + if local_registry is not None and not same_directory(local_registry.root, settings().local_registry_dir): + keywords += f", local_registry={str(local_registry.root)!r}" + if force: + keywords += ", force=True" + return f"timenet.build('{dataset_id}'{keywords})" + + @override + def download(self, release: DatasetRef | str, facts: Facts, *, force: bool = False) -> str: + client = "" if facts.request in {Operation.READ, Operation.DOWNLOAD} else "timenet.TimeNet()." + return f"{client}download('{release}'{', force=True' if force else ''})" + + @override + def add_registry(self) -> str: + return "Add a registry that holds it to the sources" + + @override + def or_build_from_connectors(self, facts: Facts) -> str: + return f", or add {_connectors(facts)} to build it" + + @override + def build_locally_from_connectors(self, facts: Facts) -> str: + return f"Add {_connectors(facts)} to the sources to build it locally." + + @override + def copy_intact(self, release: DatasetRef, facts: Facts) -> str: + return f"Add a registry that holds an intact copy to the sources, then call {self.download(release, facts)}." + + @override + def read_exact_release(self) -> str: + return "open('@')" + + @override + def leave_unreachable(self) -> str: + return "leave the unreachable registry out of the sources" + + @override + def run_in_this_interpreter(self) -> str: + return "TIMENET_ISOLATION=off" + + @override + def publish_to_open_only(self, dest: WritableRegistry) -> str | None: + # The SDK names the target of a publish only in publish(), which has a hint of its own. + return None + + +@dataclass(frozen=True) +class CliWording(HintWording): + """The parts of a hint for a CLI: commands and flags, with the registry flags that the command got.""" + + local_registry: str | None = None + """The ``--local-registry`` value that the command got, or ``None``.""" + remote_registry: str | None = None + """The ``--remote-registry`` value that the command got, or ``None``.""" + + def build_command(self, dataset_id: str, *, force: bool = False) -> str: + """Create a ``timenet-build build`` command with the registry flags of the command. + + Args: + dataset_id: The dataset ID, optionally with ``@version``. + force: Add ``--force``. + + Returns: + The command line. + """ + return f"timenet-build build {dataset_id}{self._flags()}{' --force' if force else ''}" + + def download_command( + self, release: DatasetRef | str, *, force: bool = False, remote_registry: str | None = None + ) -> str: + """Create a ``timenet download`` command with the registry flags of the command. + + Args: + release: The release, or a dataset ID with an optional ``@version``. + force: Add ``--force``. + remote_registry: A placeholder such as ```` for ``--remote-registry``, written as it is, or + ``None`` for the flag of the command. + + Returns: + The command line. + """ + return f"timenet download {release}{self._flags(remote_registry)}{' --force' if force else ''}" + + @override + def build(self, dataset_id: str, facts: Facts, *, force: bool = False) -> str: + return f"`{self.build_command(dataset_id, force=force)}`" + + @override + def download(self, release: DatasetRef | str, facts: Facts, *, force: bool = False) -> str: + return f"`{self.download_command(release, force=force)}`" + + @override + def add_registry(self) -> str: + return "Pass a registry that holds it with --remote-registry " + + @override + def or_build_from_connectors(self, facts: Facts) -> str: + return ", or build it from its installed connector with --auto-build" + + @override + def build_locally_from_connectors(self, facts: Facts) -> str: + return "Build it locally from its installed connector with --auto-build." + + @override + def copy_intact(self, release: DatasetRef, facts: Facts) -> str: + command = self.download_command(release, remote_registry="") + return f"Copy an intact release into the local registry from a registry that holds it with `{command}`." + + @override + def read_exact_release(self) -> str: + return f"`timenet info @{self._flags()}`" + + @override + def leave_unreachable(self) -> str: + return "pass a registry that you can reach with --remote-registry " + + @override + def run_in_this_interpreter(self) -> str: + return "--no-isolation" + + @override + def publish_to_open_only(self, dest: WritableRegistry) -> str | None: + return ( + f"{dest} serves open releases only, so --publish-to takes open releases only. Build it into the local " + "registry without --publish-to instead." + ) + + def _flags(self, remote_registry: str | None = None) -> str: + """Return the registry flags of the command, for a command that it suggests.""" + remote = remote_registry or (None if self.remote_registry is None else shlex.quote(self.remote_registry)) + local = None if self.local_registry is None else shlex.quote(self.local_registry) + flags = (("--local-registry", local), ("--remote-registry", remote)) + return "".join(f" {flag} {value}" for flag, value in flags if value is not None) + + +_CURRENT_HINT_WORDING: ContextVar[HintWording] = ContextVar( + "current_hint_wording", + default=SdkWording(), # ruff: ignore[mutable-contextvar-default] (SdkWording holds no state) +) +"""The wording of the hints that :func:`explain` adds: SDK calls, unless a CLI command set its own.""" + + +@contextmanager +def use_wording(wording: HintWording) -> Generator[None]: + """Give the hints of the block the given wording. + + Args: + wording: The calls or the commands that the hints name. + + Yields: + Nothing. + """ + token = _CURRENT_HINT_WORDING.set(wording) + try: + yield + finally: + _CURRENT_HINT_WORDING.reset(token) + + +@contextmanager +def explain(facts: Facts) -> Generator[None]: + """Add a hint in the current wording to a TimeNet error from the block and raise it again. + + An outer block replaces any hint set by an inner block. + + Args: + facts: Details of the operation. + + Yields: + Nothing. + + Raises: + TimeNetError: The error of the block, with its hint. + """ + try: + yield + except TimeNetError as error: + error.hint = hint(error, facts, _CURRENT_HINT_WORDING.get()) + raise + + +def hint(error: TimeNetError, facts: Facts, wording: HintWording) -> str | None: + """Suggest a way to resolve a TimeNet error. + + Return ``None`` if no remedy is known or a lookup fails with a registry, format or card error. + + Args: + error: The error. + facts: Details of the operation. + wording: The calls or the commands that the hint names. + + Returns: + One or two sentences, or ``None``. + """ + try: + return _remedy(error, facts, wording) + except (TimeNetRegistryError, TimeFFormatError, TimeNetInvalidCardError): + return None + + +Remedy: TypeAlias = Callable[[Any, Facts, HintWording], str | None] +"""A function that returns the hint of an error of its class, or None.""" + + +def _remedy(error: TimeNetError, facts: Facts, wording: HintWording) -> str | None: + """Return the hint of the most specific class of the error that has a remedy, or None.""" + for cls in type(error).__mro__: + remedy = _REMEDIES.get(cls) + if remedy is not None: + return remedy(error, facts, wording) + return None + + +def _not_found(facts: Facts, wording: HintWording) -> str | None: + """Return a hint that names a registry, a build or a connector package for the requested release, or None.""" + if facts.wanted is None: + return None + if facts.request is Operation.BUILD: + # Only an installed connector builds the requested release. + return None if builders.has_backends() else f"Install {_PACKAGE}." + return _missing(facts.wanted.dataset_id, facts.wanted.version, facts, wording) + + +def _missing(dataset_id: str, version: str | None, facts: Facts, wording: HintWording) -> str | None: + """Return a hint that names a registry or a build for a release that no source of the call provides.""" + if facts.request is Operation.PUBLISH: + # The release must be in the local registry first. + return _into_local_registry(dataset_id, version, facts, wording) + return f"{wording.add_registry()}{_or_build(dataset_id, version, facts, wording)}." + + +def _or_build(dataset_id: str, version: str | None, facts: Facts, wording: HintWording) -> str: + """Return the second half of a sentence that names the build of a release, or an empty string.""" + if facts.request is Operation.DOWNLOAD: + # A download never builds. + return "" + if not builders.has_backends(): + return f", or install {_PACKAGE}" + if _lists_connectors(facts) or not _builds(dataset_id, version): + return "" + return wording.or_build_from_connectors(facts) + + +def _not_built(release: DatasetRef, facts: Facts, wording: HintWording) -> str: + """Return a hint that names the build of a release that only a connector provides, or a registry version.""" + build = wording.build(release.dataset_id, facts) + if facts.request is Operation.READ: + return f"Read its card with get_card('{release.dataset_id}'), or build it with {build}." + held = None + if ( + facts.request is Operation.DOWNLOAD + and facts.wanted is not None + and facts.wanted.dataset_id == release.dataset_id + ): + # A registry can hold another version of the requested release. + registries = [source for source in facts.sources if isinstance(source, BaseRegistry)] + answer = resolve_release(registries, release.dataset_id, None) + held = DatasetRef(dataset_id=release.dataset_id, version=answer.version) if isinstance(answer, Found) else None + if held is None: + return f"Build it with {build}." + download = wording.download(held, facts) + return f"Build it with {build}, or download the version that a registry holds with {download}." + + +def _version(error: TimeNetVersionUnavailableError, facts: Facts, wording: HintWording) -> str: + """Return a hint that names the version that the installed connector builds.""" + if facts.request is Operation.DOWNLOAD: + # A download never builds, so the version of the connector needs a build first. + return _not_built(DatasetRef(dataset_id=error.release.dataset_id, version=error.declared), facts, wording) + return f"Ask for version {error.declared}, or leave out the version." + + +def _access(release: DatasetRef, facts: Facts, wording: HintWording) -> str: + """Return a hint that names the local build of a release that no source serves, or a registry for it.""" + dest = facts.dest + if dest is not None and dest.requires_open_access: + if facts.request is Operation.PUBLISH: + return f"{dest} serves open releases only. Publish it to a registry of your own, such as a local directory." + refused = wording.publish_to_open_only(dest) + if refused is not None: + return refused + build = wording.build(release.dataset_id, facts) + card = builders.find_card(release.dataset_id) + if card is None or card.dataset_version != release.version: + other = "" if card is None else f"The installed connector builds version {card.dataset_version}. " + return f"{other}Install the connector package that builds {release}, then build it locally with {build}." + if facts.request is Operation.DOWNLOAD or _lists_connectors(facts): + return f"Build it locally with {build}." + return wording.build_locally_from_connectors(facts) + + +def _damaged(release: DatasetRef, facts: Facts, wording: HintWording) -> str | None: + """Return a hint that names the copy or the build that replaces a damaged copy in the local registry, or None.""" + local_registry = facts.local_registry + held = None if local_registry is None else local_registry.find_manifest(release) + if local_registry is None or held is None or local_registry.damage(release, held) is None: + return None + remedy = _into_local_registry(release.dataset_id, str(release.version), facts, wording, again=True) + if remedy is not None: + return remedy + return wording.copy_intact(release, facts) + + +def _into_local_registry( + dataset_id: str, version: str | None, facts: Facts, wording: HintWording, *, again: bool = False +) -> str | None: + """Return a hint that names the copy from a registry, else the build, that puts a release in the local registry.""" + local_registry = facts.local_registry + if local_registry is None: + return None + wanted = str(RequestedRelease(dataset_id, version)) + download = ( + f"Copy {'an intact release' if again else 'it'} into the local registry with {wording.download(wanted, facts)}." + ) + # A publish asks no registry, so its hint names only what the local facts show. + publish = facts.request is Operation.PUBLISH + if not publish and isinstance(resolve_release(_registries(local_registry, facts), dataset_id, version), Found): + return download + if _builds(dataset_id, version): + if again: + return f"Build it again with {wording.build(dataset_id, facts, force=True)}." + return f"Build it into the local registry with {wording.build(wanted, facts)}." + # Without a connector, a publish names the download, but no registry was asked if it holds the release. + return download if publish and not again else None + + +def _conflict(error: TimeNetVersionConflictError, facts: Facts) -> str: + """Return a hint that names the delete of the local copy for a consumer, or a new version for a producer.""" + local_registry = facts.local_registry + if local_registry is not None and error.registry_location == local_registry.location: + # The local registry holds another copy, so a consumer takes the copy of the registry instead. + return f"Delete {release_dir(local_registry.root, error.release)} to take the copy of {error.source}." + if facts.request is Operation.BUILD and facts.dest is not None: + # Only timenet-build build has a destination for a build: --publish-to. + return ( + "Build it again with `--force` to compare a fresh build with the published release, or give a changed " + "release a new version." + ) + return "A published version cannot change, so a changed release needs a new version." + + +def _bad_manifest(error: TimeNetManifestError, facts: Facts) -> str | None: + """Return a hint that names the delete that lets a download or a build replace a release, or None.""" + local_registry, path = facts.local_registry, error.path + release = None if local_registry is None or path is None else local_registry.release_at(path) + if path is None or release is None: + return None + directory = path.parent + # A download or a build reads the manifest first, so it fails until the directory is gone. + again = f"Delete {directory} and download or build {release} again" + return f"{again}, then clear again." if facts.request is Operation.CLEAR else f"{again}." + + +def _bad_origin(release: DatasetRef, facts: Facts, wording: HintWording) -> str: + """Return a hint that names the forced download and the forced build that replace a release and its record.""" + download = wording.download(release, facts, force=True) + build = wording.build(str(release), facts, force=True) + return ( + f"Get the release again with {download} if it came from a registry, or with {build} if it was built. " + "Both replace the release and its record." + ) + + +def _unreachable(facts: Facts, wording: HintWording) -> str | None: + """Return a hint that names the local copy or a registry instead of one that the call cannot reach.""" + if facts.dest is not None: + return None + leave = wording.leave_unreachable() + held = None + wanted = facts.wanted + local_registry = facts.local_registry + if facts.request in {Operation.READ, Operation.DOWNLOAD} and local_registry is not None and wanted is not None: + # Only "latest" asks every source. The local registry answers an exact version first. + answer = local_registry.resolve(wanted.dataset_id, None) if wanted.version == LATEST_VERSION else None + held = DatasetRef(dataset_id=wanted.dataset_id, version=answer.version) if isinstance(answer, Found) else None + if held is None: + return f"{leave[0].upper()}{leave[1:]}." + return f"Ask for {held} to read the copy in the local registry, or {leave}." + + +def _builds(dataset_id: str, version: str | None) -> bool: + """Return whether an installed connector builds a release.""" + return isinstance(resolve_release([InstalledConnectors()], dataset_id, version), Found) + + +def _lists_connectors(facts: Facts) -> bool: + """Return whether the sources of a call list the installed connectors.""" + return any(isinstance(source, InstalledConnectors) for source in facts.sources) + + +def _connectors(facts: Facts) -> str: + """Return what an SDK call adds to its sources to build a release.""" + if facts.local_registry is not None: + return "InstalledConnectors()" + return "LocalRegistry() and InstalledConnectors()" + + +def _registries(local_registry: LocalRegistry, facts: Facts) -> list[BaseRegistry]: + """Return the registries of the call other than the local registry, which the suggested download reads.""" + return [ + source + for source in facts.sources + if isinstance(source, BaseRegistry) and not is_same_registry(source, local_registry) + ] + + +_REMEDIES: dict[type[TimeNetError], Remedy] = { + TimeNetVersionUnavailableError: _version, + TimeNetNotBuiltError: lambda error, facts, wording: _not_built(error.release, facts, wording), + TimeNetMissingParentError: lambda error, facts, wording: _missing( + error.missing.dataset_id, str(error.missing.version), facts, wording + ), + TimeNetDatasetNotFoundError: lambda _, facts, wording: _not_found(facts, wording), + TimeNetAccessError: lambda error, facts, wording: _access(error.dataset, facts, wording), + TimeNetFileError: lambda error, facts, wording: _damaged(error.release, facts, wording), + TimeNetVersionConflictError: lambda error, facts, _: _conflict(error, facts), + TimeNetManifestError: lambda error, facts, _: _bad_manifest(error, facts), + TimeNetOriginError: lambda error, facts, wording: _bad_origin(error.release, facts, wording), + TimeNetIsolationError: lambda _, __, wording: ( + f"Build in this interpreter with {wording.run_in_this_interpreter()}." + ), + TimeNetUnreachableError: lambda _, facts, wording: _unreachable(facts, wording), + TimeNetNoCatalogError: lambda _, __, wording: f"Read an exact release with {wording.read_exact_release()}.", +} +"""The remedy of each error class. A subclass without its own entry gets the remedy of its nearest base class.""" diff --git a/packages/timenet/src/timenet/builders.py b/packages/timenet/src/timenet/builders.py index ae9984b1a..a48d632ba 100644 --- a/packages/timenet/src/timenet/builders.py +++ b/packages/timenet/src/timenet/builders.py @@ -157,6 +157,11 @@ def claims() -> Iterator[ConnectorClaim]: yield ConnectorClaim(dataset_id, backend, entry) +def has_backends() -> bool: + """Return whether any installed package registers a builder backend.""" + return any(True for _ in entry_points(group=ENTRY_POINT_GROUP)) + + def installed_builder(dataset_id: str) -> InstalledBuilder | None: """Find the first registered backend that lists a dataset, and read its card. diff --git a/packages/timenet/src/timenet/cli/app.py b/packages/timenet/src/timenet/cli/app.py index 3d9af8c59..202d113b8 100644 --- a/packages/timenet/src/timenet/cli/app.py +++ b/packages/timenet/src/timenet/cli/app.py @@ -4,7 +4,6 @@ from contextlib import contextmanager from functools import partial from pathlib import Path -import shlex from typing import TypeVar import warnings @@ -12,6 +11,7 @@ from rich.table import Table import typer +from timenet._hints import CliWording, Facts, Operation, explain, use_wording from timenet.cache import ( cache_size, clear_cache, @@ -20,15 +20,16 @@ releases_to_clear, stored_releases_by_kind, ) +from timenet.cli.hints import local_registry_from_flag, registry_flag, sources_from_flags from timenet.cli.options import AUTO_BUILD, LOCAL_REGISTRY, REMOTE_REGISTRY from timenet.cli.runner import run_cli from timenet.cli.ui import console as ui, progress_renderer -from timenet.client import TimeNet, default_local_registry, default_sources +from timenet.client import TimeNet from timenet.config import settings -from timenet.errors import CatalogWarning, ConnectorCardWarning, TimeNetDatasetNotFoundError, TimeNetNotBuiltError -from timenet.producer import publish as publish_release +from timenet.errors import CatalogWarning, ConnectorCardWarning, TimeNetNotBuiltError +from timenet.producer import _publish_from from timenet.progress import ProgressCallback, ProgressEvent, StepStarted -from timenet.registry import open_registry +from timenet.registry import LocalRegistry from timenet.registry.origin import OriginKind from timenet.types import DatasetMetadata, Domain, License, TaskType from timenet.types.access import access_requirements, effective_access @@ -37,7 +38,7 @@ from timenet.types.tasks import TASKS -app = typer.Typer(help="Browse a TimeNet registry and fetch datasets.", no_args_is_help=True) +app = typer.Typer(help="Find, read, download and publish TimeNet datasets.", no_args_is_help=True) console = Console() T = TypeVar("T") @@ -45,7 +46,7 @@ @app.callback() def _root(quiet: bool = typer.Option(False, "--quiet", "-q", help="Suppress status output.")) -> None: - """Browse a TimeNet registry and fetch datasets.""" + """Find, read, download and publish TimeNet datasets.""" ui.quiet = quiet @@ -72,9 +73,9 @@ def _enum_list(values: list[str], factory: Callable[[str], T], flag: str, choice def _client(local_registry: str | None, remote_registry: str | None, auto_build: bool | None) -> TimeNet: - """Create a client with the selected registries and build setting. + """Create a client from CLI options. - Pass ``None`` to use the matching ``TIMENET_*`` environment variable or its default. + A ``None`` flag uses the matching ``TIMENET_*`` environment variable or its default. Args: local_registry: The local registry path. @@ -82,10 +83,9 @@ def _client(local_registry: str | None, remote_registry: str | None, auto_build: auto_build: Whether to build missing datasets with installed connectors. Returns: - The configured client. + The client. """ - sources = default_sources(local_registry=local_registry, remote_registry=remote_registry, auto_build=auto_build) - return TimeNet(sources=sources) + return TimeNet(sources=sources_from_flags(local_registry_from_flag(local_registry), remote_registry, auto_build)) def _metadata_table(rows: list[DatasetMetadata], title: str) -> Table: @@ -138,7 +138,7 @@ def list_datasets( auto_build: bool | None = AUTO_BUILD, ) -> None: """List each dataset at its highest version across all sources.""" - with _warning_lines(): + with use_wording(CliWording(local_registry, remote_registry)), _warning_lines(): rows = _client(local_registry, remote_registry, auto_build).list() if not rows: typer.echo("No datasets in the sources.") @@ -164,7 +164,7 @@ def search( # ruff: ignore[too-many-arguments, too-many-positional-arguments] Repeat a filter flag to pass several values. """ - with _warning_lines(): + with use_wording(CliWording(local_registry, remote_registry)), _warning_lines(): results = _client(local_registry, remote_registry, auto_build).search( query=query or None, domain=_enum_list(domain, Domain, "--domain", [d.value for d in Domain]), @@ -193,18 +193,16 @@ def info( For an unbuilt release, show its metadata card and build command instead. """ client = _client(local_registry, remote_registry, auto_build) - try: - manifest = client.get_manifest(dataset_id) - except TimeNetNotBuiltError as error: - console.print(_card_table(error.card, lineage=None)) - build = f"timenet-build build {split_ref(dataset_id)[0]}" - if local_registry is not None: - build += f" --local-registry {shlex.quote(local_registry)}" - if remote_registry is not None: - build += f" --remote-registry {shlex.quote(remote_registry)}" - # A plain line, so that a long path does not wrap. - typer.echo(f"The schema and the counts appear after a build: `{build}`.") - return + wording = CliWording(local_registry, remote_registry) + with use_wording(wording): + try: + manifest = client.get_manifest(dataset_id) + except TimeNetNotBuiltError as error: + console.print(_card_table(error.card, lineage=None)) + # A plain line, so that a long path does not wrap. + build = wording.build_command(split_ref(dataset_id)[0]) + typer.echo(f"The schema and the counts appear after a build: `{build}`.") + return counts = manifest.counts table = _card_table(manifest.metadata, manifest.lineage) table.add_row("specs", ", ".join(spec.spec_type for spec in manifest.dataset_schema.time_series_specs) or "-") @@ -277,13 +275,13 @@ def download( The command prints the release directory to stdout. """ started: list[StepStarted] = [] - with progress_renderer() as show: - client = _client(local_registry, remote_registry, auto_build) + local = local_registry_from_flag(local_registry) + client = TimeNet(sources=sources_from_flags(local, remote_registry, auto_build)) + with use_wording(CliWording(local_registry, remote_registry)), progress_renderer() as show: target = client.download(dataset_id, force=force, progress=partial(_keep_steps, started, show)) if not started: # The local registry already held the release and its payload sources. - store = default_local_registry(local_registry=local_registry) - ui.success(f"Nothing to do: '{split_ref(dataset_id)[0]}' {target.name} is already in {store}") + ui.success(f"Nothing to do: '{split_ref(dataset_id)[0]}' {target.name} is already in {local}") typer.echo(str(target)) @@ -315,32 +313,16 @@ def publish( ) -> None: """Copy a release and its payload sources to another registry. - Read from ``--source`` or the local registry, and print the release to stdout. + 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 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") - try: - with progress_renderer() as progress: - release = publish_release(dataset_id, dest=dest, source=origin, progress=progress) - except TimeNetDatasetNotFoundError as error: - # The hint fits only a release that the local registry lacks. A subclass, such as a missing payload - # source, names its own registries. - if source is not None or type(error) is not TimeNetDatasetNotFoundError: - raise - 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. - 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}{flag}`, " - f"or copy it there with `timenet download {wanted}{flag}`." - ) - raise TimeNetDatasetNotFoundError(f"{str(error).rstrip('.')}. {hint}") from error + """ + local = local_registry_from_flag(local_registry) + target = registry_flag(dest, "--dest") + origin = local if source is None else registry_flag(source, "--source") + with use_wording(CliWording(local_registry)), progress_renderer() as progress: + # Only the local registry gets the hints that copy or build a release into it. + hinted = local if source is None else None + release = _publish_from(dataset_id, dest=target, origin=origin, local_registry=hinted, progress=progress) ui.success(f"Published {release} to {dest}") typer.echo(str(release)) @@ -368,28 +350,30 @@ def publish( @local_app.command("info") def local_info(local_registry: str | None = LOCAL_REGISTRY) -> None: """Show paths and sizes for the cache, downloads, and builds.""" - store, cache_dir = default_local_registry(local_registry=local_registry), settings().cache_dir - by_kind = stored_releases_by_kind(store) + local, cache_dir = local_registry_from_flag(local_registry), settings().cache_dir + with _local_hints(local, local_registry, Operation.READ): + by_kind = stored_releases_by_kind(local) # Plain lines, so that a long path does not wrap. typer.echo(_cache_line(cache_dir)) for kind, of_kind in by_kind.items(): title = _KIND_WORDS[kind][1] size = human_bytes(sum(item.size() for item in of_kind)) - typer.echo(f"{title} in {store.root}: {_count(len(of_kind), 'release')}, {size}") + typer.echo(f"{title} in {local.root}: {_count(len(of_kind), 'release')}, {size}") @local_app.command("clear") def local_clear(yes: bool = _YES, local_registry: str | None = LOCAL_REGISTRY) -> None: """Delete the cache, every download and every build.""" - store, cache_dir = default_local_registry(local_registry=local_registry), settings().cache_dir - if not yes: - builds = _count(len(stored_releases_by_kind(store)[OriginKind.BUILD]), "build") - typer.confirm( - f"Delete the cache in {cache_dir} and every release in {store.root}, {builds} included?", abort=True - ) - # The cache clear checks the folders before it deletes anything, so it runs first. - cache = clear_cache(cache_dir, local_registry=store) - releases = clear_releases(store, kinds=tuple(OriginKind)) + local, cache_dir = local_registry_from_flag(local_registry), settings().cache_dir + with _local_hints(local, local_registry, Operation.CLEAR): + if not yes: + builds = _count(len(stored_releases_by_kind(local)[OriginKind.BUILD]), "build") + typer.confirm( + f"Delete the cache in {cache_dir} and every release in {local.root}, {builds} included?", abort=True + ) + # The cache clear checks the folders before it deletes anything, so it runs first. + cache = clear_cache(cache_dir, local_registry=local) + releases = clear_releases(local, kinds=tuple(OriginKind)) _warn_unmarked(cache.kept) _freed(releases.freed + cache.freed, len(releases.removed) + len(cache.removed)) @@ -402,11 +386,11 @@ def cache_info() -> None: @cache_app.command("clear") def cache_clear(yes: bool = _YES) -> None: - """Delete the raw source files and the control databases in the cache.""" + """Clear cached source files and control databases.""" cache_dir = settings().cache_dir if not yes: typer.confirm(f"Delete the raw source files and control databases in {cache_dir}?", abort=True) - cleared = clear_cache(cache_dir, local_registry=default_local_registry()) + cleared = clear_cache(cache_dir, local_registry=local_registry_from_flag(None)) _warn_unmarked(cleared.kept) _freed(cleared.freed, len(cleared.removed)) @@ -437,13 +421,14 @@ def builds_clear(yes: bool = _YES, local_registry: str | None = LOCAL_REGISTRY) def _show_releases(kind: OriginKind, local_registry: str | None) -> None: """Show releases of one kind in a table, or a message if none exist.""" - store = default_local_registry(local_registry=local_registry) - releases = stored_releases_by_kind(store)[kind] + local = local_registry_from_flag(local_registry) + with _local_hints(local, local_registry, Operation.READ): + releases = stored_releases_by_kind(local)[kind] noun, title = _KIND_WORDS[kind] if not releases: - typer.echo(f"No {noun}s in {store.root}.") + typer.echo(f"No {noun}s in {local.root}.") return - table = Table(title=f"{title} in {store.root}", title_justify="left", title_style="bold") + table = Table(title=f"{title} in {local.root}", title_justify="left", title_style="bold") table.add_column("Dataset", style="cyan") table.add_column("Version") table.add_column("Source", overflow="fold") @@ -460,16 +445,33 @@ def _show_releases(kind: OriginKind, local_registry: str | None) -> None: def _clear_releases(kind: OriginKind, yes: bool, local_registry: str | None) -> None: """Delete the releases of one kind after a prompt that counts them, and name each release that stays.""" - store = default_local_registry(local_registry=local_registry) - if not yes: - releases = releases_to_clear(store, kinds=[kind]).releases - typer.confirm(f"Delete {_count(len(releases), _KIND_WORDS[kind][0])} from {store.root}?", abort=True) - cleared = clear_releases(store, kinds=[kind]) + local = local_registry_from_flag(local_registry) + with _local_hints(local, local_registry, Operation.CLEAR): + if not yes: + releases = releases_to_clear(local, kinds=[kind]).releases + typer.confirm(f"Delete {_count(len(releases), _KIND_WORDS[kind][0])} from {local.root}?", abort=True) + cleared = clear_releases(local, kinds=[kind]) for item in cleared.kept: ui.warn(f"Kept {item.release}: {item.needed_by} needs it.") _freed(cleared.freed, len(cleared.removed)) +@contextmanager +def _local_hints(local: LocalRegistry, local_registry: str | None, request: Operation) -> Generator[None]: + """Give a TimeNet error of the block the CLI hint of a command that works on the local registry alone. + + Args: + local: The local registry. + local_registry: The value of ``--local-registry``, or ``None``. + request: What the command does. + + Yields: + Nothing. + """ + with use_wording(CliWording(local_registry)), explain(Facts([local], local, request=request)): + yield + + def _cache_line(cache_dir: Path) -> str: """Return the path and the size of the cache.""" return f"Cache {cache_dir}: {human_bytes(cache_size(cache_dir))}" diff --git a/packages/timenet/src/timenet/cli/hints.py b/packages/timenet/src/timenet/cli/hints.py new file mode 100644 index 000000000..fd98c378f --- /dev/null +++ b/packages/timenet/src/timenet/cli/hints.py @@ -0,0 +1,73 @@ +"""Validate the registry options of the CLI. + +A flag that names no registry is a usage error: it exits with code 2 and names the flag. A bad ``TIMENET_*`` +variable stays a TimeNet error, which exits with code 1 and names the variable. +""" + +import typer + +from timenet._registry_uri import local_registry_path +from timenet.client import _sources_after_local_registry, default_local_registry +from timenet.config import settings +from timenet.errors import TimeNetRegistryError +from timenet.registry import LocalRegistry, WritableRegistry, open_registry +from timenet.sources import ReleaseSource + + +def registry_flag(value: str, flag: str) -> WritableRegistry: + """Open the registry that a flag names. + + Args: + value: The value of the flag. + flag: The flag, such as ``--dest``. + + Returns: + The registry. + + Raises: + BadParameter: If the value names no registry. + """ + try: + return open_registry(value, label=flag) + except TimeNetRegistryError as error: + raise typer.BadParameter(str(error)) from error + + +def local_registry_from_flag(local_registry: str | None) -> LocalRegistry: + """Open the local registry that ``--local-registry`` names, else the default one. + + Args: + local_registry: The value of ``--local-registry``, or ``None``. + + Returns: + The local registry. + + Raises: + BadParameter: If the value names no local directory. + """ + if local_registry is None: + return default_local_registry() + try: + path = local_registry_path(local_registry, label="--local-registry") + except TimeNetRegistryError as error: + raise typer.BadParameter(str(error)) from error + return default_local_registry(local_registry=path) + + +def sources_from_flags( + local_registry: LocalRegistry, remote_registry: str | None, auto_build: bool | None +) -> list[ReleaseSource]: + """Select release sources from CLI options and configured defaults. + + Args: + local_registry: The local registry of the command, from :func:`local_registry_from_flag`. + remote_registry: The value of ``--remote-registry``, or ``None``. + auto_build: The value of ``--auto-build``, or ``None``. + + Returns: + The local registry, the remote registry and the optional installed connectors, in lookup order. + """ + remote = None if remote_registry is None else registry_flag(remote_registry, "--remote-registry") + # Both registries are arguments too, so a bad variable that a flag replaces raises nothing. + cfg = settings(local_registry=local_registry.root, remote_registry=remote_registry, auto_build=auto_build) + return [local_registry, *_sources_after_local_registry(cfg, remote)] diff --git a/packages/timenet/src/timenet/cli/options.py b/packages/timenet/src/timenet/cli/options.py index b348d08af..557c4dbc5 100644 --- a/packages/timenet/src/timenet/cli/options.py +++ b/packages/timenet/src/timenet/cli/options.py @@ -11,8 +11,8 @@ None, "--local-registry", metavar="PATH", - help="A local registry: the first source that a lookup checks, and the place where downloads and builds land " - "(default: $TIMENET_LOCAL_REGISTRY, else ~/.timenet/registry).", + help="The local registry, where downloads and builds land and which a lookup checks first (default: " + "$TIMENET_LOCAL_REGISTRY, else ~/.timenet/registry).", ) REMOTE_REGISTRY = typer.Option( None, diff --git a/packages/timenet/src/timenet/client.py b/packages/timenet/src/timenet/client.py index 56c94244c..89defd20e 100644 --- a/packages/timenet/src/timenet/client.py +++ b/packages/timenet/src/timenet/client.py @@ -7,48 +7,27 @@ # ``list[str]`` resolved to the builtin type, not to the method. from __future__ import annotations -from collections.abc import Generator, Sequence -from contextlib import contextmanager +from collections.abc import Sequence +from contextlib import AbstractContextManager import os from pathlib import Path -import shlex from typing import TYPE_CHECKING, TypeAlias, TypeVar +from timenet._hints import Facts, Operation, RequestedRelease, explain from timenet._warnings import warn_at_caller from timenet.builders import BuildOptions -from timenet.config import settings +from timenet.config import TimeNetSettings, settings from timenet.coordinator.build import run_plan from timenet.coordinator.plan import Lookups, plan_download from timenet.coordinator.read import raise_not_found, read_release from timenet.dataset import TimeFDataset -from timenet.errors import ( - CatalogWarning, - TimeFValidationError, - TimeNetAccessError, - TimeNetDatasetNotFoundError, - TimeNetError, - TimeNetFileError, - TimeNetMissingParentError, - TimeNetNoCatalogError, - TimeNetNotBuiltError, - TimeNetVersionUnavailableError, -) +from timenet.errors import CatalogWarning, TimeFValidationError, TimeNetNoCatalogError, TimeNetNotBuiltError from timenet.manifest import Manifest from timenet.progress import ProgressCallback from timenet.reader import LoadPlan, TimeFReader from timenet.registry import BaseRegistry, LocalRegistry, open_registry from timenet.registry._paths import release_dir -from timenet.registry.local import is_same_registry -from timenet.sources import ( - Found, - FoundInConnector, - FoundInRegistry, - InstalledConnectors, - NotFound, - NotServed, - ReleaseSource, - resolve_release, -) +from timenet.sources import FoundInConnector, InstalledConnectors, NotFound, NotServed, ReleaseSource from timenet.types import DatasetFilter, DatasetMetadata, Domain, License, ListedRelease, Task from timenet.types.references import DatasetRef, split_ref @@ -105,9 +84,25 @@ def default_sources( TimeNetRegistryError: If ``local_registry`` names a remote registry, or the remote registry cannot be opened. """ # ruff: ignore[docstring-extraneous-exception] (raised by settings and open_registry) cfg = settings(local_registry=local_registry, remote_registry=remote_registry, auto_build=auto_build) - local = LocalRegistry(cfg.local_registry_dir) - remote = open_registry(cfg.remote_registry, label="TIMENET_REMOTE_REGISTRY") - return [local, remote, *([InstalledConnectors()] if cfg.auto_build else [])] + return [LocalRegistry(cfg.local_registry_dir), *_sources_after_local_registry(cfg)] + + +def _sources_after_local_registry(cfg: TimeNetSettings, remote: BaseRegistry | None = None) -> list[ReleaseSource]: + """Return the remote registry and the optional installed connectors of the settings, in lookup order. + + Args: + cfg: The settings. + remote: The remote registry, already open, or ``None`` for the remote registry of the settings. + + Returns: + The sources that follow the local registry. + + Raises: + TimeNetRegistryError: If the remote registry cannot be opened. + """ # ruff: ignore[docstring-extraneous-exception] (raised by open_registry) + if remote is None: + remote = open_registry(cfg.remote_registry, label="TIMENET_REMOTE_REGISTRY") + return [remote, *([InstalledConnectors()] if cfg.auto_build else [])] def _resolve_ref(dataset_id: str, version: str | None) -> tuple[str, str | None]: @@ -214,7 +209,8 @@ def list(self) -> _Metadatas: TimeNetNoCatalogError: If no source has a catalog. TimeNetInvalidCardError: If an installed backend cannot list its dataset IDs. """ # ruff: ignore[docstring-extraneous-exception] (raised by the backend) - return [release.card for release in self._listed()] + with self._explain(Operation.READ): + return [release.card for release in self._listed()] def _listed(self) -> _Listed: """Return each dataset at its highest version across the sources, sorted by dataset ID. @@ -264,10 +260,10 @@ def get_manifest(self, dataset_id: str, version: str | None = None) -> Manifest: TimeFValidationError: If ``dataset_id`` and ``version`` both give a version. """ # ruff: ignore[docstring-extraneous-exception] (raised by the resolution of the release) dataset_id, version = _resolve_ref(dataset_id, version) - with self._hints(dataset_id): + with self._explain(Operation.READ, dataset_id, version): found = self._manifest_or_connector(dataset_id, version) - if isinstance(found, FoundInConnector): - raise TimeNetNotBuiltError(found.builder.release, found.builder.card) + if isinstance(found, FoundInConnector): + raise TimeNetNotBuiltError(found.builder.release, found.builder.card) return found def get_card(self, dataset_id: str, version: str | None = None) -> DatasetMetadata: @@ -290,9 +286,9 @@ def get_card(self, dataset_id: str, version: str | None = None) -> DatasetMetada TimeFValidationError: If ``dataset_id`` and ``version`` both give a version. """ # ruff: ignore[docstring-extraneous-exception] (raised by the resolution of the release) dataset_id, version = _resolve_ref(dataset_id, version) - with self._hints(dataset_id): + with self._explain(Operation.READ, dataset_id, version): found = self._manifest_or_connector(dataset_id, version) - return found.builder.card if isinstance(found, FoundInConnector) else found.metadata + return found.builder.card if isinstance(found, FoundInConnector) else found.metadata def search( # ruff: ignore[too-many-arguments] self, @@ -342,7 +338,8 @@ def search( # ruff: ignore[too-many-arguments] tag=tag, limit=limit, ) - return wanted.select(self._listed()) + with self._explain(Operation.READ): + return wanted.select(self._listed()) def download( self, @@ -385,30 +382,18 @@ def download( "download() needs a writable local registry to copy into. " "Add a LocalRegistry without read_only=True to the sources" ) - lookups = Lookups() - answer = lookups.resolve(self._sources, dataset_id, version) - if isinstance(answer, NotServed): - # A registry lists the release, but no source serves its files. - error = answer.access_error(dataset_id) - error.hint = _access_hint(dataset_id, self._sources) - raise error - if isinstance(answer, NotFound): - # No source provides the release. - try: + with self._explain(Operation.DOWNLOAD, dataset_id, version): + lookups = Lookups() + answer = lookups.resolve(self._sources, dataset_id, version) + if isinstance(answer, NotServed): + # A registry lists the release, but no source serves its files. + raise answer.access_error(dataset_id) + if isinstance(answer, NotFound): + # No source provides the release. raise_not_found(dataset_id, version, self._sources) - except TimeNetVersionUnavailableError: - # An installed connector builds another version, and the error names it. - raise - except TimeNetDatasetNotFoundError as error: - raise TimeNetDatasetNotFoundError(f"{error} Add a registry that holds it to the sources.") from error - release = DatasetRef(dataset_id=dataset_id, version=answer.version) - try: + release = DatasetRef(dataset_id=dataset_id, version=answer.version) plan = plan_download(release, local_registry, sources=self._sources, force=force, lookups=lookups) - except TimeNetFileError as error: - # The local registry holds a damaged copy that no other registry provides. - error.hint = self._file_hint(error) - raise - run_plan(plan, local_registry, options=BuildOptions(progress=progress)) + run_plan(plan, local_registry, options=BuildOptions(progress=progress)) return release_dir(local_registry.root, release) def open(self, dataset_id: str, version: str | None = None) -> TimeFReader: @@ -463,7 +448,7 @@ def open(self, dataset_id: str, version: str | None = None) -> TimeFReader: TimeNetBuildError: If a build runs but fails. """ # ruff: ignore[docstring-extraneous-exception] (raised by read_release and the builder) dataset_id, version = _resolve_ref(dataset_id, version) - with self._hints(dataset_id): + with self._explain(Operation.READ, dataset_id, version): return TimeFReader( read_release(dataset_id, version, sources=self._sources, target_registry=self._local_registry) ) @@ -498,39 +483,12 @@ def load(self, dataset_id: str, version: str | None = None) -> TimeFDataset: with self.open(dataset_id, version) as reader: return reader.read(load=LoadPlan.ALL_EAGER) - @contextmanager - def _hints(self, dataset_id: str) -> Generator[None]: - """Add recovery hints to errors raised inside the block. - - Args: - dataset_id: The dataset ID without a version suffix. - - Yields: - Nothing. - - Raises: - TimeNetAccessError: If access fails. - TimeNetFileError: If a file is missing or damaged. - TimeNetMissingParentError: If a required release is missing. - TimeNetDatasetNotFoundError: If the dataset is missing. - """ - try: - yield - except TimeNetAccessError as error: - error.hint = _access_hint(dataset_id, self._sources) - raise - except TimeNetFileError as error: - error.hint = self._file_hint(error) - raise - except TimeNetMissingParentError as error: - error.hint = _not_provided_hint(self._sources) - raise - except TimeNetDatasetNotFoundError as error: - if type(error) is not TimeNetDatasetNotFoundError: - raise - raise TimeNetDatasetNotFoundError( - f"{str(error).rstrip('.')}. {_not_provided_hint(self._sources)}" - ) from error + def _explain( + self, request: Operation, dataset_id: str | None = None, version: str | None = None + ) -> AbstractContextManager[None]: + """Return a context that gives a TimeNet error raised in it a hint in the current wording.""" + wanted = None if dataset_id is None else RequestedRelease(dataset_id, version) + return explain(Facts(self._sources, self._local_registry, request=request, wanted=wanted)) def _manifest_or_connector(self, dataset_id: str, version: str | None) -> Manifest | FoundInConnector: """Resolve a release to its manifest, or to the answer of the connector that provides it. @@ -562,30 +520,6 @@ def _manifest_or_connector(self, dataset_id: str, version: str | None) -> Manife raise_not_found(dataset_id, version, self._sources) return manifest - def _file_hint(self, error: TimeNetFileError) -> str | None: - """Suggest how to replace a damaged release in the local registry. - - Args: - error: The error that names the damaged release. - - Returns: - A repair hint, or ``None`` if the local registry has no damaged copy or the lookup raises a TimeNet error. - """ - local_registry = self._local_registry - if local_registry is None: - return None - damaged = error.release - try: - held = local_registry.find_manifest(damaged) - if held is None or local_registry.damage(damaged, held) is None: - # The local registry holds no damaged copy, so the error is about another registry. - return None - found = _copy_source(self._sources, local_registry, damaged) - except TimeNetError: - # The hint must not hide the error that it explains, for example if a registry is offline. - return None - return _repair_hint(damaged, found, local_registry) - def open_torch( self, dataset_id: str, version: str | None = None, *, load: LoadPlan = LoadPlan.ALL_EAGER ) -> TimeFTorchDataset: @@ -609,76 +543,3 @@ def open_torch( from timenet.torch import TimeFTorchDataset # ruff: ignore[import-outside-top-level] return TimeFTorchDataset.from_reader(self.open(dataset_id, version), load=load) - - -def _not_provided_hint(sources: Sequence[ReleaseSource]) -> str: - """Suggest sources for a missing dataset release. - - Args: - sources: The configured sources. - - Returns: - Instructions to add a registry or installed connectors. - """ - if any(isinstance(source, InstalledConnectors) for source in sources): - return "Add a registry that holds it to the sources." - return "Add a registry that holds it to the sources, or add InstalledConnectors() to build it." - - -def _access_hint(dataset_id: str, sources: Sequence[ReleaseSource]) -> str: - """Suggest how to obtain a release whose files are unavailable. - - Args: - dataset_id: The dataset ID without a version suffix. - sources: The configured sources. - - Returns: - A hint with options to add a source or build the release locally. - """ - build = f"`timenet-build build {dataset_id}`" - if any(isinstance(source, InstalledConnectors) for source in sources): - return ( - "No source serves its files, and no installed connector builds this release. " - f"Install its connector package and build it locally with {build}." - ) - return f"No source serves its files. Add InstalledConnectors() to the sources, or build it locally with {build}." - - -def _copy_source(sources: Sequence[ReleaseSource], local_registry: LocalRegistry, release: DatasetRef) -> Found | None: - """Find the answer of a release source outside the local registry. - - Args: - sources: The sources to search, in order. - local_registry: The local registry to exclude. - release: The dataset ID and exact version. - - Returns: - The answer of the source that provides or builds the release, or ``None`` if none exists. - """ - others = [source for source in sources if not is_same_registry(source, local_registry)] - answer = resolve_release(others, release.dataset_id, str(release.version)) - return answer if isinstance(answer, Found) else None - - -def _repair_hint(release: DatasetRef, found: Found | None, local_registry: LocalRegistry) -> str: - """Suggest how to replace a damaged local release. - - Args: - release: The damaged release. - found: The answer of a source for the release, or ``None``. - local_registry: The local registry that holds the damaged release. - - Returns: - A download or rebuild hint, or an explanation that no replacement is available. - """ - if isinstance(found, FoundInRegistry) and not is_same_registry(found.registry, local_registry): - # 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} --local-registry {shlex.quote(str(local_registry.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/src/timenet/config.py b/packages/timenet/src/timenet/config.py index 077278de0..de0be333d 100644 --- a/packages/timenet/src/timenet/config.py +++ b/packages/timenet/src/timenet/config.py @@ -32,9 +32,6 @@ ) """The cache marker text, starting with the standard cache tag signature.""" -_FLAGS = {"local_registry": "--local-registry", "remote_registry": "--remote-registry"} -"""CLI flag names for the registry settings.""" - class TimeNetSettings(BaseSettings): """TimeNet settings from arguments, ``TIMENET_*`` environment variables, and an optional ``.env`` file.""" @@ -77,9 +74,8 @@ def _not_empty(cls, value: Any, info: ValidationInfo) -> Any: value = os.fspath(value) if isinstance(value, str) and not value.strip(): name = str(info.field_name) - flag = f" (also as {_FLAGS[name]})" if name in _FLAGS else "" raise TimeFValidationError( - f"TIMENET_{name.upper()} must not be empty{flag}; give it a value, " + f"TIMENET_{name.upper()} must not be empty; give it a value, " f"or leave it unset for {cls.model_fields[name].default}." ) return value @@ -91,7 +87,7 @@ def local_registry_dir(self) -> Path: Raises: TimeNetRegistryError: If the local registry path is blank, remote or uses an invalid URI. """ # ruff: ignore[docstring-extraneous-exception] (raised by local_registry_path) - return local_registry_path(self.local_registry) + return local_registry_path(self.local_registry, label="TIMENET_LOCAL_REGISTRY") @property def cache_dir(self) -> Path: diff --git a/packages/timenet/src/timenet/coordinator/plan.py b/packages/timenet/src/timenet/coordinator/plan.py index f6918d353..83975a146 100644 --- a/packages/timenet/src/timenet/coordinator/plan.py +++ b/packages/timenet/src/timenet/coordinator/plan.py @@ -353,8 +353,7 @@ def local(self, reference: DatasetRef, needed_by: DatasetRef | None, role: Role, elif held is not None: # With force, the copy needs a registry other than the target registry, and none provides the release. raise TimeNetDatasetNotFoundError( - f"No registry other than the local registry provides {reference}, so download(force=True) cannot copy " - "it again." + f"no registry other than the local registry provides {reference} to copy again" ) elif isinstance(answer, FoundInConnector): if not self.builds: diff --git a/packages/timenet/src/timenet/errors.py b/packages/timenet/src/timenet/errors.py index 54457fef1..e47577f92 100644 --- a/packages/timenet/src/timenet/errors.py +++ b/packages/timenet/src/timenet/errors.py @@ -7,16 +7,19 @@ Every TimeNet-raised warning derives from :class:`TimeNetWarning`. Some errors carry facts as attributes and leave the remedy to the front end that called the lower -layer: the CLI names its flags, the SDK names its keywords. A front end sets ``hint`` to a full -sentence and raises the error again. The message ends with a period, and the hint follows as its own +layer: the CLI names its flags, the SDK names its keywords. A front end sets ``hint`` to one or two +full sentences and raises the error again. ``str()`` adds the hint after the message as its own sentence. """ +from enum import Enum, auto from functools import partial -from typing import TYPE_CHECKING, Literal +from typing import TYPE_CHECKING if TYPE_CHECKING: + from pathlib import Path + from timenet.types.access import Access, AccessRequirement from timenet.types.metadata import DatasetMetadata from timenet.types.references import DatasetRef @@ -24,13 +27,35 @@ class TimeNetError(Exception): - """Base class for all TimeNet errors.""" + """Base class for all TimeNet errors. + + String conversion appends ``hint`` to the error message. + + Attributes: + hint: A suggested way to resolve the error, or ``None``. + """ + + hint: str | None = None + + def __str__(self) -> str: + message = self._message() + if self.hint is None: + return message + return f"{message} {self.hint}" if message.endswith((".", "?", "!")) else f"{message}. {self.hint}" + + def _message(self) -> str: + """Return the message without the hint.""" + return super().__str__() class TimeNetRegistryError(TimeNetError): """TimeNet could not load or reach a registry, or the registry could not serve a request.""" +class TimeNetUnreachableError(TimeNetRegistryError): + """A request to a registry or its storage failed on the network.""" + + class TimeNetNoCatalogError(TimeNetRegistryError): """No source of a dataset listing has a catalog.""" @@ -54,7 +79,7 @@ def __init__(self, release: "DatasetRef", card: "DatasetMetadata") -> None: self.release = release self.card = card - def __str__(self) -> str: + def _message(self) -> str: return f"{self.release} is not built yet: only an installed connector provides it." @@ -66,7 +91,6 @@ class TimeNetMissingParentError(TimeNetDatasetNotFoundError): needed_by: The release that needs it. payload_source: ``True`` if ``needed_by`` reads files of ``missing``, ``False`` for a declared build parent. searched: The search location. - hint: Advice on obtaining the release, or ``None``. """ def __init__(self, missing: "DatasetRef", needed_by: "DatasetRef", searched: str, *, payload_source: bool) -> None: @@ -75,17 +99,24 @@ def __init__(self, missing: "DatasetRef", needed_by: "DatasetRef", searched: str self.needed_by = needed_by self.payload_source = payload_source self.searched = searched - self.hint: str | None = None def __reduce__(self) -> tuple[object, ...]: # Pickle passes the arguments by position, so the keyword-only flag travels in a partial. rebuild = partial(type(self), payload_source=self.payload_source) return rebuild, (self.missing, self.needed_by, self.searched), self.__dict__ - def __str__(self) -> str: + def _message(self) -> str: role = "payload source" if self.payload_source else "parent release" - message = f"{role} {self.missing} of {self.needed_by} is not in {self.searched}." - return message if self.hint is None else f"{message} {self.hint}" + return f"{role} {self.missing} of {self.needed_by} is not in {self.searched}." + + +class ReleaseDifference(Enum): + """What differs between two manifests of the same release.""" + + FILES = auto() + """The file records differ.""" + CARD = auto() + """The file records are the same, and the rest of the manifest differs.""" class TimeNetVersionConflictError(TimeNetError): @@ -96,36 +127,33 @@ class TimeNetVersionConflictError(TimeNetError): Attributes: release: The release that differs. registry: The destination registry, as text. + registry_location: The ``location`` of the destination registry. source: The source registry, as text. + difference: What differs. needed_by: The requested release that needs this payload source, or ``None`` if ``release`` is the request. - difference: ``"files"`` if the file records differ, else ``"card"`` for the rest of the manifest. - hint: Advice to append to the error message, or ``None``. """ - def __init__( + def __init__( # ruff: ignore[too-many-arguments, too-many-positional-arguments] - a pickled error passes them in order self, release: "DatasetRef", registry: str, + registry_location: str, source: str, + difference: ReleaseDifference, needed_by: "DatasetRef | None" = None, - difference: Literal["files", "card"] = "files", ) -> None: - super().__init__(release, registry, source, needed_by, difference) + super().__init__(release, registry, registry_location, source, difference, needed_by) self.release = release self.registry = registry + self.registry_location = registry_location self.source = source self.needed_by = needed_by self.difference = difference - self.hint: str | None = None - def __str__(self) -> str: + def _message(self) -> str: held = str(self.release) if self.needed_by is None else f"{self.release}, a payload source of {self.needed_by}," - other = "other files" if self.difference == "files" else "another card" - message = ( - f"{self.registry} already holds {held} with {other} than {self.source}. " - "A published version cannot change, so a changed release needs a new version." - ) - return message if self.hint is None else f"{message} {self.hint}" + other = "other files" if self.difference is ReleaseDifference.FILES else "another card" + return f"{self.registry} already holds {held} with {other} than {self.source}." class TimeNetAccessError(TimeNetError): @@ -138,7 +166,6 @@ class TimeNetAccessError(TimeNetError): requirements: Each card, of the release itself or of an ancestor, whose access is not open, with its access level and its ``access_url``. A user needs access at each of them. See :func:`~timenet.types.access.access_requirements`. - hint: What the caller can do instead, added by the front end, or ``None``. """ def __init__(self, dataset: "DatasetRef", access: "Access", requirements: "tuple[AccessRequirement, ...]") -> None: @@ -146,15 +173,14 @@ def __init__(self, dataset: "DatasetRef", access: "Access", requirements: "tuple self.dataset = dataset self.access = access self.requirements = requirements - self.hint: str | None = None - def __str__(self) -> str: + def _message(self) -> str: message = f"{self.dataset} is {self.access.value}." if self.requirements: pages = [f"{item.access_url} for {item.release}" for item in self.requirements] listed = pages[0] if len(pages) == 1 else f"{', at '.join(pages[:-1])} and at {pages[-1]}" message = f"{message} Get access at {listed}." - return message if self.hint is None else f"{message} {self.hint}" + return message class TimeNetConnectorError(TimeNetError): @@ -192,22 +218,18 @@ class TimeNetVersionUnavailableError(TimeNetDatasetNotFoundError, TimeFValidatio Attributes: release: The requested release. declared: The version that the connector builds. - hint: Suggested next steps to append to the error message, or ``None``. """ def __init__(self, release: "DatasetRef", declared: "Version") -> None: super().__init__(release, declared) self.release = release self.declared = declared - self.hint: str | None = None - def __str__(self) -> str: - message = ( + def _message(self) -> str: + return ( f"connector for {self.release.dataset_id!r} builds version {self.declared}, " - f"not the requested {self.release.version}. " - f"Ask for version {self.declared}, or leave out the version." + f"not the requested {self.release.version}." ) - return message if self.hint is None else f"{message} {self.hint}" class TimeNetInvalidCardError(TimeNetError, ValueError): @@ -218,6 +240,10 @@ class TimeNetBuildError(TimeNetError): """A build run failed: the connector build, or the environment it needed.""" +class TimeNetIsolationError(TimeNetBuildError): + """An isolated build cannot make its environment.""" + + class TimeNetWarning(UserWarning): """Base class for all TimeNet warnings.""" @@ -239,18 +265,16 @@ class TimeFFormatError(TimeNetError): class TimeNetFileError(TimeFFormatError): - """A file of a release is missing, or it does not match its manifest record. + """A release file is missing or differs from its manifest record. A local copy is checked by file size. A transfer is checked by size and checksum. Attributes: release: The release that the file belongs to. owner: The release, payload source, or transfer that the file belongs to, as text. - path: The file, relative to the release directory. For a local copy, this is the first - missing or resized file in manifest order. + path: The relative file path, using the first missing or resized file in manifest order for local checks. expected: What the manifest records, for example ``"1,024 bytes"`` or ``"sha256:"``. actual: What the file holds, in the same form, or ``None`` if the file is missing. - hint: How the caller can get an intact copy, added by the front end, or ``None``. """ def __init__(self, release: "DatasetRef", owner: str, path: str, expected: str, actual: str | None) -> None: @@ -260,12 +284,48 @@ def __init__(self, release: "DatasetRef", owner: str, path: str, expected: str, self.path = path self.expected = expected self.actual = actual - self.hint: str | None = None - def __str__(self) -> str: + def _message(self) -> str: found = "is missing" if self.actual is None else f"has {self.actual}" - message = f"{self.owner}: file {self.path!r} {found}, expected {self.expected}." - return message if self.hint is None else f"{message} {self.hint}" + return f"{self.owner}: file {self.path!r} {found}, expected {self.expected}." + + +class TimeNetManifestError(TimeFFormatError): + """A stored manifest is not a valid manifest. + + Attributes: + where: The file of the manifest, or a description of where a registry keeps it. + detail: The first problem that the parser found. + path: The file of the manifest, or ``None`` for a manifest that a registry served. + """ + + def __init__(self, where: str, detail: str, path: "Path | None" = None) -> None: + super().__init__(where, detail, path) + self.where = where + self.detail = detail + self.path = path + + def _message(self) -> str: + return f"{self.where} is not a valid manifest: {self.detail}" + + +class TimeNetOriginError(TimeFFormatError): + """The provenance record of a release in the local registry is not valid. + + Attributes: + release: The release of the record. + path: The file of the record. + detail: The first problem that the parser found. + """ + + def __init__(self, release: "DatasetRef", path: str, detail: str) -> None: + super().__init__(release, path, detail) + self.release = release + self.path = path + self.detail = detail + + def _message(self) -> str: + return f"{self.path} is not a valid provenance record: {self.detail}." class TimeFReaderError(TimeNetError): diff --git a/packages/timenet/src/timenet/manifest/__init__.py b/packages/timenet/src/timenet/manifest/__init__.py index 9f8984f9a..480bdf788 100644 --- a/packages/timenet/src/timenet/manifest/__init__.py +++ b/packages/timenet/src/timenet/manifest/__init__.py @@ -2,7 +2,7 @@ from timenet.manifest.counts import ManifestCounts from timenet.manifest.files import FilePart, ManifestFiles -from timenet.manifest.manifest import BuildEnvironment, Manifest, parse_manifest, read_manifest, same_release +from timenet.manifest.manifest import BuildEnvironment, Manifest, parse_manifest, read_manifest from timenet.manifest.payload_source import PayloadSource @@ -15,5 +15,4 @@ "PayloadSource", "parse_manifest", "read_manifest", - "same_release", ] diff --git a/packages/timenet/src/timenet/manifest/manifest.py b/packages/timenet/src/timenet/manifest/manifest.py index 0d48886f9..c2b099509 100644 --- a/packages/timenet/src/timenet/manifest/manifest.py +++ b/packages/timenet/src/timenet/manifest/manifest.py @@ -6,7 +6,7 @@ from pydantic import Field, StrictStr, ValidationError, model_validator from typing_extensions import TypedDict -from timenet.errors import TimeFFormatError +from timenet.errors import ReleaseDifference, TimeFFormatError, TimeNetManifestError from timenet.manifest.counts import ManifestCounts from timenet.manifest.files import ManifestFiles from timenet.manifest.payload_source import PayloadSource @@ -120,37 +120,46 @@ def to_json(self) -> str: validated = type(self).model_validate(self) return validated.model_dump_json(indent=2, by_alias=True) + def difference_from(self, other: "Manifest") -> ReleaseDifference | None: + """Compare this manifest with another one of the same release, ignoring their build environments. -def same_release(a: Manifest, b: Manifest) -> bool: - """Compare two manifests, ignoring their build environments. + Args: + other: The other manifest. - Args: - a: One manifest. - b: The other manifest. + Returns: + ``FILES`` if the file records differ, else ``CARD`` if the rest differs, or ``None`` for the same + release. + """ + if _file_records(self) != _file_records(other): + return ReleaseDifference.FILES + if self.model_copy(update={"build_env": {}}) != other.model_copy(update={"build_env": {}}): + return ReleaseDifference.CARD + return None - Returns: - Whether the manifests are equal apart from ``build_env``. - """ - return a.model_copy(update={"build_env": {}}) == b.model_copy(update={"build_env": {}}) +def _file_records(manifest: Manifest) -> frozenset[tuple[str, int, str]]: + """Return the path, size, and checksum of each file of a manifest, in no order.""" + return frozenset((part.path, part.size, part.checksum) for part in manifest.files.all_files()) -def parse_manifest(text: str | bytes, where: str) -> Manifest: + +def parse_manifest(text: str | bytes, where: str, *, path: Path | None = None) -> Manifest: """Parse the JSON text of a stored manifest. Args: text: The JSON text. where: The source location to include in errors. + path: The file of the manifest, or ``None`` for a manifest that a registry served. Returns: The manifest. Raises: - TimeFFormatError: If the text is not a valid manifest. + TimeNetManifestError: If the text is not a valid manifest. """ try: return Manifest.model_validate_json(text) except ValidationError as error: - raise TimeFFormatError(f"{where} is not a valid manifest: {_first_problem(error)}") from error + raise TimeNetManifestError(where, _first_problem(error), path) from error def read_manifest(path: Path) -> Manifest: @@ -163,10 +172,11 @@ def read_manifest(path: Path) -> Manifest: The manifest. Raises: - TimeFFormatError: If the file cannot be read or is not a valid manifest. - """ + TimeFFormatError: If the file cannot be read. + TimeNetManifestError: If the file is not a valid manifest. + """ # ruff: ignore[docstring-extraneous-exception] - raised by parse_manifest try: text = path.read_bytes() except OSError as error: raise TimeFFormatError(f"cannot read the manifest {path}: {error.strerror or error}") from error - return parse_manifest(text, str(path)) + return parse_manifest(text, str(path), path=path) diff --git a/packages/timenet/src/timenet/producer.py b/packages/timenet/src/timenet/producer.py index d32caf49d..e4516104c 100644 --- a/packages/timenet/src/timenet/producer.py +++ b/packages/timenet/src/timenet/producer.py @@ -7,6 +7,7 @@ from collections.abc import Sequence from pathlib import Path +from timenet._hints import Facts, Operation, RequestedRelease, explain from timenet._registry_uri import registry_location from timenet.builders import BuildOptions, declared from timenet.client import as_release_sources, default_local_registry, default_sources @@ -17,7 +18,7 @@ from timenet.registry import BaseRegistry, LocalRegistry, WritableRegistry, copy_release, open_registry from timenet.registry._paths import release_dir from timenet.sources import NotFound, NotServed, ReleaseSource, resolve_release -from timenet.types.references import DatasetRef, is_latest, split_ref +from timenet.types.references import DatasetRef, is_latest def build( # ruff: ignore[too-many-arguments] @@ -67,13 +68,14 @@ def build( # ruff: ignore[too-many-arguments] if sources is None else as_release_sources(sources, keyword="sources") ) - bare_id, version = split_ref(dataset_id) - # A connector builds only the version of its card, so "latest" asks for that version. - builder = declared(bare_id, None if is_latest(version) else version) - release, steps = plan_steps( - builder, target, parents_from=parents, force_build=force, open_access_only=open_access_only - ) - run_plan(Plan(tuple(steps)), target, options=BuildOptions(keep_cache=keep_cache, progress=progress)) + wanted = RequestedRelease.parse(dataset_id) + with explain(Facts([target, *parents], target, request=Operation.BUILD, wanted=wanted)): + # A connector builds only the version of its card, so "latest" asks for that version. + builder = declared(wanted.dataset_id, None if is_latest(wanted.version) else wanted.version) + release, steps = plan_steps( + builder, target, parents_from=parents, force_build=force, open_access_only=open_access_only + ) + run_plan(Plan(tuple(steps)), target, options=BuildOptions(keep_cache=keep_cache, progress=progress)) return release_dir(target.root, release) @@ -108,22 +110,55 @@ def publish( """ # ruff: ignore[docstring-extraneous-exception] - raised by open_registry and copy_release target = open_registry(dest, label="dest") if isinstance(dest, (str, Path)) else dest if source is None: - origin = default_local_registry() - else: - origin = open_registry(source, label="source") if isinstance(source, (str, Path)) else source - if isinstance(dataset_id, DatasetRef): - # The caller names the exact release, so the copy starts at once. - copy_release(dataset_id, origin, target, progress=progress) - return dataset_id - bare_id, version = split_ref(dataset_id) - answer = resolve_release([origin], bare_id, version) - if isinstance(answer, NotServed): - raise answer.access_error(bare_id) - if isinstance(answer, NotFound): - wanted = bare_id if version is None else f"{bare_id}@{version}" - raise TimeNetDatasetNotFoundError(f"{wanted} is not in {origin}.") - release = DatasetRef(dataset_id=bare_id, version=answer.version) - copy_release(release, origin, target, progress=progress) + local_registry = default_local_registry() + return _publish_from( + dataset_id, dest=target, origin=local_registry, local_registry=local_registry, progress=progress + ) + origin = open_registry(source, label="source") if isinstance(source, (str, Path)) else source + # Only the default local registry gets the hints that copy or build a release into it. + return _publish_from(dataset_id, dest=target, origin=origin, local_registry=None, progress=progress) + + +def _publish_from( + dataset_id: str | DatasetRef, + *, + dest: WritableRegistry, + origin: BaseRegistry, + local_registry: LocalRegistry | None, + progress: ProgressCallback | None, +) -> DatasetRef: + """Copy a release and its required payload sources from one opened registry to another, as :func:`publish` does. + + Args: + dataset_id: The release, as in :func:`publish`. + dest: The registry to copy to. + origin: The registry to copy from. + local_registry: The local registry whose hints copy or build the release into it, or ``None`` for no such hint. + progress: A callback for copy progress events. + + Returns: + The ID and version of the release in ``dest``. + + Raises: + TimeNetDatasetNotFoundError: If ``origin`` lacks the release. + TimeNetAccessError: If a registry withholds files or requires open access that a copied release lacks. + TimeNetVersionConflictError: If ``dest`` holds different files for the release or a required payload source. + TimeNetMissingParentError: If a required payload source is missing from ``origin`` and incomplete in ``dest``. + """ # ruff: ignore[docstring-extraneous-exception] - raised by copy_release + wanted = RequestedRelease.parse(dataset_id) + facts = Facts([origin], local_registry, request=Operation.PUBLISH, dest=dest, wanted=wanted) + with explain(facts): + if isinstance(dataset_id, DatasetRef): + # The caller names the exact release, so the copy needs no lookup. + release = dataset_id + else: + answer = resolve_release([origin], wanted.dataset_id, wanted.version) + if isinstance(answer, NotServed): + raise answer.access_error(wanted.dataset_id) + if isinstance(answer, NotFound): + raise TimeNetDatasetNotFoundError(f"{wanted} is not in {origin}.") + release = DatasetRef(dataset_id=wanted.dataset_id, version=answer.version) + copy_release(release, origin, dest, progress=progress) return release diff --git a/packages/timenet/src/timenet/registry/_copy.py b/packages/timenet/src/timenet/registry/_copy.py index a1b060ed4..91017cd43 100644 --- a/packages/timenet/src/timenet/registry/_copy.py +++ b/packages/timenet/src/timenet/registry/_copy.py @@ -9,7 +9,7 @@ import tempfile from timenet.errors import TimeFValidationError, TimeNetMissingParentError, TimeNetVersionConflictError -from timenet.manifest import Manifest, same_release +from timenet.manifest import Manifest from timenet.progress import BytesMoved, ProgressCallback, StepFinished, StepStarted from timenet.registry._paths import release_dir from timenet.registry.base import BaseRegistry @@ -240,11 +240,11 @@ def _needs_copy( held = dest.find_manifest(reference) if held is None: return True - files_differ = _file_records(held) != _file_records(manifest) - if not files_differ and same_release(held, manifest): + difference = held.difference_from(manifest) + if difference is None: return not _complete(reference, manifest, dest) needed_by = None if reference == requested else requested - raise TimeNetVersionConflictError(reference, str(dest), str(source), needed_by, "files" if files_differ else "card") + raise TimeNetVersionConflictError(reference, str(dest), dest.location, str(source), difference, needed_by) def _complete(reference: DatasetRef, manifest: Manifest, dest: WritableRegistry) -> bool: @@ -262,15 +262,3 @@ def _complete(reference: DatasetRef, manifest: Manifest, dest: WritableRegistry) ``True`` for a complete local copy or any release in a non-local registry. """ return not isinstance(dest, LocalRegistry) or dest.damage(reference, manifest) is None - - -def _file_records(manifest: Manifest) -> frozenset[tuple[str, int, str]]: - """Collect each file's path, size, and checksum from a manifest. - - Args: - manifest: The manifest to read. - - Returns: - A frozen set of ``(path, size, checksum)`` entries. - """ - return frozenset((part.path, part.size, part.checksum) for part in manifest.files.all_files()) diff --git a/packages/timenet/src/timenet/registry/local.py b/packages/timenet/src/timenet/registry/local.py index 3f752bcdb..5f65a45af 100644 --- a/packages/timenet/src/timenet/registry/local.py +++ b/packages/timenet/src/timenet/registry/local.py @@ -13,7 +13,13 @@ from timenet._fs import delete_and_measure, same_directory, staged_directory from timenet.dataset import TimeFDataset -from timenet.errors import TimeFFormatError, TimeFValidationError, TimeNetDatasetNotFoundError, TimeNetFileError +from timenet.errors import ( + TimeFFormatError, + TimeFValidationError, + TimeNetDatasetNotFoundError, + TimeNetFileError, + TimeNetOriginError, +) from timenet.format.constants import MANIFEST_FILE from timenet.manifest import Manifest, PayloadSource, read_manifest from timenet.registry._paths import release_dir @@ -97,6 +103,27 @@ def releases(self) -> list[DatasetRef]: """ # ruff: ignore[docstring-extraneous-exception] (raised by _walk) return self._walk()[0] + def release_at(self, path: Path) -> DatasetRef | None: + """Return the release whose folder holds a path. + + Args: + path: A path, whether or not it exists. + + Returns: + The release, or ``None`` if no release folder of this registry holds the path. + """ + resolved = path.expanduser().resolve() + release_depth = 3 # // below the root + for folder in (resolved, *resolved.parents): + if len(folder.parents) < release_depth or not same_directory(folder.parents[release_depth - 1], self._root): + continue + try: + release = DatasetRef.parse(f"{folder.parent.parent.name}/{folder.parent.name}@{folder.name}") + except TimeFValidationError: + return None + return release if folder.name == str(release.version) else None + return None + def check_no_linked_folders(self) -> None: """Check that no dataset or release folder is a symlink, which :meth:`releases` would skip. @@ -210,7 +237,7 @@ def read_origin(self, release: DatasetRef) -> Origin | None: The record, or ``None`` if the release has none. Raises: - TimeFFormatError: If the record is invalid. + TimeNetOriginError: If the record is invalid. """ path = release_dir(self._root, release) / _ORIGIN_FILE try: @@ -220,7 +247,7 @@ def read_origin(self, release: DatasetRef) -> Origin | None: try: return Origin.model_validate_json(text) except ValidationError as error: - raise TimeFFormatError(f"{path} is not a valid provenance record: {_first_problem(error)}.") from error + raise TimeNetOriginError(release, str(path), _first_problem(error)) from error def get_manifest(self, dataset_id: str, version: str | None = None) -> Manifest: """Return a dataset's manifest (latest version if unspecified). @@ -238,11 +265,7 @@ def get_manifest(self, dataset_id: str, version: str | None = None) -> Manifest: """ resolved = self._latest_version(dataset_id) if is_latest(version) else version if resolved is None: - known = ", ".join(m.dataset_id for m in self.list_datasets()) or "(none)" - raise TimeNetDatasetNotFoundError( - f"no committed version for dataset {dataset_id!r}; this registry has: {known}. " - "Build one with `timenet-build build `." - ) + raise TimeNetDatasetNotFoundError(f"{self} holds no version of {dataset_id!r}") path = release_dir(self._root, DatasetRef.parse(f"{dataset_id}@{resolved}")) / MANIFEST_FILE if not path.exists(): raise TimeNetDatasetNotFoundError(f"no manifest for {dataset_id!r} version {resolved!r}") diff --git a/packages/timenet/src/timenet/registry/remote/_http.py b/packages/timenet/src/timenet/registry/remote/_http.py index bfbdbf721..936f5b2ee 100644 --- a/packages/timenet/src/timenet/registry/remote/_http.py +++ b/packages/timenet/src/timenet/registry/remote/_http.py @@ -18,7 +18,7 @@ import httpx -from timenet.errors import TimeNetDatasetNotFoundError, TimeNetRegistryError +from timenet.errors import TimeNetDatasetNotFoundError, TimeNetRegistryError, TimeNetUnreachableError _LOG = logging.getLogger(__name__) @@ -133,12 +133,12 @@ def _reaching(where: str) -> Generator[None]: where: The registry or storage location to name in the error. Raises: - TimeNetRegistryError: If a request fails to reach ``where``. + TimeNetUnreachableError: If a request fails to reach ``where``. """ try: yield except httpx.TransportError as error: - raise TimeNetRegistryError(f"cannot reach {where}: {error}") from error + raise TimeNetUnreachableError(f"cannot reach {where}: {error}") from error def timenet_user_agent() -> str: diff --git a/packages/timenet/src/timenet/registry/s3.py b/packages/timenet/src/timenet/registry/s3.py index 41fc3bce4..1209c028f 100644 --- a/packages/timenet/src/timenet/registry/s3.py +++ b/packages/timenet/src/timenet/registry/s3.py @@ -21,7 +21,7 @@ import pyarrow.fs as pafs -from timenet.errors import TimeNetDatasetNotFoundError, TimeNetRegistryError +from timenet.errors import TimeNetDatasetNotFoundError, TimeNetRegistryError, TimeNetUnreachableError from timenet.format.constants import MANIFEST_FILE from timenet.manifest import Manifest, parse_manifest from timenet.registry.lazy_read import LazyReadRegistry @@ -234,14 +234,14 @@ def _reaching(self) -> Generator[None]: """Name the bucket in the error of a request that fails on the network. Raises: - TimeNetRegistryError: If a request fails to reach the bucket. + TimeNetUnreachableError: If a request fails to reach the bucket. """ import botocore.exceptions # ruff: ignore[import-outside-top-level] - optional S3 dependency try: yield except (botocore.exceptions.ConnectionError, botocore.exceptions.HTTPClientError) as error: - raise TimeNetRegistryError(f"cannot reach {self._display_root()}: {error}") from error + raise TimeNetUnreachableError(f"cannot reach {self._display_root()}: {error}") from error def _key(self, *parts: str) -> str: # ruff: ignore[docstring-missing-returns] diff --git a/packages/timenet/tests/registry/test_copy.py b/packages/timenet/tests/registry/test_copy.py index 871a76098..db04faaef 100644 --- a/packages/timenet/tests/registry/test_copy.py +++ b/packages/timenet/tests/registry/test_copy.py @@ -16,6 +16,7 @@ from timenet.dataset import Record, Signal, Source, TimeFDataset from timenet.errors import ( + ReleaseDifference, TimeFFormatError, TimeFValidationError, TimeNetAccessError, @@ -215,7 +216,7 @@ def test_copy_release_refuses_a_release_with_the_same_files_and_another_card(sou # A fresh registry: the rewrite can keep the mtime of the first write, so a cached manifest could stay. with pytest.raises(TimeNetVersionConflictError, match="another card than") as raised: copy_release(child_ref, source, LocalRegistry(dest.root)) - assert raised.value.difference == "card" + assert raised.value.difference is ReleaseDifference.CARD assert path.read_bytes() == before @@ -232,11 +233,15 @@ def test_copy_release_refuses_another_build_of_the_version(source, dest): # the destination holds child_ref built from other records dest.put_dataset(_other_build(child_ref.dataset_id)) before = dest.get_manifest(*child) - with pytest.raises(TimeNetVersionConflictError, match="new version") as raised: + with pytest.raises(TimeNetVersionConflictError, match="with other files than") as raised: copy_release(child_ref, source, dest) assert (raised.value.release, raised.value.needed_by) == (child_ref, None) - assert (raised.value.registry, raised.value.source) == (str(dest), str(source)) - assert raised.value.difference == "files" # the card differs too, but the files come first + assert (raised.value.registry, raised.value.registry_location, raised.value.source) == ( + str(dest), + dest.location, + str(source), + ) + assert raised.value.difference is ReleaseDifference.FILES # the card differs too, but the files come first assert dest.get_manifest(*child) == before assert not dest.exists(*parent) # nothing is copied, not even the payload source @@ -346,7 +351,7 @@ def test_copy_release_with_force_copies_a_complete_local_copy_again(source, tmp_ # force=True does not replace a copy with other files. other = LocalRegistry(tmp_path / "other") other.put_dataset(_other_build(child_ref.dataset_id)) - with pytest.raises(TimeNetVersionConflictError, match="new version"): + with pytest.raises(TimeNetVersionConflictError, match="with other files than"): copy_release(child_ref, source, other, force=True) diff --git a/packages/timenet/tests/registry/test_factory.py b/packages/timenet/tests/registry/test_factory.py index 45cb3febb..37bb7d2dd 100644 --- a/packages/timenet/tests/registry/test_factory.py +++ b/packages/timenet/tests/registry/test_factory.py @@ -4,6 +4,7 @@ from timenet.registry import LocalRegistry -def test_unknown_dataset_error_is_actionable(tmp_path): - with pytest.raises(TimeNetDatasetNotFoundError, match="Build one with"): +def test_an_unknown_dataset_names_the_registry_and_the_dataset(tmp_path): + with pytest.raises(TimeNetDatasetNotFoundError) as caught: LocalRegistry(tmp_path).get_manifest("nope/missing") + assert str(caught.value) == f"local registry {tmp_path} holds no version of 'nope/missing'" diff --git a/packages/timenet/tests/registry/test_remote.py b/packages/timenet/tests/registry/test_remote.py index aa62730d5..d8072482b 100644 --- a/packages/timenet/tests/registry/test_remote.py +++ b/packages/timenet/tests/registry/test_remote.py @@ -618,10 +618,6 @@ def test_a_read_or_a_download_checks_access_before_it_reads_from_a_remote_regist getattr(client, entry_point)(dataset_id) assert caught.value.requirements == (AccessRequirement(release, Access.CREDENTIALED, "https://example.org/dua"),) - assert caught.value.hint == ( - "No source serves its files. Add InstalledConnectors() to the sources, or build it locally with " - f"`timenet-build build {dataset_id}`." - ) assert not any("/download/" in request.url.path or request.url.host == "blob.local" for request in requests) assert not (tmp_path / "local").exists() diff --git a/packages/timenet/tests/registry/test_s3.py b/packages/timenet/tests/registry/test_s3.py index fa57bb601..188069cad 100644 --- a/packages/timenet/tests/registry/test_s3.py +++ b/packages/timenet/tests/registry/test_s3.py @@ -156,3 +156,18 @@ def test_an_unreachable_bucket_raises_a_registry_error_that_names_it(monkeypatch monkeypatch.setattr(registry, "_client", _Unreachable) with pytest.raises(TimeNetRegistryError, match=r"^cannot reach s3://tn-test/registry: Could not connect"): registry.resolve("timenet/hello-world", Version(1, 0, 0)) + + +def test_the_latest_version_over_an_unreachable_bucket_names_the_version_in_the_local_registry(tmp_path, monkeypatch): + store = LocalRegistry(tmp_path / "store") + store.put_dataset(make_dataset()) + registry = S3Registry("s3://tn-test/registry") + monkeypatch.setattr(registry, "_client", _Unreachable) + + with pytest.raises(TimeNetRegistryError) as caught: + TimeNet(sources=[store, registry]).open("timenet/hello-world@latest") + + assert caught.value.hint == ( + "Ask for timenet/hello-world@1.0.0 to read the copy in the local registry, or leave the unreachable " + "registry out of the sources." + ) diff --git a/packages/timenet/tests/test_cli.py b/packages/timenet/tests/test_cli.py index 591a7fa08..49759ec8c 100644 --- a/packages/timenet/tests/test_cli.py +++ b/packages/timenet/tests/test_cli.py @@ -1,14 +1,17 @@ import importlib +import shlex import shutil import subprocess import sys +from _origin_record import origin_file import click import pytest from rich.console import Console import typer from typer.testing import CliRunner +from timenet import _hints as hints_module from timenet.cache import human_bytes, stored_releases from timenet.cli.app import _enum_list, app from timenet.client import TimeNet @@ -18,14 +21,17 @@ TimeFValidationError, TimeNetDatasetNotFoundError, TimeNetInvalidCardError, + TimeNetManifestError, TimeNetMissingParentError, + TimeNetNotBuiltError, + TimeNetOriginError, TimeNetRegistryError, ) from timenet.reader import TimeFReader from timenet.registry import LocalRegistry, open_local_version from timenet.registry.origin import OriginKind from timenet.testing import install_backends, make_dataset -from timenet.types import Access, Domain +from timenet.types import Access, Domain, Version from timenet.types.reference_keys import ParentAlias from timenet.types.references import DatasetRef from timenet.writer import TimeFWriter @@ -245,6 +251,38 @@ def _flags(tmp_path, remote): return ["--local-registry", str(tmp_path / "store"), "--remote-registry", str(remote), "--no-auto-build"] +_NO_PROVIDER = ( + "Pass a registry that holds it with --remote-registry , or install a connector package that builds it, such " + "as timenet-connectors." +) + + +def test_a_failed_read_computes_its_hint_once(tmp_path, monkeypatch): + computed = [] + remedy = hints_module._remedy + monkeypatch.setattr(hints_module, "_remedy", lambda error, *rest: computed.append(error) or remedy(error, *rest)) + + result = runner.invoke(app, ["info", "x/y", *_flags(tmp_path, tmp_path / "empty")]) + + assert isinstance(result.exception, TimeNetDatasetNotFoundError) + assert result.exception.hint == _NO_PROVIDER + assert computed == [result.exception] + + +def test_an_sdk_call_after_a_command_gets_the_hint_of_the_sdk(tmp_path): + command = runner.invoke(app, ["info", "x/y", *_flags(tmp_path, tmp_path / "empty")]) + assert isinstance(command.exception, TimeNetDatasetNotFoundError) + assert command.exception.hint == _NO_PROVIDER + + with pytest.raises(TimeNetDatasetNotFoundError) as caught: + TimeNet(sources=[LocalRegistry(tmp_path / "store")]).open("x/y") + + assert caught.value.hint == ( + "Add a registry that holds it to the sources, or install a connector package that builds it, such as " + "timenet-connectors." + ) + + def test_search_no_match(registry_root, tmp_path): result = runner.invoke(app, ["search", "--domain", "cardiology", *_flags(tmp_path, registry_root)]) assert result.exit_code == 0, result.output @@ -252,20 +290,20 @@ def test_search_no_match(registry_root, tmp_path): class _Backend: - """A builder backend with one connector, whose card declares ``test/gated@1.0.0``.""" + """A builder backend whose card of each dataset is the demo card under its ID, with ``changes``.""" + + def __init__(self, *dataset_ids, **changes): + self.ids = dataset_ids + self.changes = changes def dataset_ids(self): - return ["test/gated"] + return list(self.ids) def card(self, dataset_id): - return make_dataset().metadata.model_copy( - update={ - "dataset_id": dataset_id, - "access": Access.CREDENTIALED, - "access_url": "https://gated.example", - "citation": "A citation.", - } - ) + return make_dataset().metadata.model_copy(update={"dataset_id": dataset_id, **self.changes}) + + +_GATED = {"access": Access.CREDENTIALED, "access_url": "https://gated.example", "citation": "A citation."} def test_list_shows_a_warning_as_a_line_without_a_file_and_a_line_number(tmp_path, monkeypatch): @@ -273,7 +311,7 @@ class _Broken(_Backend): def card(self, dataset_id): raise TimeNetInvalidCardError("the card cannot be read") - install_backends(monkeypatch, _Broken()) + install_backends(monkeypatch, _Broken("test/gated")) monkeypatch.setattr(_cli_module.ui, "rich", Console(stderr=True, width=1000)) flags = ["--local-registry", str(tmp_path / "store"), "--remote-registry", str(tmp_path / "empty"), "--auto-build"] @@ -287,7 +325,7 @@ def card(self, dataset_id): def test_info_shows_the_card_of_a_dataset_that_only_a_connector_has(tmp_path, monkeypatch): - install_backends(monkeypatch, _Backend()) + install_backends(monkeypatch, _Backend("test/gated", **_GATED)) store = tmp_path / "store" flags = ["--local-registry", str(store), "--remote-registry", str(tmp_path / "empty"), "--auto-build"] @@ -295,7 +333,7 @@ def test_info_shows_the_card_of_a_dataset_that_only_a_connector_has(tmp_path, mo assert result.exit_code == 0, result.output cells = _table_cells(result.stdout) - card = _Backend().card("test/gated") + card = _Backend("test/gated", **_GATED).card("test/gated") assert cells == [ ["version", "1.0.0"], ["name", card.name], @@ -314,15 +352,8 @@ def test_info_shows_the_card_of_a_dataset_that_only_a_connector_has(tmp_path, mo def test_info_of_an_unbuilt_child_says_that_its_access_leaves_out_its_parents(tmp_path, monkeypatch): - class _Child(_Backend): - def dataset_ids(self): - return ["test/child"] - - def card(self, dataset_id): - parents = {"a": "test/a@1.0.0", "b": "test/b@1.0.0"} - return make_dataset(dataset_id=dataset_id, parents=parents).metadata - - install_backends(monkeypatch, _Child()) + parents = {ParentAlias(name): DatasetRef.parse(f"test/{name}@1.0.0") for name in ("a", "b")} + install_backends(monkeypatch, _Backend("test/child", parents=parents)) # The access row is long, so the console must not wrap it. monkeypatch.setattr(_cli_module, "console", Console(width=1000)) flags = ["--local-registry", str(tmp_path / "store"), "--remote-registry", str(tmp_path / "empty"), "--auto-build"] @@ -342,7 +373,7 @@ def card(self, dataset_id): reads.append(dataset_id) return super().card(dataset_id) - install_backends(monkeypatch, _Counting()) + install_backends(monkeypatch, _Counting("test/gated")) flags = ["--local-registry", str(tmp_path / "store"), "--remote-registry", str(tmp_path / "empty"), "--auto-build"] result = runner.invoke(app, ["info", "test/gated", *flags]) @@ -466,11 +497,16 @@ def test_publish_copies_a_release_from_the_local_registry(tmp_path): assert "✅ Published timenet/hello-world@1.0.0 to" in result.stderr +def _words(output): + """Return the unstyled output with all whitespace removed, since a usage panel wraps its lines.""" + return "".join(click.unstyle(output).split()) + + def test_publish_rejects_a_source_that_cannot_be_opened(tmp_path): result = runner.invoke(app, ["publish", "x/y", "--dest", str(tmp_path / "dest"), "--source", "foo://x"]) - assert result.exit_code == 1 - assert isinstance(result.exception, TimeNetRegistryError) - assert str(result.exception).startswith("--source 'foo://x' has an unsupported registry scheme.") + # A bad flag is a usage error that names the flag. + assert result.exit_code == 2 + assert "--source'foo://x'hasanunsupportedregistryscheme." in _words(result.output) def test_publish_to_an_empty_dest_fails_and_writes_nothing_into_the_working_directory(tmp_path, monkeypatch): @@ -481,8 +517,8 @@ def test_publish_to_an_empty_dest_fails_and_writes_nothing_into_the_working_dire monkeypatch.chdir(cwd) argv = ["publish", "timenet/hello-world", "--dest", "", "--local-registry", str(store.root)] result = runner.invoke(app, argv) - assert isinstance(result.exception, TimeNetRegistryError) - assert str(result.exception).startswith("dest is blank") + assert result.exit_code == 2 + assert "--destisblank" in _words(result.output) assert list(cwd.iterdir()) == [] @@ -493,47 +529,53 @@ def test_publish_needs_dest(tmp_path): assert "--dest" in click.unstyle(result.output) -def test_publish_of_a_release_that_the_local_registry_lacks_names_the_fix(tmp_path): - store, other, dest = tmp_path / "store", tmp_path / "other", str(tmp_path / "dest") - result = runner.invoke(app, ["publish", "timenet/hello-world", "--dest", dest, "--local-registry", str(store)]) - 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 --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. - result = runner.invoke(app, ["publish", "timenet/hello-world@1.0.0", "--dest", dest]) - assert str(result.exception).endswith( - "Build it into the local registry with `timenet-build build timenet/hello-world@1.0.0`, " - "or copy it there with `timenet download timenet/hello-world@1.0.0`." - ) - # A registry that --source names is not the store, so the error names only that registry. - result = runner.invoke(app, ["publish", "timenet/hello-world", "--dest", dest, "--source", str(other)]) - assert str(result.exception) == f"timenet/hello-world is not in local registry {other}." +@pytest.mark.parametrize("provider", ["connector", "no connector", "source"]) +def test_publish_of_a_release_that_the_local_registry_lacks_names_the_fix(tmp_path, monkeypatch, provider): + local, other, dest = tmp_path / "local", tmp_path / "other", str(tmp_path / "dest") + if provider != "no connector": + install_backends(monkeypatch, _Backend("timenet/hello-world")) + argv = ["publish", "timenet/hello-world@1.0.0", "--dest", dest, "--local-registry", str(local)] + if provider == "source": + argv += ["--source", str(other)] + result = runner.invoke(app, argv) -def test_publish_adds_no_hint_to_a_payload_source_that_the_local_registry_lacks(tmp_path): - store = LocalRegistry(tmp_path / "store") - store.put_dataset(make_dataset()) + assert type(result.exception) is TimeNetDatasetNotFoundError + registry = other if provider == "source" else local + # The hint repeats the version and the local registry. A registry that --source names gets none. + # Without a connector, the hint names the download, although it asks no registry for the release. + hint = { + "connector": " Build it into the local registry with " + f"`timenet-build build timenet/hello-world@1.0.0 --local-registry {local}`.", + "no connector": " Copy it into the local registry with " + f"`timenet download timenet/hello-world@1.0.0 --local-registry {local}`.", + "source": "", + }[provider] + assert str(result.exception) == f"timenet/hello-world@1.0.0 is not in local registry {registry}.{hint}" + + +def test_publish_names_the_download_of_a_payload_source_that_the_local_registry_lacks(tmp_path): + local = LocalRegistry(tmp_path / "local") + local.put_dataset(make_dataset()) child = TimeFDataset( metadata=make_dataset().metadata.model_copy( update={"dataset_id": "test/child", "parents": {ParentAlias("hello"): HELLO}} ) ) # The child reads the values of one Record of the parent, so the parent is its payload source. - with TimeNet(sources=[store]).open(str(HELLO)) as reader: + with TimeNet(sources=[local]).open(str(HELLO)) as reader: child.import_from(reader, alias=ParentAlias("hello"), records=[make_dataset().records[0].id]) - store.put_dataset(child) - shutil.rmtree(store.root / "timenet") + local.put_dataset(child) + shutil.rmtree(local.root / "timenet") dest = tmp_path / "dest" - result = runner.invoke(app, ["publish", "test/child", "--dest", str(dest), "--local-registry", str(store.root)]) + result = runner.invoke(app, ["publish", "test/child", "--dest", str(dest), "--local-registry", str(local.root)]) - # The error of the payload source names its own registries, and the hint for the release does not apply. + # The hint names the payload source, not the release of the command. assert type(result.exception) is TimeNetMissingParentError - assert str(result.exception).startswith(f"payload source {HELLO} of test/child@1.0.0 is not in ") - assert "timenet-build" not in str(result.exception) + assert result.exception.hint == ( + f"Copy it into the local registry with `timenet download {HELLO} --local-registry {local.root}`." + ) @pytest.mark.parametrize(("flag", "variable"), [("--auto-build", "false"), ("--no-auto-build", "true")]) @@ -558,8 +600,92 @@ def test_an_empty_flag_is_an_error(tmp_path, monkeypatch, argv): # `local clear` would delete its releases. monkeypatch.chdir(tmp_path) result = runner.invoke(app, argv) - assert isinstance(result.exception, TimeFValidationError) - assert f"must not be empty (also as {argv[-2]})" in str(result.exception) + # A usage error that names the flag. + assert result.exit_code == 2 + assert f"{argv[-2]}isblank" in _words(result.output) + + +def test_a_bad_variable_is_an_error_that_names_the_variable(monkeypatch): + monkeypatch.setenv("TIMENET_LOCAL_REGISTRY", "https://registry.example") + result = runner.invoke(app, ["list"]) + # The command line is fine, so this is no usage error. + assert result.exit_code == 1 + assert isinstance(result.exception, TimeNetRegistryError) + assert str(result.exception).startswith("TIMENET_LOCAL_REGISTRY 'https://registry.example' is remote") + + +def test_info_of_a_release_that_no_source_provides_names_the_flags_that_add_one(monkeypatch): + install_backends(monkeypatch, _Backend("test/x")) + + result = runner.invoke(app, ["info", "test/x", "--no-auto-build"]) + + # The CLI names its flags, not the calls of the SDK. + assert isinstance(result.exception, TimeNetDatasetNotFoundError) + assert result.exception.hint == ( + "Pass a registry that holds it with --remote-registry , or build it from its installed connector " + "with --auto-build." + ) + + +def test_download_of_the_latest_version_names_the_build_and_the_version_that_a_registry_holds( + registry_root, tmp_path, monkeypatch +): + # The connector declares 2.0.0, and the registry holds 1.0.0. + backend = _Backend("timenet/hello-world", dataset_version=Version(2, 0, 0)) + install_backends(monkeypatch, backend) + flags = ["--local-registry", str(tmp_path / "store"), "--remote-registry", str(registry_root)] + + result = runner.invoke(app, ["download", "timenet/hello-world@latest", *flags]) + + assert isinstance(result.exception, TimeNetNotBuiltError) + assert result.exception.hint == ( + f"Build it with `timenet-build build timenet/hello-world --local-registry {tmp_path / 'store'} " + f"--remote-registry {registry_root}`, or download the version that a registry holds with " + f"`timenet download timenet/hello-world@1.0.0 --local-registry {tmp_path / 'store'} " + f"--remote-registry {registry_root}`." + ) + + +@pytest.mark.parametrize("command", [["list", "--no-auto-build"], ["local", "downloads", "clear", "--yes"]]) +def test_a_manifest_that_is_not_valid_names_the_delete_that_replaces_its_release(state, command): + store, _ = state + directory = store / "timenet/hello-world/1.0.0" + (directory / "manifest.json").write_text("{cut") + + result = runner.invoke(app, command) + + assert result.exit_code == 1 + assert isinstance(result.exception, TimeNetManifestError) + assert str(result.exception).startswith(f"{directory / 'manifest.json'} is not a valid manifest: ") + # Download and build read the manifest first, so only a delete lets them replace the release. + again = f"Delete {directory} and download or build timenet/hello-world@1.0.0 again" + assert result.exception.hint == (f"{again}, then clear again." if command[0] == "local" else f"{again}.") + + +@pytest.mark.parametrize( + "command", [["local", "downloads", "clear", "--yes"], ["local", "info"], ["local", "downloads", "info"]] +) +def test_a_damaged_provenance_record_names_the_forced_download_and_build(tmp_path, command): + store = tmp_path / "store" + LocalRegistry(store).put_dataset(make_dataset()) + origin_file(LocalRegistry(store), HELLO).write_text("{") + + result = runner.invoke(app, [*command, "--local-registry", str(store)]) + + assert result.exit_code == 1 + assert isinstance(result.exception, TimeNetOriginError) + flag = f"--local-registry {shlex.quote(str(store))}" + assert result.exception.hint == ( + f"Get the release again with `timenet download {HELLO} {flag} --force` if it came from a registry, or " + f"with `timenet-build build {HELLO} {flag} --force` if it was built. Both replace the release and its record." + ) + + +def test_the_local_registry_flag_replaces_a_variable_that_is_not_valid(registry_root, tmp_path, monkeypatch): + monkeypatch.setenv("TIMENET_LOCAL_REGISTRY", "") + result = runner.invoke(app, ["list", *_flags(tmp_path, registry_root)]) + assert result.exit_code == 0, result.output + assert "timenet/hello-world" in result.stdout def test_search_rejects_unknown_filter_value(registry_root, tmp_path): diff --git a/packages/timenet/tests/test_client.py b/packages/timenet/tests/test_client.py index f9771d96c..0fc57cba4 100644 --- a/packages/timenet/tests/test_client.py +++ b/packages/timenet/tests/test_client.py @@ -54,12 +54,13 @@ class _Builder: VERSION = "1.0.0" BACKEND = "parquet" + IDS = ("test/child", "timenet/hello-world") def __init__(self, calls): self.calls = calls def dataset_ids(self): - return ("test/child", "timenet/hello-world") + return self.IDS def card(self, dataset_id): version = DatasetRef.parse(f"{dataset_id}@{self.VERSION}").version @@ -80,12 +81,11 @@ def test_a_child_reads_its_missing_payload_source_from_a_later_registry(tmp_path with pytest.raises(TimeNetMissingParentError) as caught: TimeNet(sources=[store]).open("test/child") assert (caught.value.missing, caught.value.needed_by) == (reference, DatasetRef.parse("test/child@1.0.0")) - assert str(caught.value).endswith( - ". Add a registry that holds it to the sources, or add InstalledConnectors() to build it." + # No connector package is installed, so the hint names the package that builds the payload source. + assert caught.value.hint == ( + "Add a registry that holds it to the sources, or install a connector package that builds it, such as " + "timenet-connectors." ) - # No installed connector builds the payload source, so with InstalledConnectors() the hint names only a registry. - with pytest.raises(TimeNetMissingParentError, match=r"\. Add a registry that holds it to the sources\.$"): - TimeNet(sources=[store, InstalledConnectors()]).open("test/child") with TimeNet(sources=[store, str(origin.root), InstalledConnectors()]).open("test/child") as reader: assert reader.read().records[0].signals[0].to_arrow().equals(make_dataset().records[0].signals[0].to_arrow()) # The read opens the payload source in origin, where it is, and copies nothing. @@ -159,6 +159,22 @@ def test_a_damaged_release_outside_the_local_registry_names_no_download(tmp_path assert (caught.value.release, caught.value.hint) == (DatasetRef.parse("test/child@1.0.0"), None) +def test_a_failed_transfer_names_no_repair_of_the_intact_copy_in_the_local_registry(registry_root, tmp_path): + client = TimeNet(sources=[LocalRegistry(tmp_path / "store"), str(registry_root)]) + client.download("timenet/hello-world") + # The registry now serves a file with other bytes of the same size. + part = next((registry_root / "timenet/hello-world/1.0.0").rglob("*.parquet")) + data = bytearray(part.read_bytes()) + data[-20] ^= 0xFF + part.write_bytes(bytes(data)) + + with pytest.raises(TimeNetFileError, match=r"^download of ") as caught: + client.download("timenet/hello-world", force=True) + + # The copy in the local registry is intact, so the local registry needs no repair. + assert caught.value.hint is None + + def test_a_damaged_build_in_the_local_registry_names_the_build_that_replaces_it(tmp_path, monkeypatch): calls = [] install_backends(monkeypatch, _Builder(calls)) @@ -172,8 +188,8 @@ def test_a_damaged_build_in_the_local_registry_names_the_build_that_replaces_it( client.load("timenet/hello-world@1.0.0") # the build puts the release into the store (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 --local-registry {store.root} --force`." + # No registry other than the local registry provides the release, so only a build replaces it. + hint = f"Build it again with timenet.build('timenet/hello-world', local_registry='{store.root}', force=True)." with pytest.raises(TimeNetFileError) as caught: client.open("timenet/hello-world@1.0.0") assert caught.value.hint == hint @@ -382,7 +398,10 @@ def test_list_leaves_out_a_registry_without_a_catalog_with_a_warning(tmp_path, r def test_list_and_search_raise_if_no_source_has_a_catalog(tmp_path): client = TimeNet(sources=[_WithoutCatalog(tmp_path / "hidden")]) - message = f"^none of the sources has a catalog: local registry {re.escape(str(tmp_path / 'hidden'))}$" + message = ( + f"^none of the sources has a catalog: local registry {re.escape(str(tmp_path / 'hidden'))}\\. " + r"Read an exact release with open\('@'\)\.$" + ) with pytest.raises(TimeNetNoCatalogError, match=message): client.list() with pytest.raises(TimeNetNoCatalogError, match=message): @@ -498,7 +517,10 @@ def test_get_manifest_of_a_release_that_is_not_built_raises_with_the_card_of_the client.get_manifest("test/child") assert caught.value.release == DatasetRef.parse("test/child@1.0.0") assert caught.value.card == _Builder([]).card("test/child") - assert str(caught.value) == "test/child@1.0.0 is not built yet: only an installed connector provides it." + assert str(caught.value) == ( + "test/child@1.0.0 is not built yet: only an installed connector provides it. Read its card with " + f"get_card('test/child'), or build it with timenet.build('test/child', local_registry='{tmp_path / 'store'}')." + ) assert client.get_card("test/child").dataset_id == "test/child" assert not (tmp_path / "store").exists() @@ -551,8 +573,8 @@ def test_get_manifest_names_every_source_when_none_provides_the_release(tmp_path names = ", ".join(str(source) for source in sources) # The hint is the hint of open(). assert str(caught.value) == ( - f"x/y is in none of the sources: {names}. " - "Add a registry that holds it to the sources, or add InstalledConnectors() to build it." + f"x/y is in none of the sources: {names}. Add a registry that holds it to the sources, or install a " + "connector package that builds it, such as timenet-connectors." ) @@ -592,7 +614,10 @@ def test_download_of_a_release_that_only_a_connector_provides_builds_nothing(tmp with pytest.raises(TimeNetNotBuiltError) as caught: TimeNet(sources=[store, InstalledConnectors()]).download("timenet/hello-world") - assert str(caught.value) == "timenet/hello-world@1.0.0 is not built yet: only an installed connector provides it." + assert str(caught.value) == ( + "timenet/hello-world@1.0.0 is not built yet: only an installed connector provides it. " + f"Build it with timenet.build('timenet/hello-world', local_registry='{store.root}')." + ) assert calls == [] assert not store.root.exists() @@ -605,13 +630,50 @@ def test_download_of_latest_fails_when_a_connector_declares_a_newer_version(regi client = TimeNet(sources=[store, str(registry_root), InstalledConnectors()]) # "Latest" is the version of the connector, which no registry holds. - with pytest.raises(TimeNetNotBuiltError, match=r"^timenet/hello-world@2\.0\.0 is not built yet: "): + with pytest.raises(TimeNetNotBuiltError, match=r"^timenet/hello-world@2\.0\.0 is not built yet: ") as caught: client.download("timenet/hello-world@latest") assert calls == [] + # The hint names both ways out, and the registry still provides its exact version. + assert caught.value.hint == ( + f"Build it with timenet.build('timenet/hello-world', local_registry='{store.root}'), or download the version " + "that a registry holds with download('timenet/hello-world@1.0.0')." + ) # Without a version, the download takes the registry, the first source that serves the dataset. assert client.download("timenet/hello-world") == store.root / "timenet/hello-world/1.0.0" +def test_download_of_a_release_whose_payload_source_no_registry_provides_names_only_a_registry(tmp_path): + origin, reference = _child_of_the_demo(tmp_path / "origin") + shutil.copytree(origin.root / "test/child", tmp_path / "first/test/child") + store = LocalRegistry(tmp_path / "store") + + with pytest.raises(TimeNetMissingParentError) as caught: + TimeNet(sources=[store, tmp_path / "first"]).download("test/child") + + assert caught.value.missing == reference + # A download never builds, so the hint names only a registry. + assert caught.value.hint == "Add a registry that holds it to the sources." + + +def test_the_latest_version_offline_names_the_version_in_the_local_registry(tmp_path): + store = LocalRegistry(tmp_path / "store") + store.put_dataset(make_dataset()) + + def offline(request): + raise httpx.ConnectError("offline", request=request) + + remote = RemoteRegistry("http://api.local", transport=httpx.MockTransport(offline)) + + with pytest.raises(TimeNetRegistryError) as caught: + TimeNet(sources=[store, remote]).open("timenet/hello-world@latest") + + # "Latest" asks every source, and an exact version reads the store first. + assert caught.value.hint == ( + "Ask for timenet/hello-world@1.0.0 to read the copy in the local registry, or leave the unreachable " + "registry out of the sources." + ) + + def test_download_of_a_release_that_no_source_provides_names_each_source(tmp_path): sources = [LocalRegistry(tmp_path / "store"), LocalRegistry(tmp_path / "empty"), InstalledConnectors()] @@ -626,11 +688,16 @@ def test_download_of_a_release_that_no_source_provides_names_each_source(tmp_pat def test_download_of_a_version_that_the_connector_does_not_build_names_the_built_version(tmp_path, monkeypatch): calls = [] install_backends(monkeypatch, _Builder(calls)) - client = TimeNet(sources=[LocalRegistry(tmp_path / "store"), InstalledConnectors()]) + store = LocalRegistry(tmp_path / "store") + client = TimeNet(sources=[store, InstalledConnectors()]) - with pytest.raises(TimeNetVersionUnavailableError, match=r"builds version 1\.0\.0, not the requested 9\.9\.9"): + with pytest.raises( + TimeNetVersionUnavailableError, match=r"builds version 1\.0\.0, not the requested 9\.9\.9" + ) as caught: client.download("timenet/hello-world@9.9.9") assert calls == [] + # A download never builds, so the version of the connector needs a build first. + assert caught.value.hint == f"Build it with timenet.build('timenet/hello-world', local_registry='{store.root}')." def test_download_copies_each_release_from_the_first_registry_that_provides_it(tmp_path): @@ -668,9 +735,12 @@ def test_download_refuses_a_local_registry_copy_with_other_files(registry_root, (directory / "control.duckdb").write_bytes(b"cut short") before = (directory / "manifest.json").read_text() - with pytest.raises(TimeNetVersionConflictError, match="new version"): + with pytest.raises(TimeNetVersionConflictError) as caught: client.download("timenet/hello-world", force=case == "force") + # A consumer cannot make a new version, so the hint takes the copy of the registry. + assert caught.value.hint == f"Delete {directory} to take the copy of local registry {registry_root}." + assert (directory / "manifest.json").read_text() == before @@ -682,9 +752,7 @@ def test_download_names_the_local_registry_when_no_other_registry_provides_its_c client = TimeNet(sources=[store, InstalledConnectors()]) # The store holds the release, so the error is about its copy, not about a build. - message = ( - r"^No registry other than the local registry provides timenet/hello-world@1\.0\.0, so download\(force=True\)" - ) + message = r"^no registry other than the local registry provides timenet/hello-world@1\.0\.0 to copy again\. " with pytest.raises(TimeNetDatasetNotFoundError, match=message): client.download("timenet/hello-world", force=True) assert calls == [] @@ -709,15 +777,15 @@ def test_download_counts_a_registry_over_the_directory_of_the_local_registry_as_ with pytest.raises(TimeNetDatasetNotFoundError) as caught: client.download("timenet/hello-world", force=True) assert str(caught.value) == ( - "No registry other than the local registry provides timenet/hello-world@1.0.0, so download(force=True) " - "cannot copy it again." + "no registry other than the local registry provides timenet/hello-world@1.0.0 to copy again. " + "Add a registry that holds it to the sources." ) else: with pytest.raises(TimeNetFileError) as caught: client.download("timenet/hello-world") - assert ( - caught.value.hint - == "No registry other than the local registry provides it, so download() cannot replace it." + # Neither another registry nor a connector provides the release. + assert caught.value.hint == ( + "Add a registry that holds an intact copy to the sources, then call download('timenet/hello-world@1.0.0')." ) assert {path: path.read_bytes() for path in directory.rglob("*") if path.is_file()} == before @@ -729,7 +797,7 @@ def test_download_counts_a_symlink_to_the_local_registry_directory_as_the_local_ client = TimeNet(sources=[store, LocalRegistry(tmp_path / "link")]) # The second registry reads the directory of the store, so it cannot copy the release again. - with pytest.raises(TimeNetDatasetNotFoundError, match=r"^No registry other than the local registry provides "): + with pytest.raises(TimeNetDatasetNotFoundError, match=r"^no registry other than the local registry provides "): client.download("timenet/hello-world", force=True) @@ -1023,27 +1091,25 @@ def test_load_names_the_buildable_version_when_a_pin_cannot_be_built(registry_ro install_backends(monkeypatch, _Builder(calls)) client = TimeNet(sources=[LocalRegistry(registry_root), InstalledConnectors()]) - with pytest.raises(TimeNetVersionUnavailableError, match=r"builds version 1\.0\.0, not the requested 9\.9\.9"): + with pytest.raises( + TimeNetVersionUnavailableError, match=r"builds version 1\.0\.0, not the requested 9\.9\.9" + ) as caught: client.load("timenet/hello-world@9.9.9") assert calls == [] # the pin was rejected before the build ran + assert caught.value.hint == "Ask for version 1.0.0, or leave out the version." -@pytest.mark.parametrize("connectors", [True, False]) -def test_a_release_that_no_source_provides_names_how_to_get_it(tmp_path, connectors): - sources = [ - LocalRegistry(tmp_path / "reg"), - LocalRegistry(tmp_path / "empty"), - *([InstalledConnectors()] if connectors else []), - ] +def test_a_release_that_no_installed_connector_builds_names_only_a_registry(tmp_path, monkeypatch): + # An installed connector package that does not build x/y. The list has no InstalledConnectors(), so only the + # connector decides whether the hint names a build. + install_backends(monkeypatch, _Builder([])) + sources = [LocalRegistry(tmp_path / "reg"), LocalRegistry(tmp_path / "empty")] with pytest.raises(TimeNetDatasetNotFoundError) as caught: TimeNet(sources=sources).open("x/y") - message = f"x/y is in none of the sources: {', '.join(str(entry) for entry in sources)}." - # The hint names InstalledConnectors() only if the list does not have it. - hint = " Add a registry that holds it to the sources" - hint += "." if connectors else ", or add InstalledConnectors() to build it." - assert str(caught.value) == message + hint + names = ", ".join(str(entry) for entry in sources) + assert str(caught.value) == f"x/y is in none of the sources: {names}. Add a registry that holds it to the sources." def test_load_accepts_the_latest_sentinel_after_a_build(tmp_path, monkeypatch): @@ -1072,6 +1138,16 @@ def test_a_list_without_installed_connectors_does_not_build_a_missing_version(re assert calls == [] # the list skips the connector entirely +def test_a_missing_latest_release_names_the_build_of_its_installed_connector(tmp_path, monkeypatch): + install_backends(monkeypatch, _Builder([])) + client = TimeNet(sources=[LocalRegistry(tmp_path / "store")]) + + with pytest.raises(TimeNetDatasetNotFoundError) as caught: + client.open("timenet/hello-world@latest") + + assert caught.value.hint == "Add a registry that holds it to the sources, or add InstalledConnectors() to build it." + + def _card(dataset_id, access, access_url=None): return DatasetMetadata( dataset_id=dataset_id, @@ -1094,36 +1170,31 @@ def get_manifest(self, dataset_id, version=None): return _Hosted("http://api.local", transport=httpx.MockTransport(lambda request: httpx.Response(404))) -def test_load_rejects_a_credentialed_dataset_from_a_hosted_registry(tmp_path): - hosted = _hosted(tmp_path, _card("org/gated", Access.CREDENTIALED, "https://physionet.example/dua")) - store = LocalRegistry(tmp_path / "store") - client = TimeNet(sources=[store, hosted]) - message = ( - "org/gated@1.0.0 is credentialed. Get access at https://physionet.example/dua for org/gated@1.0.0. " - "No source serves its files. Add InstalledConnectors() to the sources, or build it locally with " - "`timenet-build build org/gated`." - ) - with pytest.raises(TimeNetAccessError) as caught: - client.load("org/gated") - assert str(caught.value) == message - assert not store.root.exists() - - -def test_the_access_hint_says_that_no_installed_connector_builds_the_release(tmp_path): +@pytest.mark.parametrize( + ("version", "built"), [(None, ""), ("2.0.0", "The installed connector builds version 2.0.0. ")] +) +def test_the_access_hint_names_the_package_that_builds_the_release(tmp_path, monkeypatch, version, built): + # No connector package, or one whose connector builds another version. + if version is not None: + monkeypatch.setattr(_Builder, "IDS", ("org/gated",)) + monkeypatch.setattr(_Builder, "VERSION", version) + install_backends(monkeypatch, _Builder([])) card = _card("org/gated", Access.CREDENTIALED, "https://physionet.example/dua") - client = TimeNet(sources=[LocalRegistry(tmp_path / "store"), _hosted(tmp_path, card), InstalledConnectors()]) + store = LocalRegistry(tmp_path / "store") + client = TimeNet(sources=[store, _hosted(tmp_path, card), InstalledConnectors()]) with pytest.raises(TimeNetAccessError) as caught: - client.open("org/gated") + client.open("org/gated@1.0.0") - assert str(caught.value) == ( - "org/gated@1.0.0 is credentialed. Get access at https://physionet.example/dua for org/gated@1.0.0. " - "No source serves its files, and no installed connector builds this release. " - "Install its connector package and build it locally with `timenet-build build org/gated`." + assert caught.value.hint == ( + f"{built}Install the connector package that builds org/gated@1.0.0, then build it locally with " + f"timenet.build('org/gated', local_registry='{store.root}')." ) -def test_the_hint_for_a_hosted_child_names_the_access_page_of_each_ancestor(tmp_path): +def test_the_hint_for_a_hosted_child_names_the_access_page_of_each_ancestor(tmp_path, monkeypatch): + monkeypatch.setattr(_Builder, "IDS", ("org/child",)) + install_backends(monkeypatch, _Builder([])) # The card of the child stays open and has no access page. Its parents set the access. parents = ( _card("org/a", Access.CREDENTIALED, "https://a.example"), @@ -1142,6 +1213,6 @@ def test_the_hint_for_a_hosted_child_names_the_access_page_of_each_ancestor(tmp_ assert str(caught.value) == ( "org/child@1.0.0 is restricted. " "Get access at https://a.example for org/a@1.0.0 and at https://b.example for org/b@1.0.0. " - "No source serves its files. Add InstalledConnectors() to the sources, or build it locally with " - "`timenet-build build org/child`." + # A client without a store cannot build, so the hint names the store too. + "Add LocalRegistry() and InstalledConnectors() to the sources to build it locally." ) diff --git a/packages/timenet/tests/test_config.py b/packages/timenet/tests/test_config.py index fdb79c966..b98c977f7 100644 --- a/packages/timenet/tests/test_config.py +++ b/packages/timenet/tests/test_config.py @@ -73,22 +73,13 @@ def test_an_empty_or_blank_setting_raises_instead_of_naming_a_directory(monkeypa settings() -@pytest.mark.parametrize( - ("name", "message"), - [ - ( - "remote_registry", - "TIMENET_REMOTE_REGISTRY must not be empty (also as --remote-registry); give it a value, " - "or leave it unset for timenet://.", - ), - ("cache", "TIMENET_CACHE must not be empty; give it a value, or leave it unset for ~/.cache/timenet."), - ], -) -def test_an_empty_argument_raises_with_the_text_of_its_variable_and_its_flag(name, message): - # TIMENET_CACHE has no flag, so its message names only the variable. +def test_an_empty_argument_raises_with_the_text_of_its_variable(): + # The CLIs check their flags first, so the message names only the variable. with pytest.raises(TimeFValidationError) as error: - settings(**{name: ""}) - assert str(error.value) == message + settings(remote_registry="") + assert str(error.value) == ( + "TIMENET_REMOTE_REGISTRY must not be empty; give it a value, or leave it unset for timenet://." + ) def test_an_argument_replaces_an_invalid_variable(monkeypatch, tmp_path): diff --git a/packages/timenet/tests/test_errors.py b/packages/timenet/tests/test_errors.py index 6a4883171..0af6f601c 100644 --- a/packages/timenet/tests/test_errors.py +++ b/packages/timenet/tests/test_errors.py @@ -1,8 +1,10 @@ +from pathlib import Path import pickle import pytest from timenet.errors import ( + ReleaseDifference, TimeFFormatError, TimeFNotFoundError, TimeFReaderError, @@ -13,6 +15,7 @@ TimeNetDownloadError, TimeNetError, TimeNetFileError, + TimeNetManifestError, TimeNetMissingParentError, TimeNetNotBuiltError, TimeNetRegistryError, @@ -127,33 +130,44 @@ def test_catchable_as_base(): (), ), ( - TimeNetVersionConflictError(_A, "registry here", "registry there"), + TimeNetVersionConflictError(_A, "registry here", "/here", "registry there", ReleaseDifference.FILES), { "release": _A, "registry": "registry here", + "registry_location": "/here", "source": "registry there", "needed_by": None, - "difference": "files", + "difference": ReleaseDifference.FILES, }, - "registry here already holds org/a@1.0.0 with other files than registry there. " - "A published version cannot change, so a changed release needs a new version.", + "registry here already holds org/a@1.0.0 with other files than registry there.", (), ), ( - TimeNetVersionConflictError(_A, "registry here", "registry there", _B, "card"), - {"release": _A, "needed_by": _B, "difference": "card"}, + TimeNetVersionConflictError(_A, "registry here", "/here", "registry there", ReleaseDifference.CARD, _B), + {"release": _A, "needed_by": _B, "difference": ReleaseDifference.CARD}, "registry here already holds org/a@1.0.0, a payload source of org/b@2.0.0, with another card than " - "registry there. A published version cannot change, so a changed release needs a new version.", + "registry there.", (), ), ( TimeNetVersionUnavailableError(_B, Version(1, 0, 0)), {"release": _B, "declared": Version(1, 0, 0)}, - "connector for 'org/b' builds version 1.0.0, not the requested 2.0.0. " - "Ask for version 1.0.0, or leave out the version.", + "connector for 'org/b' builds version 1.0.0, not the requested 2.0.0.", # The client reports a missing version, and a build reports invalid input. (TimeNetDatasetNotFoundError, TimeFValidationError), ), + ( + TimeNetManifestError( + "/store/org/a/1.0.0/manifest.json", "Invalid JSON", Path("/store/org/a/1.0.0/manifest.json") + ), + { + "where": "/store/org/a/1.0.0/manifest.json", + "detail": "Invalid JSON", + "path": Path("/store/org/a/1.0.0/manifest.json"), + }, + "/store/org/a/1.0.0/manifest.json is not a valid manifest: Invalid JSON", + (TimeFFormatError,), + ), ], ) def test_an_error_with_facts_ends_with_the_hint_of_its_front_end(error, facts, message, caught_as): @@ -164,8 +178,8 @@ def test_an_error_with_facts_ends_with_the_hint_of_its_front_end(error, facts, m assert {name: getattr(restored, name) for name in facts} == facts # A front end can catch the error as each of these classes. assert all(isinstance(restored, kind) for kind in caught_as) - # The message ends with a period, and the hint follows as its own sentence. - assert str(restored) == f"{message} Do this instead." + # The hint follows the message as a sentence of its own. + assert str(restored) == f"{message.removesuffix('.')}. Do this instead." def test_a_not_built_error_keeps_its_release_and_card_through_pickle(): diff --git a/packages/timenet/tests/test_hints.py b/packages/timenet/tests/test_hints.py new file mode 100644 index 000000000..e06906fb4 --- /dev/null +++ b/packages/timenet/tests/test_hints.py @@ -0,0 +1,104 @@ +from _origin_record import origin_file +import pytest + +from timenet import _hints as hints_module +from timenet._hints import CliWording, Facts, RequestedRelease, SdkWording, hint +from timenet.errors import ( + TimeFFormatError, + TimeFValidationError, + TimeNetDatasetNotFoundError, + TimeNetInvalidCardError, + TimeNetNoCatalogError, + TimeNetOriginError, + TimeNetRegistryError, +) +from timenet.registry import LocalRegistry +from timenet.testing import install_backends, make_dataset + + +def test_a_failed_hint_lookup_gives_no_hint_and_a_bug_in_it_surfaces(monkeypatch): + error, facts = ( + TimeNetDatasetNotFoundError("x/y is in none of the sources."), + Facts([], None, wanted=RequestedRelease("x/y")), + ) + + lookups = [ + TimeNetRegistryError("the registry is offline"), + TimeFFormatError("the manifest is not valid"), + TimeNetInvalidCardError("the card cannot be read"), + ] + for failure in lookups: + monkeypatch.setattr(hints_module, "_remedy", lambda error, facts, wording, failure=failure: _raise(failure)) + assert hint(error, facts, SdkWording()) is None + + # Any other error of the lookup is a bug, also a TimeNet error. + for bug in [AttributeError("a typo in the lookup"), TimeFValidationError("a manifest at an odd depth")]: + monkeypatch.setattr(hints_module, "_remedy", lambda error, facts, wording, bug=bug: _raise(bug)) + with pytest.raises(type(bug), match=str(bug)): + hint(error, facts, SdkWording()) + + +def _raise(error): + raise error + + +def test_a_subclass_of_an_error_without_its_own_remedy_gets_the_remedy_of_its_base_class(tmp_path): + class _SubclassError(TimeNetDatasetNotFoundError): + pass + + store = LocalRegistry(tmp_path / "store") + facts = Facts([store], store, wanted=RequestedRelease("x/y")) + + assert hint(_SubclassError("x/y is in none of the sources."), facts, SdkWording()) == ( + "Add a registry that holds it to the sources, or install a connector package that builds it, such as " + "timenet-connectors." + ) + + +def test_a_damaged_provenance_record_names_the_forced_download_and_build_of_the_sdk(tmp_path): + store = LocalRegistry(tmp_path / "store") + release = store.put_dataset(make_dataset()) + origin_file(store, release).write_text("{") + with pytest.raises(TimeNetOriginError) as caught: + store.read_origin(release) + + wording = hint(caught.value, Facts([store], store), SdkWording()) + + assert wording == ( + f"Get the release again with download('{release}', force=True) if it came from a registry, or with " + f"timenet.build('{release}', local_registry={str(store.root)!r}, force=True) if it was built. Both replace " + "the release and its record." + ) + + +def test_sources_without_a_catalog_name_the_read_of_an_exact_release_in_the_cli(tmp_path): + store = LocalRegistry(tmp_path / "store") + wording = CliWording(remote_registry="s3://bucket/data") + + assert hint(TimeNetNoCatalogError("none of the sources has a catalog"), Facts([store], store), wording) == ( + "Read an exact release with `timenet info @ --remote-registry s3://bucket/data`." + ) + + +class _Backend: + """A builder backend that holds the card of the demo dataset, at version 1.0.0.""" + + def dataset_ids(self): + return [make_dataset().metadata.dataset_id] + + def card(self, dataset_id): + return make_dataset().metadata + + def build(self, release, store, *, force, pinned_by, options): + raise AssertionError(f"a hint never builds {release}") + + +def test_a_version_in_another_spelling_names_the_build_of_its_connector(tmp_path, monkeypatch): + install_backends(monkeypatch, _Backend()) + store = LocalRegistry(tmp_path / "store") + # 01.0.0 is the version 1.0.0 of the card, written in another way. + facts = Facts([store], store, wanted=RequestedRelease("timenet/hello-world", "01.0.0")) + error = TimeNetDatasetNotFoundError("timenet/hello-world@01.0.0 is in none of the sources.") + assert hint(error, facts, SdkWording()) == ( + "Add a registry that holds it to the sources, or add InstalledConnectors() to build it." + ) diff --git a/packages/timenet/tests/test_producer.py b/packages/timenet/tests/test_producer.py index c79a3e964..3daccba02 100644 --- a/packages/timenet/tests/test_producer.py +++ b/packages/timenet/tests/test_producer.py @@ -87,8 +87,9 @@ def test_build_with_local_registry_and_sources_takes_both_only_from_them(tmp_pat monkeypatch.setenv("TIMENET_REMOTE_REGISTRY", str(origin.root)) store = tmp_path / "store" # The remote registry of the environment holds the parent, but the list of sources is empty. - with pytest.raises(TimeNetMissingParentError): + with pytest.raises(TimeNetMissingParentError) as caught: timenet.build(CHILD.dataset_id, local_registry=store, sources=[]) + assert caught.value.hint == "Add a registry that holds it to the sources, or add InstalledConnectors() to build it." assert not env_store.exists() assert cards == [] @@ -142,6 +143,12 @@ def test_timenet_exports_the_client_and_its_sources(): ) +def test_build_of_a_dataset_without_a_connector_package_names_one(tmp_path): + with pytest.raises(TimeNetDatasetNotFoundError) as caught: + timenet.build("x/y", local_registry=tmp_path / "store") + assert caught.value.hint == "Install a connector package that builds it, such as timenet-connectors." + + @pytest.mark.parametrize("kind", ["remote", "url", "read-only"]) def test_build_rejects_a_local_registry_that_it_cannot_write(tmp_path, cards, kind): target = { @@ -251,6 +258,24 @@ def test_publish_of_a_release_that_the_source_lacks_names_the_source(tmp_path): assert not (tmp_path / "dest").exists() +def test_the_hint_of_a_publish_asks_no_registry_that_the_call_does_not_open(tmp_path, monkeypatch): + requests = [] + + def send(self, request, **kwargs): + requests.append(request.url) + raise httpx.ConnectError("offline", request=request) + + monkeypatch.setattr(httpx.Client, "send", send) + monkeypatch.setenv("TIMENET_LOCAL_REGISTRY", str(tmp_path / "local")) + monkeypatch.setenv("TIMENET_REMOTE_REGISTRY", "https://registry.example") + + with pytest.raises(TimeNetDatasetNotFoundError) as caught: + timenet.publish("x/y@1.0.0", dest=tmp_path / "dest") + + assert requests == [] + assert caught.value.hint == "Copy it into the local registry with timenet.TimeNet().download('x/y@1.0.0')." + + def test_publish_of_a_listed_release_that_the_source_does_not_serve_raises_the_access_error(tmp_path): card = _card( DatasetRef.parse("org/gated@1.0.0"), access=Access.CREDENTIALED, access_url="https://physionet.example/dua" @@ -263,9 +288,7 @@ def get_manifest(self, dataset_id, version=None): hosted = _Hosted("http://api.local", transport=httpx.MockTransport(lambda request: httpx.Response(404))) with pytest.raises(TimeNetAccessError) as caught: timenet.publish("org/gated", dest=tmp_path / "dest", source=hosted) - assert str(caught.value) == ( - "org/gated@1.0.0 is credentialed. Get access at https://physionet.example/dua for org/gated@1.0.0." - ) + assert caught.value.dataset == DatasetRef.parse("org/gated@1.0.0") assert not (tmp_path / "dest").exists()