feat(cli)!: take --local-registry, --remote-registry and --auto-build

packages/timenet/src/timenet/cli/app.py

@@ -10,6 +10,7 @@
 import typer
 
 from timenet.cache import cache_size, clear_cache, clear_releases, human_bytes, stored_releases
+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
@@ -24,22 +25,6 @@
 app = typer.Typer(help="Browse a TimeNet registry and fetch datasets.", no_args_is_help=True)
 console = Console()
 
-_registry_option = typer.Option(
-    None,
-    "--registry",
-    "-r",
-    help="Registry URL or path that this command reads (else $TIMENET_REMOTE_REGISTRY, then timenet://).",
-)
-# TODO(adrian): Remove temporary glue which maps --registry onto the list after the local registry, until the CLI
-# takes the flags of the defaults.
-_download_registry_option = typer.Option(
-    None,
-    "--registry",
-    "-r",
-    help="Registry URL or path to put after the local registry in the list of sources. The release resolves "
-    "through the whole list.",
-)
-
 T = TypeVar("T")
 
 
@@ -71,18 +56,21 @@ def _enum_list(values: list[str], factory: Callable[[str], T], flag: str, choice
     return parsed or None
 
 
-def _client(registry: str | None) -> TimeNet:
-    """Create a client with the default sources and an optional registry.
+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.
+
+    Pass ``None`` to use the matching ``TIMENET_*`` environment variable or its default.
 
     Args:
-        registry: The registry URL or path to read after the local registry, or ``None`` for the default.
+        local_registry: The local registry path.
+        remote_registry: The remote registry URL or path.
+        auto_build: Whether to build missing datasets with installed connectors.
 
     Returns:
-        A client with the configured sources.
+        The configured client.
     """
-    # TODO(adrian): Remove temporary glue which maps --registry onto default_sources() until the
-    # CLI takes the flags of the defaults.
-    return TimeNet(sources=default_sources(remote_registry=registry))
+    sources = default_sources(local_registry=local_registry, remote_registry=remote_registry, auto_build=auto_build)
+    return TimeNet(sources=sources)
 
 
 def _metadata_table(rows: list[DatasetMetadata], title: str) -> Table:
@@ -111,9 +99,13 @@ def _metadata_table(rows: list[DatasetMetadata], title: str) -> Table:
 
 
 @app.command("list")
-def list_datasets(registry: str | None = _registry_option) -> None:
-    """Display metadata for every dataset in the selected registry."""
-    rows = _client(registry).list()
+def list_datasets(
+    local_registry: str | None = LOCAL_REGISTRY,
+    remote_registry: str | None = REMOTE_REGISTRY,
+    auto_build: bool | None = AUTO_BUILD,
+) -> None:
+    """List the datasets in the registry."""
+    rows = _client(local_registry, remote_registry, auto_build).list()
     if not rows:
         typer.echo("No datasets in the registry.")
         return
@@ -122,7 +114,6 @@ def list_datasets(registry: str | None = _registry_option) -> None:
 
 @app.command()
 def search(  # ruff: ignore[too-many-arguments, too-many-positional-arguments]
-    registry: str | None = _registry_option,
     query: list[str] = typer.Option([], "--query", "-q", help="Free-text terms."),
     domain: list[str] = typer.Option([], "--domain", help="Domain(s)."),
     task: list[str] = typer.Option([], "--task", help="Task type(s)."),
@@ -131,12 +122,15 @@ def search(  # ruff: ignore[too-many-arguments, too-many-positional-arguments]
     dataset_id: list[str] = typer.Option([], "--id", help="Dataset id(s)."),
     tag: list[str] = typer.Option([], "--tag", help="Tag(s)."),
     limit: int = typer.Option(100, "--limit", min=0, help="Maximum results."),
+    local_registry: str | None = LOCAL_REGISTRY,
+    remote_registry: str | None = REMOTE_REGISTRY,
+    auto_build: bool | None = AUTO_BUILD,
 ) -> None:
-    """Find datasets that match the filters.
+    """Search for datasets that match the filters.
 
-    Repeat a filter flag to pass several values.
+    Repeat a filter flag to supply multiple values.
     """
-    results = _client(registry).search(
+    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]),
         task=_enum_list(task, lambda value: TASKS[TaskType(value)], "--task", [t.value for t in TaskType]),
@@ -153,9 +147,14 @@ def search(  # ruff: ignore[too-many-arguments, too-many-positional-arguments]
 
 
 @app.command()
-def info(dataset_id: str, registry: str | None = _registry_option) -> None:
-    """Display a dataset's metadata, schema, and counts."""
-    manifest = _client(registry).get(dataset_id)
+def info(
+    dataset_id: str,
+    local_registry: str | None = LOCAL_REGISTRY,
+    remote_registry: str | None = REMOTE_REGISTRY,
+    auto_build: bool | None = AUTO_BUILD,
+) -> None:
+    """Show a dataset's metadata, schema, and counts."""
+    manifest = _client(local_registry, remote_registry, auto_build).get(dataset_id)
     counts = manifest.counts
     table = Table(title=manifest.dataset_id, title_justify="left", title_style="bold", show_header=False)
     table.add_column("Field", style="cyan")
@@ -197,24 +196,25 @@ def info(dataset_id: str, registry: str | None = _registry_option) -> None:
 @app.command()
 def download(
     dataset_id: str,
-    registry: str | None = _download_registry_option,
+    local_registry: str | None = LOCAL_REGISTRY,
+    remote_registry: str | None = REMOTE_REGISTRY,
+    auto_build: bool | None = AUTO_BUILD,
     force: bool = typer.Option(
         False, "--force", help="Copy the release again, even if the local registry holds a complete copy."
     ),
 ) -> None:
-    """Copy a release and its payload sources into the local registry.
+    """Copy a release and its data files into the local registry.
 
-    Print the release directory to standard output.
+    The command prints the release directory to stdout.
     """
-    sources = default_sources(remote_registry=registry)
     started: list[StepStarted] = []
     with progress_renderer() as show:
-        target = TimeNet(sources=sources).download(
-            dataset_id, force=force, progress=partial(_keep_steps, started, show)
-        )
+        client = _client(local_registry, remote_registry, auto_build)
+        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.
-        ui.success(f"Nothing to do: '{split_ref(dataset_id)[0]}' {target.name} is already in {sources[0]}")
+        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}")
     typer.echo(str(target))
 
 
@@ -244,9 +244,9 @@ def _keep_steps(started: list[StepStarted], show: ProgressCallback | None, event
 
 
 @local_app.command("info")
-def local_info() -> None:
-    """Show the path and size of the cache and the local registry."""
-    store, cache_dir = default_local_registry(), settings().cache_dir
+def local_info(local_registry: str | None = LOCAL_REGISTRY) -> None:
+    """Show cache and local registry paths and sizes."""
+    store, cache_dir = default_local_registry(local_registry=local_registry), settings().cache_dir
     releases = stored_releases(store)
     size = human_bytes(sum(item.size_bytes for item in releases))
     # Plain lines, so that a long path does not wrap.
@@ -255,9 +255,9 @@ def local_info() -> None:
 
 
 @local_app.command("clear")
-def local_clear(yes: bool = _YES) -> None:
+def local_clear(yes: bool = _YES, local_registry: str | None = LOCAL_REGISTRY) -> None:
     """Delete the cache and every release in the local registry."""
-    store, cache_dir = default_local_registry(), settings().cache_dir
+    store, cache_dir = default_local_registry(local_registry=local_registry), settings().cache_dir
     if not yes:
         typer.confirm(f"Delete the cache in {cache_dir} and every release in {store.root}?", abort=True)
     # The cache clear checks the folders before it deletes anything, so it runs first.

packages/timenet/src/timenet/cli/options.py

@@ -0,0 +1,29 @@
+"""CLI flags for registry locations and automatic builds.
+
+Each flag overrides the matching ``TIMENET_*`` environment variable.
+Omitted flags have the value ``None``.
+"""
+
+import typer
+
+
+LOCAL_REGISTRY = typer.Option(
+    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).",
+)
+REMOTE_REGISTRY = typer.Option(
+    None,
+    "--remote-registry",
+    metavar="URL|PATH",
+    help="A remote registry, as a URL or a local path: the second source that a lookup checks, if the local "
+    "registry does not have the dataset (default: $TIMENET_REMOTE_REGISTRY, else timenet://).",
+)
+AUTO_BUILD = typer.Option(
+    None,
+    "--auto-build/--no-auto-build",
+    help="Whether installed connectors build a dataset that neither the local nor the remote registry has "
+    "(default: $TIMENET_AUTO_BUILD, else true).",
+)

packages/timenet/src/timenet/config.py

@@ -32,9 +32,12 @@
 )
 """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_*`` variables and an optional ``.env`` file."""
+    """TimeNet settings from arguments, ``TIMENET_*`` environment variables, and an optional ``.env`` file."""
 
     model_config = SettingsConfigDict(env_prefix="TIMENET_", env_file=".env", extra="ignore")
 
@@ -58,24 +61,25 @@ class TimeNetSettings(BaseSettings):
     @field_validator("local_registry", "remote_registry", "cache", mode="before")
     @classmethod
     def _not_empty(cls, value: Any, info: ValidationInfo) -> Any:
-        """Convert paths to strings and reject blank settings.
+        """Convert paths to strings and reject blank strings.
 
         Args:
-            value: The value to validate.
-            info: Validation metadata that includes the field name.
+            value: A setting value.
+            info: Details of the field to validate.
 
         Returns:
-            The value with any path converted to a string.
+            The value, with any path converted to a string.
 
         Raises:
-            TimeFValidationError: If the value is empty or contains only whitespace.
+            TimeFValidationError: If the string is empty or contains only whitespace.
         """
         if isinstance(value, os.PathLike):
             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; give it a value, "
+                f"TIMENET_{name.upper()} must not be empty{flag}; give it a value, "
                 f"or leave it unset for {cls.model_fields[name].default}."
             )
         return value

packages/timenet/tests/test_cli.py

@@ -132,30 +132,24 @@ def test_local_clear_deletes_no_release_when_the_cache_check_fails(state, monkey
     assert inside.exists("timenet/hello-world", "1.0.0")
 
 
-def test_list(registry_root):
-    result = runner.invoke(app, ["list", "--registry", str(registry_root)])
+def test_the_local_commands_take_the_local_registry_flag(tmp_path):
+    # The variable of the autouse fixture names an empty store, so the release comes from the flag.
+    store = tmp_path / "store"
+    LocalRegistry(store).put_dataset(make_dataset())
+    result = runner.invoke(app, ["local", "info", "--local-registry", str(store)])
     assert result.exit_code == 0, result.output
-    assert "timenet/hello-world" in click.unstyle(result.output)
-
-
-def test_search_by_domain(registry_root):
-    result = runner.invoke(app, ["search", "--registry", str(registry_root), "--domain", "general"])
+    assert f"Local registry {store}: 1 release" in click.unstyle(result.stdout)
+    result = runner.invoke(app, ["local", "clear", "--yes", "--local-registry", str(store)])
     assert result.exit_code == 0, result.output
-    assert "timenet/hello-world" in click.unstyle(result.output)
+    assert stored_releases(LocalRegistry(store)) == []
 
 
 def test_search_no_match(registry_root):
-    result = runner.invoke(app, ["search", "--registry", str(registry_root), "--domain", "cardiology"])
+    result = runner.invoke(app, ["search", "--remote-registry", str(registry_root), "--domain", "cardiology"])
     assert result.exit_code == 0, result.output
     assert click.unstyle(result.output) == "No datasets match.\n"
 
 
-def test_info(registry_root):
-    result = runner.invoke(app, ["info", "timenet/hello-world", "--registry", str(registry_root)])
-    assert result.exit_code == 0, result.output
-    assert click.unstyle(result.output).startswith("timenet/hello-world")
-
-
 def _write(root, dataset):
     with TimeFWriter(root, dataset) as writer:
         writer.write()
@@ -190,10 +184,11 @@ def test_info_lists_the_access_page_of_each_release_that_sets_one(tmp_path):
             child.import_from(reader)
     _write(root, child)
 
-    result = runner.invoke(app, ["info", "test/child", "--registry", str(root)])
+    result = runner.invoke(app, ["info", "test/child", "--remote-registry", str(root)])
 
     assert result.exit_code == 0, result.output
-    cells = [[cell.strip() for cell in line.split("│")[1:-1]] for line in result.output.splitlines() if "│" in line]
+    lines = click.unstyle(result.output).splitlines()
+    cells = [[cell.strip() for cell in line.split("│")[1:-1]] for line in lines if "│" in line]
     start = cells.index(["access", "credentialed"])
     # The card of the child stays open. Its effective level comes from its parents, and each parent
     # names its own page.
@@ -208,7 +203,7 @@ def test_download_command_copies_into_the_store_and_then_has_nothing_to_do(regis
     # The last line names a long path, so the console must not wrap it.
     monkeypatch.setattr(_cli_module.ui, "rich", Console(stderr=True, width=1000))
     monkeypatch.setenv("TIMENET_LOCAL_REGISTRY", str(tmp_path / "store"))
-    command = ["download", "timenet/hello-world", "--registry", str(registry_root)]
+    command = ["download", "timenet/hello-world", "--remote-registry", str(registry_root)]
     target = tmp_path / "store" / "timenet/hello-world" / "1.0.0"
 
     copied = runner.invoke(app, command)
@@ -229,7 +224,7 @@ def test_download_command_copies_into_the_store_and_then_has_nothing_to_do(regis
 
 def test_download_command_with_force_copies_a_held_release_again(registry_root, tmp_path, monkeypatch):
     monkeypatch.setenv("TIMENET_LOCAL_REGISTRY", str(tmp_path / "store"))
-    command = ["download", "timenet/hello-world", "--registry", str(registry_root)]
+    command = ["download", "timenet/hello-world", "--remote-registry", str(registry_root)]
     assert runner.invoke(app, command).exit_code == 0
 
     forced = runner.invoke(app, [*command, "--force"])
@@ -239,8 +234,48 @@ def test_download_command_with_force_copies_a_held_release_again(registry_root,
     assert successes == ["✅ Copied 'timenet/hello-world' → 1.0.0"]
 
 
+def test_each_command_builds_the_list_from_its_flags(registry_root, tmp_path):
+    # The variables of the autouse fixture name an empty store and an empty registry, so each result
+    # comes from the flags.
+    store = tmp_path / "store"
+    flags = ["--local-registry", str(store), "--remote-registry", str(registry_root), "--no-auto-build"]
+    for command in (["list"], ["search", "--domain", "general"], ["info", "timenet/hello-world"]):
+        result = runner.invoke(app, [*command, *flags])
+        assert result.exit_code == 0, result.output
+        assert "timenet/hello-world" in result.stdout
+    result = runner.invoke(app, ["download", "timenet/hello-world", *flags])
+    assert result.exit_code == 0, result.output
+    assert result.stdout == f"{store / 'timenet/hello-world/1.0.0'}\n"
+
+
+@pytest.mark.parametrize(("flag", "variable"), [("--auto-build", "false"), ("--no-auto-build", "true")])
+def test_the_auto_build_flag_beats_its_variable(monkeypatch, flag, variable):
+    monkeypatch.setenv("TIMENET_AUTO_BUILD", variable)
+    result = runner.invoke(app, ["download", "x/y", flag])
+    # The error names each source of the list.
+    assert isinstance(result.exception, TimeNetDatasetNotFoundError)
+    assert ("installed connectors" in str(result.exception)) == (flag == "--auto-build")
+
+
+# The commands that read the list of sources share one path, so download stands for them.
+_EMPTY_FLAGS = [
+    ["download", "x/y", "--local-registry", ""],
+    ["local", "clear", "--yes", "--local-registry", ""],
+]
+
+
+@pytest.mark.parametrize("argv", _EMPTY_FLAGS, ids=lambda argv: " ".join(argv[:-1]))
+def test_an_empty_flag_is_an_error(tmp_path, monkeypatch, argv):
+    # An empty value must not mean the current directory: a build would land there, and
+    # `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)
+
+
 def test_search_rejects_unknown_filter_value(registry_root):
-    assert runner.invoke(app, ["search", "--registry", str(registry_root), "--domain", "bogus"]).exit_code == 2
+    assert runner.invoke(app, ["search", "--remote-registry", str(registry_root), "--domain", "bogus"]).exit_code == 2
     # The error message is a Rich-rendered panel, so assert it on the raising helper directly.
     with pytest.raises(typer.BadParameter, match="invalid --domain"):
         _enum_list(["bogus"], Domain, "--domain", [d.value for d in Domain])

packages/timenet/tests/test_client.py

@@ -445,8 +445,8 @@ def test_download_counts_a_registry_over_the_directory_of_the_store_as_the_store
     if case == "damaged":
         (directory / "control.duckdb").write_bytes(b"cut short")
     before = {path: path.read_bytes() for path in directory.rglob("*") if path.is_file()}
-    # As for `TIMENET_LOCAL_REGISTRY=./reg timenet download timenet/hello-world --registry ./reg`: the
-    # registry after the store reads the directory of the store, so it cannot replace its copy.
+    # As for `timenet download timenet/hello-world --local-registry ./reg --remote-registry ./reg`:
+    # the registry after the store reads the directory of the store, so it cannot replace its copy.
     monkeypatch.setenv("TIMENET_LOCAL_REGISTRY", str(root))
     client = TimeNet(sources=default_sources(remote_registry=root))