Skip to content

feat(hints)!: give every error one hint for the SDK and one for the CLI #698

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
karhu merged 1 commit into from
Nov 10, 2026
Merged

feat(hints)!: give every error one hint for the SDK and one for the CLI #698

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
166 changes: 61 additions & 105 deletions packages/timenet-connectors/src/timenet_connectors/builder/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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 <url>, 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:
Expand Down
18 changes: 7 additions & 11 deletions packages/timenet-connectors/src/timenet_connectors/builder/env.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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)]
Expand Down Expand Up @@ -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:
Expand Down
Loading
Loading