From 4fe3b809ab9905b05f043f78b94835d46d918b2a Mon Sep 17 00:00:00 2001 From: OmerBaddour Date: Sat, 1 Aug 2026 18:52:24 -0400 Subject: [PATCH 1/4] feat: move routing into a project __routing__.py with callable rules Routing and credentials want opposite things, so they now live apart. - ~/.clair/environments.yml stays YAML and holds credentials only. It is inert: no imports, no clair version coupling, no logic. - Routing moves into a project-level __routing__.py, keyed by environment name. That name is the join key between the two files. The file is checked in: routing is a team decision, not a secret. A routing rule is now a RoutingConfig subclass or any callable (database_name, schema_name, table_name) -> "database.schema.table". A callable rule reads the environment, so one committed rule gives each person a separate target: "dev": lambda database_name, schema_name, table_name: ( f"{database_name}_{os.environ['CLAIR_USER'].upper()}" f".{schema_name}.{table_name}" ) Python cannot statically validate a callable, so route() validates its output instead: 3 dot-separated parts, each a legal unquoted identifier within 255 characters. This closes a pre-existing gap where DatabaseOverrideRouting validated nothing at all. Adds `clair validate`: applies the rules to every Trouve and reports all problems plus collisions at once. It needs no Snowflake credentials, so CI runs it on every change. compile and run validate too (fail-fast via route()), and point at `clair validate` for the full list. Two guards against silent writes to production: - A leftover `routing:` block in environments.yml raises instead of being dropped by pydantic. - A __routing__.py that omits the active environment warns. An explicit `"prod": None` is a decision and stays quiet. Other fixes carried over from the closed #4 review: - Collision messages describe a callable by its source, not "callable". - The routing loader registers its module in sys.modules and caches by (path, mtime), matching discovery._load_config_file. A rule that reads a secret store runs one time, not once per load. - Callable parameters are database_name/schema_name/table_name per CLAUDE.md, since they are user-facing API. Docs (README, site-docs, specs) still reference the old layout and are the next step. Co-Authored-By: Claude Opus 5 --- src/clair/cli/main.py | 158 ++++++++++++-- src/clair/core/discovery.py | 6 +- src/clair/core/scaffold.py | 33 ++- src/clair/environments/environments.py | 50 ++--- src/clair/environments/project_routing.py | 161 +++++++++++++++ src/clair/environments/routing.py | 238 ++++++++++++++++++---- src/clair/exceptions.py | 35 +++- tests/conftest.py | 22 +- tests/unit/test_environments.py | 58 ++---- tests/unit/test_project_routing.py | 165 +++++++++++++++ tests/unit/test_routing.py | 106 ++++++++++ tests/unit/test_scaffold.py | 22 +- tests/unit/test_validate_cli.py | 162 +++++++++++++++ 13 files changed, 1057 insertions(+), 159 deletions(-) create mode 100644 src/clair/environments/project_routing.py create mode 100644 tests/unit/test_project_routing.py create mode 100644 tests/unit/test_validate_cli.py diff --git a/src/clair/cli/main.py b/src/clair/cli/main.py index 9d2b35f..7e52352 100644 --- a/src/clair/cli/main.py +++ b/src/clair/cli/main.py @@ -2,6 +2,7 @@ from __future__ import annotations +import os import re import shutil import sys @@ -31,8 +32,23 @@ from clair.docs.catalog import build_catalog from clair.docs.server import serve from clair.environments.environments import load_environment -from clair.environments.routing import DatabaseOverrideRouting, SchemaIsolationRouting -from clair.exceptions import ClairError, CompileError, EnvironmentsFileNotFoundError +from clair.environments.project_routing import ( + ROUTING_FILE_NAME, + ProjectRouting, + load_project_routing, +) +from clair.environments.routing import ( + collect_routing_problems, + describe_routing, + detect_routing_collisions, + route, +) +from clair.exceptions import ( + ClairError, + CompileError, + EnvironmentsFileNotFoundError, + InvalidRoutingConfigError, +) from clair.trouves.run_config import RunMode from clair.trouves.trouve import ExecutionType, TrouveType @@ -119,23 +135,42 @@ def init(project: str | None) -> None: click.echo("") +def _resolve_project_routing(project_root: Path, env_name: str) -> ProjectRouting: + """Load the project routing rule and warn about a missing entry. + + A routing file that does not name the active environment is almost always a + typo. Passthrough routing then writes to the production names, so clair + tells the user before any SQL runs. + """ + project_routing = load_project_routing(project_root, env_name) + + if project_routing.is_unnamed_environment: + click.echo( + click.style( + f"\nWarning: {ROUTING_FILE_NAME} does not name the environment " + f"'{env_name}'.", + fg="yellow", + bold=True, + ) + ) + click.echo( + f" Trouves write to their logical (production) names.\n" + f" The file names: {', '.join(project_routing.environment_names) or 'nothing'}\n" + ) + + return project_routing + + def _print_routing_collision_warnings(trouves: list, env_name: str, routing) -> None: """Print a prominent warning block for any routing collisions, before SQL runs.""" collisions = find_routing_collisions(trouves) if not collisions: return - policy_desc = "" - if routing is not None: - if isinstance(routing, DatabaseOverrideRouting): - policy_desc = f"database_override → {routing.database_name}" - elif isinstance(routing, SchemaIsolationRouting): - policy_desc = f"schema_isolation → {routing.database_name}.{routing.schema_name}" - n = len(collisions) header = f"{'collision' if n == 1 else f'{n} collisions'} detected" - if policy_desc: - header += f" (env: {env_name}, policy: {policy_desc})" + if routing is not None: + header += f" (env: {env_name}, rule: {describe_routing(routing)})" else: header += f" (env: {env_name})" @@ -147,8 +182,8 @@ def _print_routing_collision_warnings(trouves: list, env_name: str, routing) -> click.echo(f" ↳ {source}") click.echo( - "\n Fix: rename a colliding Trouve, adjust the routing policy in " - "environments.yml,\n or use --select to exclude one from this run.\n" + f"\n Fix: rename a colliding Trouve, adjust the rule in " + f"{ROUTING_FILE_NAME},\n or use --select to exclude one from this run.\n" ) @@ -261,14 +296,18 @@ def compile_cmd(select: tuple[str, ...], exclude: tuple[str, ...], project: str, run_mode_enum = RunMode(run_mode) run_id = uuid6.uuid7().hex - routing = None environment = None env_name = env or "dev" try: env_name, environment = load_environment(env) - routing = environment.routing except EnvironmentsFileNotFoundError: - logger.warning("compile.no_environments_file", detail="compiling without routing; run `clair init` to create environments.yml") + logger.warning("compile.no_environments_file", detail="compiling without an environment; run `clair init` to create environments.yml") + except ClairError as e: + logger.error("compile.error", error=str(e)) + sys.exit(1) + + try: + routing = _resolve_project_routing(project_root, env_name).rule except ClairError as e: logger.error("compile.error", error=str(e)) sys.exit(1) @@ -307,11 +346,84 @@ def _on_node_compiled(node_info: CompiledNodeInfo) -> None: write_compile_output(dag, selected, project_root, on_node_compiled=_on_node_compiled, run_mode=run_mode_enum, run_id=run_id) logger.info("compile.complete", run_id=run_id, artifacts_dir=str(artifacts_dir)) + except InvalidRoutingConfigError as e: + logger.error("compile.routing_error", error=str(e)) + click.echo("\n Run `clair validate` to see every routing problem.\n", err=True) + sys.exit(1) except ClairError as e: logger.error("compile.error", error=str(e)) sys.exit(1) +@cli.command() +@click.option( + "--project", + default=".", + help="Path to the Clair project root (defaults to current directory)", +) +@click.option( + "--env", + default=None, + help="Environment name to route for; matches a key in __routing__.py", +) +def validate(project: str, env: str | None) -> None: + """Apply the project routing rules to every Trouve. + + This command needs no Snowflake credentials, so CI runs it on every change. + """ + project_root = Path(project).resolve() + env_name = env or os.environ.get("CLAIR_ENV") or "dev" + + try: + project_routing = _resolve_project_routing(project_root, env_name) + # Discover with routing off. A bad rule then reports as a routing + # problem, and does not stop discovery at the first Trouve. + discovered = discover_project(project_root, routing=None) + except ClairError as e: + logger.error("validate.error", error=str(e)) + sys.exit(1) + + routing = project_routing.rule + routable = [ + trouve for trouve in discovered + if trouve.compiled and trouve.type != TrouveType.SOURCE + ] + + click.echo(f"\n environment: {env_name}") + click.echo(f" routing file: {project_routing.file_path or 'none'}") + click.echo(f" rule: {describe_routing(routing)}") + click.echo(f" Trouves to route: {len(routable)}\n") + + problems = collect_routing_problems(discovered, routing) + for logical_name, problem in problems: + click.echo(click.style(f" ✗ {logical_name}", fg="red", bold=True)) + click.echo(f" {problem}\n") + + collisions: list[tuple[str, list[str]]] = [] + if not problems: + logical_to_routed = { + trouve.compiled.logical_name: route( + trouve.compiled.logical_name, trouve.type, routing + ) + for trouve in routable + } + collisions = detect_routing_collisions(logical_to_routed) + for routed_target, logical_sources in collisions: + click.echo(click.style(f" ✗ {routed_target}", fg="red", bold=True)) + click.echo(" Two or more Trouves route to this one target:") + for source in logical_sources: + click.echo(f" ↳ {source}") + click.echo("") + + problem_count = len(problems) + len(collisions) + if problem_count: + label = "problem" if problem_count == 1 else "problems" + click.echo(click.style(f" {problem_count} {label} found.\n", fg="red", bold=True)) + sys.exit(1) + + click.echo(click.style(" ✓ Every routed name is valid. No collisions.\n", fg="green")) + + @cli.command() @click.option( "--select", @@ -447,8 +559,9 @@ def run(select: tuple[str, ...], exclude: tuple[str, ...], project: str, env: st "warehouse": environment.warehouse, "role": environment.role, } - discovered = discover_project(project_root, profile_defaults, routing=environment.routing, environment=environment, run_mode=run_mode_enum) - _print_routing_collision_warnings(discovered, env_name, environment.routing) + routing = _resolve_project_routing(project_root, env_name).rule + discovered = discover_project(project_root, profile_defaults, routing=routing, environment=environment, run_mode=run_mode_enum) + _print_routing_collision_warnings(discovered, env_name, routing) dag = build_dag(discovered) # Filter by selector @@ -502,6 +615,10 @@ def on_node_success(node_name: str) -> bool: finally: adapter.close() + except InvalidRoutingConfigError as e: + logger.error("run.routing_error", error=str(e)) + click.echo("\n Run `clair validate` to see every routing problem.\n", err=True) + sys.exit(1) except ClairError as e: logger.error("run.error", error=str(e)) sys.exit(1) @@ -543,14 +660,15 @@ def test( try: # Load environment - _, environment = load_environment(env) + env_name, environment = load_environment(env) # Discover and build DAG profile_defaults = { "warehouse": environment.warehouse, "role": environment.role, } - discovered = discover_project(project_root, profile_defaults, routing=environment.routing, environment=environment) + routing = _resolve_project_routing(project_root, env_name).rule + discovered = discover_project(project_root, profile_defaults, routing=routing, environment=environment) dag = build_dag(discovered) # Filter by selector -- include all nodes (even SOURCEs) so that diff --git a/src/clair/core/discovery.py b/src/clair/core/discovery.py index 9b404a8..cd2e7a9 100644 --- a/src/clair/core/discovery.py +++ b/src/clair/core/discovery.py @@ -17,7 +17,7 @@ if TYPE_CHECKING: from clair.environments.environments import Environment -from clair.environments.routing import RoutingConfig, detect_routing_collisions, route +from clair.environments.routing import RoutingRule, detect_routing_collisions, route from clair.trouves._refs import THIS_PLACEHOLDER, TROUVE_PLACEHOLDER_PREFIX from clair.trouves._refs import clear as clear_refs from clair.trouves.config import DatabaseDefaults, ResolvedConfig, SchemaDefaults @@ -145,7 +145,7 @@ def _detect_imports( def discover_project( project_root: Path, profile_defaults: dict[str, str | None] | None = None, - routing: RoutingConfig | None = None, + routing: RoutingRule | None = None, environment: Environment | None = None, run_mode: RunMode | None = None, ) -> list[Trouve]: @@ -157,7 +157,7 @@ def discover_project( Args: project_root: Absolute path to the project root directory. profile_defaults: Default warehouse/role from the active profile. - routing: Routing configuration for physical name overrides. + routing: Routing rule for physical name overrides, from __routing__.py. environment: Active environment. Exposed as ``clair.env`` so Trouve modules can read it during loading (e.g. for feature flags). run_mode: Requested run mode (FULL_REFRESH or INCREMENTAL). Exposed as diff --git a/src/clair/core/scaffold.py b/src/clair/core/scaffold.py index 311ea57..2dc6364 100644 --- a/src/clair/core/scaffold.py +++ b/src/clair/core/scaffold.py @@ -4,6 +4,8 @@ from pathlib import Path +from clair.environments.project_routing import ROUTING_FILE_NAME + # --------------------------------------------------------------------------- # File templates # --------------------------------------------------------------------------- @@ -16,9 +18,33 @@ ) ''' +_ROUTING_TEMPLATE = '''\ +"""Clair routing -- maps each environment to its physical write target. + +Each key matches a top-level key in ~/.clair/environments.yml. A rule accepts +(database_name, schema_name, table_name) and returns the physical name +"database_name.schema_name.table_name". SOURCE Trouves never route. + +Commit this file. It holds no credentials. +Run `clair validate` to apply these rules to every Trouve in the project. +""" + +import os + +routing = { + # Each person writes to a separate database. Set CLAIR_USER for each person. + "dev": lambda database_name, schema_name, table_name: ( + f"{database_name}_{os.environ['CLAIR_USER'].upper()}.{schema_name}.{table_name}" + ), + # Production writes to the logical names, so it needs no rule. + "prod": None, +} +''' + _ENVIRONMENTS_TEMPLATE = '''\ # Clair environments — connection settings per environment. -# Reference: https://github.com/your-org/clair +# Routing is not here. It lives in the project __routing__.py file. +# Reference: https://github.com/rivage-sh/clair dev: account: your-org-your-account # e.g. myorg-myaccount @@ -61,8 +87,8 @@ def scaffold_project( ) -> list[tuple[str, str]]: """Create a new Clair project at *project_dir*. - Generates an example source Trouve file and a global - ``~/.clair/environments.yml`` (if it does not already exist). + Writes an example source Trouve file, a project ``__routing__.py``, and a + global ``~/.clair/environments.yml`` if that file does not exist yet. Args: project_dir: Root directory for the new project. @@ -80,6 +106,7 @@ def scaffold_project( # All project files: (relative_path, template_content) project_files: list[tuple[str, str]] = [ (f"{source_database_name}/{source_schema_name}/{source_table_name}.py", _SOURCE_TROUVE_TEMPLATE), + (ROUTING_FILE_NAME, _ROUTING_TEMPLATE), ] results: list[tuple[str, str]] = [] diff --git a/src/clair/environments/environments.py b/src/clair/environments/environments.py index 3e79f0b..553c375 100644 --- a/src/clair/environments/environments.py +++ b/src/clair/environments/environments.py @@ -7,21 +7,23 @@ from typing import Any import yaml -from pydantic import BaseModel, ConfigDict, ValidationError +from pydantic import BaseModel, ConfigDict -from clair.environments.routing import Routing from clair.exceptions import ( EnvironmentNotFoundError, EnvironmentsFileNotFoundError, - InvalidRoutingConfigError, - InvalidRoutingPolicyError, + RoutingInEnvironmentsFileError, ) DEFAULT_ENVIRONMENTS_PATH = Path.home() / ".clair" / "environments.yml" class Environment(BaseModel): - """A single environment from environments.yml.""" + """A single environment from environments.yml. + + An environment holds connection settings only. Routing lives in the project + ``__routing__.py``, under the same environment name. + """ model_config = ConfigDict(populate_by_name=True) @@ -44,9 +46,6 @@ class Environment(BaseModel): region: str | None = None account_locator: str | None = None - # Routing - routing: Routing | None = None - def to_connection_dict(self) -> dict[str, Any]: """Return the connection dict expected by SnowflakeAdapter.connect().""" d: dict[str, Any] = { @@ -68,21 +67,6 @@ def to_connection_dict(self) -> dict[str, Any]: return d -def _validate_routing_block(routing_raw: dict[str, Any]) -> None: - """Pre-validate routing block before Pydantic parses it. - - Catches missing/unknown policy values and re-raises as clair-specific - error types that the CLI already handles. - """ - if "policy" not in routing_raw: - raise InvalidRoutingConfigError("routing block requires 'policy'") - - policy = routing_raw["policy"] - valid_policies = {"database_override", "schema_isolation"} - if policy not in valid_policies: - raise InvalidRoutingPolicyError(policy) - - def load_environment( env_name: str | None = None, environments_path: Path | None = None, @@ -104,8 +88,7 @@ def load_environment( Raises: EnvironmentsFileNotFoundError: If environments.yml does not exist. EnvironmentNotFoundError: If the requested environment is not in environments.yml. - InvalidRoutingPolicyError: If an unknown routing policy is specified. - InvalidRoutingConfigError: If the routing block is malformed. + RoutingInEnvironmentsFileError: If the environment still has a routing block. """ resolved_name = env_name or os.environ.get("CLAIR_ENV") or "dev" path = environments_path or DEFAULT_ENVIRONMENTS_PATH @@ -124,14 +107,9 @@ def load_environment( env_data: dict[str, Any] = raw[resolved_name] - routing_raw = env_data.get("routing") - if isinstance(routing_raw, dict): - _validate_routing_block(routing_raw) - - try: - environment = Environment(name=resolved_name, **env_data) - return resolved_name, environment - except ValidationError as exc: - # Surface Pydantic validation errors (e.g. missing schema_name for - # schema_isolation) as clair-specific errors the CLI already catches. - raise InvalidRoutingConfigError(str(exc)) from exc + # Pydantic drops an unknown key without a word. A leftover routing block + # would then send every write to the production names. + if "routing" in env_data: + raise RoutingInEnvironmentsFileError(str(path), resolved_name) + + return resolved_name, Environment(name=resolved_name, **env_data) diff --git a/src/clair/environments/project_routing.py b/src/clair/environments/project_routing.py new file mode 100644 index 0000000..20f39f5 --- /dev/null +++ b/src/clair/environments/project_routing.py @@ -0,0 +1,161 @@ +"""Load the project routing file, ``__routing__.py``. + +Routing lives in the project, not in ``~/.clair/environments.yml``: + +* Routing is not a secret. A team commits it and reviews it like other code. +* Routing gains the most from Python. A rule reads an environment variable, so + one committed rule gives each developer a separate target. +* A project-local file matches the clair version that the project pins. + +The file defines a ``routing`` dict. Each key is an environment name. That name +is the join key: it matches a top-level key in ``~/.clair/environments.yml``. +Each value is a ``RoutingConfig``, a callable, or None for passthrough. +""" + +from __future__ import annotations + +import hashlib +import importlib.util +import sys +from pathlib import Path +from typing import NamedTuple + +from clair.environments.routing import RoutingConfig, RoutingRule +from clair.exceptions import InvalidRoutingFileError + +ROUTING_FILE_NAME = "__routing__.py" +ROUTING_TABLE_ATTRIBUTE = "routing" + +# Cache key is (resolved path, modification time), so an edit reloads the file +# but repeated loads in one process do not run the file again. A rule that reads +# a keychain or a secret store must not run two times. +_routing_table_cache: dict[tuple[str, int], dict[str, RoutingRule | None]] = {} + + +class ProjectRouting(NamedTuple): + """The outcome of a routing file lookup for one environment.""" + + rule: RoutingRule | None + file_path: Path | None + environment_names: list[str] + has_entry: bool = False + + @property + def file_exists(self) -> bool: + """Tell the caller if the project has a ``__routing__.py``.""" + return self.file_path is not None + + @property + def is_unnamed_environment(self) -> bool: + """Tell the caller if the file omits this environment. + + An explicit ``"prod": None`` entry is a decision, so it reads as named. + A missing key is almost always a typo, so it reads as unnamed. + """ + return self.file_exists and not self.has_entry + + +def _module_name_for(path: Path) -> str: + """Build a module name that is unique per routing file path.""" + digest = hashlib.md5(str(path).encode()).hexdigest()[:8] + return f"_clair_routing_{digest}" + + +def _load_routing_table(path: Path) -> dict[str, RoutingRule | None]: + """Run a routing file and return its validated routing table. + + Args: + path: Path to the ``__routing__.py`` file. + + Returns: + The routing table, as a dict of environment name to routing rule. + + Raises: + InvalidRoutingFileError: If clair cannot run the file, or the table is + not in the expected shape. + """ + cache_key = (str(path), path.stat().st_mtime_ns) + cached = _routing_table_cache.get(cache_key) + if cached is not None: + return cached + + module_name = _module_name_for(path) + spec = importlib.util.spec_from_file_location(module_name, path) + if spec is None or spec.loader is None: + raise InvalidRoutingFileError(str(path), "clair cannot read this file") + + module = importlib.util.module_from_spec(spec) + # Register the module before execution. A class or a dataclass in the file + # then keeps one identity across loads. + sys.modules[module_name] = module + try: + spec.loader.exec_module(module) + except Exception as exc: + del sys.modules[module_name] + raise InvalidRoutingFileError( + str(path), f"{type(exc).__name__}: {exc}" + ) from exc + + if not hasattr(module, ROUTING_TABLE_ATTRIBUTE): + raise InvalidRoutingFileError( + str(path), f"the file must define a '{ROUTING_TABLE_ATTRIBUTE}' dict" + ) + + table = getattr(module, ROUTING_TABLE_ATTRIBUTE) + if not isinstance(table, dict): + raise InvalidRoutingFileError( + str(path), + f"'{ROUTING_TABLE_ATTRIBUTE}' must be a dict, " + f"but it is a {type(table).__name__}", + ) + + for env_name, rule in table.items(): + if not isinstance(env_name, str): + raise InvalidRoutingFileError( + str(path), + f"every key must be an environment name string, " + f"but one key is a {type(env_name).__name__}", + ) + if rule is None or isinstance(rule, RoutingConfig) or callable(rule): + continue + raise InvalidRoutingFileError( + str(path), + f"the rule for '{env_name}' is a {type(rule).__name__}. A rule must " + "be a RoutingConfig, a callable, or None", + ) + + _routing_table_cache[cache_key] = table + return table + + +def load_project_routing(project_root: Path, env_name: str) -> ProjectRouting: + """Find the routing rule for one environment. + + A project without a ``__routing__.py`` gets passthrough routing. An + environment without an entry in the table also gets passthrough routing, and + the caller warns about it, because passthrough writes to production names. + + Args: + project_root: Root directory of the clair project. + env_name: Resolved environment name, such as "dev". + + Returns: + A ``ProjectRouting`` with the rule, the file path, and all environment + names that the table defines. + + Raises: + InvalidRoutingFileError: If the file exists but clair cannot use it. + """ + path = project_root / ROUTING_FILE_NAME + if not path.exists(): + return ProjectRouting( + rule=None, file_path=None, environment_names=[], has_entry=False + ) + + table = _load_routing_table(path) + return ProjectRouting( + rule=table.get(env_name), + file_path=path, + environment_names=sorted(table), + has_entry=env_name in table, + ) diff --git a/src/clair/environments/routing.py b/src/clair/environments/routing.py index 931926f..74b7b4e 100644 --- a/src/clair/environments/routing.py +++ b/src/clair/environments/routing.py @@ -1,21 +1,48 @@ -"""Routing policies -- remap logical (database, schema, table) triples to physical targets.""" +"""Routing rules -- remap logical name triples to physical targets. + +A routing rule is one of two kinds: + +* A ``RoutingConfig`` subclass, such as ``DatabaseOverrideRouting``. +* Any callable with the signature + ``(database_name, schema_name, table_name) -> "database_name.schema_name.table_name"``. + +Both kinds go through ``route()``, which applies the rule and then validates the +result. That validation step is the reason a callable rule is safe: Python cannot +tell you in advance what a callable returns, so clair examines the output. +""" from __future__ import annotations +import inspect import re from abc import abstractmethod -from typing import Annotated, Literal +from collections.abc import Callable +from typing import TYPE_CHECKING, Literal -from pydantic import BaseModel, Field +from pydantic import BaseModel from clair.exceptions import InvalidRoutingConfigError from clair.trouves.trouve import TrouveType -_VALID_IDENTIFIER = re.compile(r"^[A-Z0-9_]+$") +if TYPE_CHECKING: + from clair.trouves.trouve import Trouve + + +# Snowflake accepts these characters in an unquoted identifier. +_VALID_IDENTIFIER = re.compile(r"^[A-Za-z_][A-Za-z0-9_$]*$") +_MAX_IDENTIFIER_LENGTH = 255 + +_ROUTED_NAME_PARTS = ("database_name", "schema_name", "table_name") + +# Maximum width of a rule description in a CLI message. +_MAX_DESCRIPTION_LENGTH = 200 + +# A quoted dict key at the start of a source line, such as ``"dev":``. +_DICT_KEY_PREFIX = re.compile(r"""^(['"])[^'"]*\1\s*:\s*""") class RoutingConfig(BaseModel): - """Base class for all routing policies.""" + """Base class for all typed routing rules.""" policy: str @@ -24,13 +51,10 @@ def apply(self, logical_name: str) -> str: """Remap a logical full_name to its physical target. Args: - logical_name: Filesystem-derived "database.schema.table" name. + logical_name: Filesystem-derived "database_name.schema_name.table_name". Returns: - The routed full_name string. - - Raises: - InvalidRoutingConfigError: If the routed identifier is invalid. + The routed full_name string. ``route()`` validates this value. """ @@ -41,67 +65,211 @@ class DatabaseOverrideRouting(RoutingConfig): database_name: str def apply(self, logical_name: str) -> str: - _, schema, table = logical_name.split(".") - return f"{self.database_name}.{schema}.{table}" + _, schema_name, table_name = logical_name.split(".") + return f"{self.database_name}.{schema_name}.{table_name}" class SchemaIsolationRouting(RoutingConfig): - """Collapse database.schema.table into a single table token under a fixed database and schema.""" + """Collapse a name triple into one table token under a fixed database and schema.""" policy: Literal["schema_isolation"] = "schema_isolation" database_name: str schema_name: str def apply(self, logical_name: str) -> str: - db, schema, table = logical_name.split(".") - new_table = f"{db}_{schema}_{table}".upper() - if not _VALID_IDENTIFIER.match(new_table): + database_name, schema_name, table_name = logical_name.split(".") + collapsed_table_name = f"{database_name}_{schema_name}_{table_name}".upper() + return f"{self.database_name}.{self.schema_name}.{collapsed_table_name}" + + +# A routing rule is either a typed config or a plain callable. +RoutingCallable = Callable[[str, str, str], str] +RoutingRule = RoutingConfig | RoutingCallable + + +def describe_routing(routing: RoutingRule | None) -> str: + """Return a short human-readable description of a routing rule. + + The CLI prints this text in collision and validation messages. For a callable + rule, the description is the source code of the rule, because the source is + what tells you why two names collided. + """ + if routing is None: + return "none" + if isinstance(routing, DatabaseOverrideRouting): + return f"database_override → {routing.database_name}" + if isinstance(routing, SchemaIsolationRouting): + return f"schema_isolation → {routing.database_name}.{routing.schema_name}" + if isinstance(routing, RoutingConfig): + return routing.policy + return _describe_callable(routing) + + +def _describe_callable(routing: RoutingCallable) -> str: + """Return the source text of a callable rule, or its name as a fallback. + + A long rule is truncated, not dropped. Even a partial rule tells the reader + more than the word "lambda". + """ + try: + source = inspect.getsource(routing).strip() + except (OSError, TypeError): + source = "" + + if source: + one_line = " ".join(line.strip() for line in source.splitlines()) + one_line = _strip_dict_punctuation(one_line) + if len(one_line) > _MAX_DESCRIPTION_LENGTH: + return one_line[: _MAX_DESCRIPTION_LENGTH - 1] + "…" + return one_line + + name = getattr(routing, "__name__", "") + return name or repr(routing) + + +def _strip_dict_punctuation(source: str) -> str: + """Remove the dict syntax around a rule that a routing table holds. + + ``inspect.getsource`` returns the whole line, so a lambda inside a dict + arrives as ``"dev": lambda ...: (...),``. The key and the comma add noise. + """ + stripped = source.strip().rstrip(",").strip() + key_match = _DICT_KEY_PREFIX.match(stripped) + if key_match: + stripped = stripped[key_match.end():].strip() + return stripped + + +def _apply_routing(logical_name: str, routing: RoutingRule) -> object: + """Apply a routing rule and return its raw, not yet validated, result.""" + if isinstance(routing, RoutingConfig): + return routing.apply(logical_name) + + database_name, schema_name, table_name = logical_name.split(".") + try: + return routing(database_name, schema_name, table_name) + except InvalidRoutingConfigError: + raise + except Exception as exc: + raise InvalidRoutingConfigError( + f"The routing rule `{describe_routing(routing)}` failed on " + f"'{logical_name}': {type(exc).__name__}: {exc}" + ) from exc + + +def _validate_routed_name( + routed_name: object, logical_name: str, routing: RoutingRule +) -> str: + """Confirm that a routing rule returned a usable physical name. + + Args: + routed_name: The raw value that the routing rule returned. + logical_name: The name that clair gave to the rule. + routing: The rule itself. Used for the error message. + + Returns: + The routed name, as a validated string. + + Raises: + InvalidRoutingConfigError: If the value is not a valid 3-part name. + """ + rule_text = f"The routing rule `{describe_routing(routing)}`" + + if not isinstance(routed_name, str): + raise InvalidRoutingConfigError( + f"{rule_text} returned {type(routed_name).__name__} for '{logical_name}'. " + "A routing rule must return a " + "'database_name.schema_name.table_name' string." + ) + + parts = routed_name.split(".") + if len(parts) != 3: + raise InvalidRoutingConfigError( + f"{rule_text} returned '{routed_name}' for '{logical_name}'. " + f"A routed name needs 3 dot-separated parts, but this name has {len(parts)}." + ) + + for part_label, part in zip(_ROUTED_NAME_PARTS, parts): + if len(part) > _MAX_IDENTIFIER_LENGTH: raise InvalidRoutingConfigError( - f"schema_isolation produced invalid identifier '{new_table}' " - "(only A-Z, 0-9, _ are allowed)" + f"{rule_text} returned the {part_label} '{part}' for '{logical_name}' " + f"({len(part)} characters, maximum {_MAX_IDENTIFIER_LENGTH})." ) - if len(new_table) > 255: + if not _VALID_IDENTIFIER.match(part): raise InvalidRoutingConfigError( - f"schema_isolation produced identifier '{new_table}' " - f"({len(new_table)} chars, max 255)" + f"{rule_text} returned the invalid {part_label} '{part}' for " + f"'{logical_name}'. An identifier starts with a letter or an " + "underscore. The other characters are letters, digits, underscores " + "or dollar signs." ) - return f"{self.database_name}.{self.schema_name}.{new_table}" - -# Discriminated union used for parsing routing blocks from YAML/dicts. -Routing = Annotated[ - DatabaseOverrideRouting | SchemaIsolationRouting, - Field(discriminator="policy"), -] + return routed_name def route( logical_name: str, trouve_type: TrouveType, - routing: RoutingConfig | None, + routing: RoutingRule | None, ) -> str: - """Apply a routing policy to a logical full_name. + """Apply a routing rule to a logical full_name. - SOURCE Trouves always pass through regardless of the routing policy. + SOURCE Trouves always pass through, whatever the rule is. Args: - logical_name: Filesystem-derived "database.schema.table" name. + logical_name: Filesystem-derived "database_name.schema_name.table_name". trouve_type: SOURCE, TABLE, or VIEW. - routing: Active routing config, or None for passthrough. + routing: Active routing rule, or None for passthrough. Returns: The routed full_name string. + + Raises: + InvalidRoutingConfigError: If the rule fails, or returns an unusable name. """ if routing is None or trouve_type == TrouveType.SOURCE: return logical_name - return routing.apply(logical_name) + + routed_name = _apply_routing(logical_name, routing) + return _validate_routed_name(routed_name, logical_name, routing) + + +def collect_routing_problems( + trouves: list[Trouve], + routing: RoutingRule | None, +) -> list[tuple[str, str]]: + """Apply a routing rule to every Trouve and collect all failures. + + ``route()`` stops at the first bad name, which is correct for a run. This + function instead reports every problem at once, so that ``clair validate`` + shows a complete list. + + Args: + trouves: All Trouves in the project, discovered with routing disabled. + routing: The routing rule to test. + + Returns: + List of ``(logical_name, problem_text)`` pairs, in discovery order. + """ + if routing is None: + return [] + + problems: list[tuple[str, str]] = [] + for trouve in trouves: + if not trouve.compiled: + continue + logical_name = trouve.compiled.logical_name + try: + route(logical_name, trouve.type, routing) + except InvalidRoutingConfigError as exc: + problems.append((logical_name, str(exc))) + return problems def detect_routing_collisions(logical_to_routed: dict[str, str]) -> list[tuple[str, list[str]]]: """Return (target, sources) pairs for any routing collisions. A collision occurs when two TABLE/VIEW Trouves route to the same physical target. - The last write in execution order will determine the final state of that target. + The last write in execution order sets the final state of that target. Args: logical_to_routed: Mapping of logical_name -> routed_name for non-SOURCE Trouves. diff --git a/src/clair/exceptions.py b/src/clair/exceptions.py index 8fee110..94f9123 100644 --- a/src/clair/exceptions.py +++ b/src/clair/exceptions.py @@ -33,25 +33,40 @@ class EnvironmentsFileNotFoundError(ClairError): def __init__(self, path: str) -> None: self.path = path super().__init__( - f"environments.yml not found at {path}. " - "Run `clair init` to create one, or rename your profiles.yml " - "and add a routing block." + f"environments.yml not found at {path}. Run `clair init` to create one." ) -class InvalidRoutingPolicyError(ClairError): - """Raised when an unknown routing policy is specified.""" +class RoutingInEnvironmentsFileError(ClairError): + """Raised when environments.yml still holds a routing block. - def __init__(self, policy: str) -> None: - self.policy = policy + Routing moved out of environments.yml and into the project __routing__.py. + A silent skip of the old block would send writes to the production names, so + clair stops and asks the user to move the rule. + """ + + def __init__(self, path: str, env_name: str) -> None: + self.path = path + self.env_name = env_name super().__init__( - f"Unknown routing policy '{policy}'. " - "Valid policies: database_override, schema_isolation" + f"Environment '{env_name}' in {path} has a 'routing' block, but " + "routing moved to the project. Delete the block, then add the rule " + f"to __routing__.py under the key '{env_name}'. " + "Run `clair validate` to test the new rule." ) +class InvalidRoutingFileError(ClairError): + """Raised when the project __routing__.py exists but clair cannot use it.""" + + def __init__(self, path: str, detail: str) -> None: + self.path = path + self.detail = detail + super().__init__(f"Invalid routing file at {path}: {detail}") + + class InvalidRoutingConfigError(ClairError): - """Raised when a routing config block is malformed.""" + """Raised when a routing rule fails, or returns an unusable name.""" def __init__(self, detail: str) -> None: super().__init__(detail) diff --git a/tests/conftest.py b/tests/conftest.py index e50266a..676ead7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -32,7 +32,7 @@ def clean_sys_modules(): for mod_name in after - before: if any( part in mod_name - for part in ("source.", "analytics.", "db.", "tmp_project") + for part in ("source.", "analytics.", "db.", "tmp_project", "_clair_routing_") ): del sys.modules[mod_name] @@ -68,7 +68,7 @@ def tmp_environments(tmp_path: Path) -> Path: private_key_passphrase: s3cr3t warehouse: key_wh -with_routing: +legacy_routing: account: test-account user: test-user authenticator: externalbrowser @@ -76,17 +76,15 @@ def tmp_environments(tmp_path: Path) -> Path: routing: policy: database_override database_name: OMER_DEV - -with_schema_isolation: - account: test-account - user: test-user - authenticator: externalbrowser - warehouse: test_wh - routing: - policy: schema_isolation - database_name: DEV - schema_name: obaddour """ environments_file = tmp_path / "environments.yml" environments_file.write_text(environments_content) return environments_file + + +@pytest.fixture +def routing_project(tmp_path: Path) -> Path: + """Create a project directory that holds a __routing__.py file.""" + project_dir = tmp_path / "routing_project" + project_dir.mkdir() + return project_dir diff --git a/tests/unit/test_environments.py b/tests/unit/test_environments.py index c815014..ac3f971 100644 --- a/tests/unit/test_environments.py +++ b/tests/unit/test_environments.py @@ -7,12 +7,10 @@ import pytest from clair.environments.environments import load_environment -from clair.environments.routing import SchemaIsolationRouting from clair.exceptions import ( EnvironmentNotFoundError, EnvironmentsFileNotFoundError, - InvalidRoutingConfigError, - InvalidRoutingPolicyError, + RoutingInEnvironmentsFileError, ) @@ -57,42 +55,28 @@ def test_encrypted_private_key_environment(self, tmp_environments: Path): assert env.private_key_path == "/secrets/snowflake_key_enc.p8" assert env.private_key_passphrase == "s3cr3t" - def test_routing_parsed_for_database_override(self, tmp_environments: Path): - _, env = load_environment(env_name="with_routing", environments_path=tmp_environments) - assert env.routing is not None - assert env.routing.policy == "database_override" - assert env.routing.database_name == "OMER_DEV" + def test_environment_has_no_routing_attribute(self, tmp_environments: Path): + _, env = load_environment(env_name="dev", environments_path=tmp_environments) + assert not hasattr(env, "routing") - def test_routing_parsed_for_schema_isolation(self, tmp_environments: Path): - _, env = load_environment(env_name="with_schema_isolation", environments_path=tmp_environments) - assert isinstance(env.routing, SchemaIsolationRouting) - assert env.routing.policy == "schema_isolation" - assert env.routing.database_name == "DEV" - assert env.routing.schema_name == "obaddour" - def test_no_routing_returns_none(self, tmp_environments: Path): - _, env = load_environment(env_name="dev", environments_path=tmp_environments) - assert env.routing is None - - -class TestLoadEnvironmentValidation: - def test_missing_policy_raises(self, tmp_path: Path): - bad = tmp_path / "env.yml" - bad.write_text("dev:\n account: x\n user: y\n warehouse: z\n routing:\n database_name: FOO\n") - with pytest.raises(InvalidRoutingConfigError, match="policy"): - load_environment(environments_path=bad) - - def test_unknown_policy_raises(self, tmp_path: Path): - bad = tmp_path / "env.yml" - bad.write_text("dev:\n account: x\n user: y\n warehouse: z\n routing:\n policy: nonsense\n database_name: FOO\n") - with pytest.raises(InvalidRoutingPolicyError, match="nonsense"): - load_environment(environments_path=bad) - - def test_schema_isolation_missing_schema_name_raises(self, tmp_path: Path): - bad = tmp_path / "env.yml" - bad.write_text("dev:\n account: x\n user: y\n warehouse: z\n routing:\n policy: schema_isolation\n database_name: DEV\n") - with pytest.raises(InvalidRoutingConfigError, match="schema_name"): - load_environment(environments_path=bad) +class TestLegacyRoutingBlock: + """Routing moved to the project __routing__.py. The old block must not pass silently.""" + + def test_legacy_routing_block_raises(self, tmp_environments: Path): + with pytest.raises(RoutingInEnvironmentsFileError, match="legacy_routing"): + load_environment(env_name="legacy_routing", environments_path=tmp_environments) + + def test_error_names_the_new_file(self, tmp_environments: Path): + with pytest.raises(RoutingInEnvironmentsFileError, match=r"__routing__\.py"): + load_environment(env_name="legacy_routing", environments_path=tmp_environments) + + def test_environment_without_routing_block_loads(self, tmp_path: Path): + good = tmp_path / "env.yml" + good.write_text("dev:\n account: x\n user: y\n warehouse: z\n") + name, env = load_environment(environments_path=good) + assert name == "dev" + assert env.account == "x" class TestToConnectionDict: diff --git a/tests/unit/test_project_routing.py b/tests/unit/test_project_routing.py new file mode 100644 index 0000000..ad1933d --- /dev/null +++ b/tests/unit/test_project_routing.py @@ -0,0 +1,165 @@ +"""Tests for the project __routing__.py loader.""" + +from __future__ import annotations + +import textwrap +from pathlib import Path + +import pytest + +from clair.environments.project_routing import ( + ROUTING_FILE_NAME, + load_project_routing, +) +from clair.environments.routing import DatabaseOverrideRouting, route +from clair.exceptions import InvalidRoutingFileError +from clair.trouves.trouve import TrouveType + + +def _write_routing_file(project_dir: Path, body: str) -> Path: + path = project_dir / ROUTING_FILE_NAME + path.write_text(textwrap.dedent(body)) + return path + + +class TestLoadProjectRouting: + def test_missing_file_gives_passthrough(self, routing_project: Path): + result = load_project_routing(routing_project, "dev") + assert result.rule is None + assert result.file_path is None + assert result.file_exists is False + + def test_loads_a_callable_rule(self, routing_project: Path): + _write_routing_file(routing_project, ''' + routing = { + "dev": lambda database_name, schema_name, table_name: ( + f"{database_name}_dev.{schema_name}.{table_name}" + ), + } + ''') + result = load_project_routing(routing_project, "dev") + assert callable(result.rule) + assert route("refined.products.catalog", TrouveType.TABLE, result.rule) == ( + "refined_dev.products.catalog" + ) + + def test_loads_a_typed_rule(self, routing_project: Path): + _write_routing_file(routing_project, ''' + from clair.environments.routing import DatabaseOverrideRouting + + routing = {"dev": DatabaseOverrideRouting(database_name="OMER_DEV")} + ''') + result = load_project_routing(routing_project, "dev") + assert isinstance(result.rule, DatabaseOverrideRouting) + assert result.rule.database_name == "OMER_DEV" + + def test_environment_without_a_rule_gives_passthrough(self, routing_project: Path): + _write_routing_file(routing_project, ''' + routing = {"dev": None, "prod": None} + ''') + result = load_project_routing(routing_project, "prod") + assert result.rule is None + assert result.file_exists is True + + def test_unknown_environment_reports_the_known_names(self, routing_project: Path): + _write_routing_file(routing_project, ''' + routing = {"dev": None, "staging": None} + ''') + result = load_project_routing(routing_project, "typo") + assert result.rule is None + assert result.environment_names == ["dev", "staging"] + + def test_rule_reads_an_environment_variable(self, routing_project: Path, monkeypatch): + monkeypatch.setenv("CLAIR_USER", "obaddour") + _write_routing_file(routing_project, ''' + import os + + routing = { + "dev": lambda database_name, schema_name, table_name: ( + f"{database_name}_{os.environ['CLAIR_USER'].upper()}" + f".{schema_name}.{table_name}" + ), + } + ''') + result = load_project_routing(routing_project, "dev") + assert route("analytics.finance.revenue", TrouveType.TABLE, result.rule) == ( + "analytics_OBADDOUR.finance.revenue" + ) + + +class TestRoutingFileValidation: + def test_syntax_error_raises(self, routing_project: Path): + _write_routing_file(routing_project, "routing = {\n") + with pytest.raises(InvalidRoutingFileError, match="SyntaxError"): + load_project_routing(routing_project, "dev") + + def test_error_at_import_time_raises(self, routing_project: Path): + _write_routing_file(routing_project, ''' + raise ValueError("boom") + ''') + with pytest.raises(InvalidRoutingFileError, match="boom"): + load_project_routing(routing_project, "dev") + + def test_missing_routing_table_raises(self, routing_project: Path): + _write_routing_file(routing_project, "# no routing table here\n") + with pytest.raises(InvalidRoutingFileError, match="'routing' dict"): + load_project_routing(routing_project, "dev") + + def test_routing_table_not_a_dict_raises(self, routing_project: Path): + _write_routing_file(routing_project, "routing = 'not a dict'\n") + with pytest.raises(InvalidRoutingFileError, match="must be a dict"): + load_project_routing(routing_project, "dev") + + def test_rule_of_wrong_type_raises(self, routing_project: Path): + _write_routing_file(routing_project, ''' + routing = {"dev": "OMER_DEV"} + ''') + with pytest.raises(InvalidRoutingFileError, match="must "): + load_project_routing(routing_project, "dev") + + def test_non_string_key_raises(self, routing_project: Path): + _write_routing_file(routing_project, ''' + routing = {1: None} + ''') + with pytest.raises(InvalidRoutingFileError, match="environment name string"): + load_project_routing(routing_project, "dev") + + +class TestRoutingFileCache: + def test_repeated_loads_run_the_file_one_time(self, routing_project: Path): + _write_routing_file(routing_project, ''' + import os + + os.environ["CLAIR_TEST_LOAD_COUNT"] = str( + int(os.environ.get("CLAIR_TEST_LOAD_COUNT", "0")) + 1 + ) + + routing = {"dev": None} + ''') + import os + + os.environ.pop("CLAIR_TEST_LOAD_COUNT", None) + try: + load_project_routing(routing_project, "dev") + load_project_routing(routing_project, "dev") + load_project_routing(routing_project, "dev") + assert os.environ["CLAIR_TEST_LOAD_COUNT"] == "1" + finally: + os.environ.pop("CLAIR_TEST_LOAD_COUNT", None) + + def test_an_edit_reloads_the_file(self, routing_project: Path): + _write_routing_file(routing_project, ''' + from clair.environments.routing import DatabaseOverrideRouting + + routing = {"dev": DatabaseOverrideRouting(database_name="FIRST")} + ''') + first = load_project_routing(routing_project, "dev") + assert first.rule.database_name == "FIRST" + + _write_routing_file(routing_project, ''' + from clair.environments.routing import DatabaseOverrideRouting + + routing = {"dev": DatabaseOverrideRouting(database_name="SECOND")} + ''') + second = load_project_routing(routing_project, "dev") + assert second.rule.database_name == "SECOND" diff --git a/tests/unit/test_routing.py b/tests/unit/test_routing.py index 8aa8082..a895d64 100644 --- a/tests/unit/test_routing.py +++ b/tests/unit/test_routing.py @@ -8,6 +8,7 @@ from clair.environments.routing import ( DatabaseOverrideRouting, SchemaIsolationRouting, + describe_routing, detect_routing_collisions, route, ) @@ -74,6 +75,111 @@ def test_no_routing_passthrough_for_view(self): assert result == "analytics.finance.summary" +class TestCallableRouting: + def test_callable_routes_a_table(self): + def routing(database_name, schema_name, table_name): + return f"{database_name}_dev.{schema_name}.{table_name}" + + result = route("refined.products.catalog", TrouveType.TABLE, routing) + assert result == "refined_dev.products.catalog" + + def test_callable_receives_three_separate_arguments(self): + seen = [] + + def routing(database_name, schema_name, table_name): + seen.append((database_name, schema_name, table_name)) + return f"{database_name}.{schema_name}.{table_name}" + + route("analytics.finance.revenue", TrouveType.TABLE, routing) + assert seen == [("analytics", "finance", "revenue")] + + def test_source_passthrough_with_callable(self): + def routing(database_name, schema_name, table_name): + return f"{database_name}_dev.{schema_name}.{table_name}" + + result = route("refined.products.catalog", TrouveType.SOURCE, routing) + assert result == "refined.products.catalog" + + def test_callable_returning_two_parts_raises(self): + routing = lambda database_name, schema_name, table_name: f"{schema_name}.{table_name}" + with pytest.raises(InvalidRoutingConfigError, match="3 dot-separated parts"): + route("analytics.finance.revenue", TrouveType.TABLE, routing) + + def test_callable_returning_non_string_raises(self): + routing = lambda database_name, schema_name, table_name: None + with pytest.raises(InvalidRoutingConfigError, match="NoneType"): + route("analytics.finance.revenue", TrouveType.TABLE, routing) + + def test_callable_returning_invalid_identifier_raises(self): + routing = lambda database_name, schema_name, table_name: f"my-db.{schema_name}.{table_name}" + with pytest.raises(InvalidRoutingConfigError, match="invalid database_name"): + route("analytics.finance.revenue", TrouveType.TABLE, routing) + + def test_callable_that_raises_is_wrapped(self, monkeypatch): + import os + + monkeypatch.delenv("CLAIR_USER", raising=False) + + def routing(database_name, schema_name, table_name): + return f"{database_name}_{os.environ['CLAIR_USER']}.{schema_name}.{table_name}" + + with pytest.raises(InvalidRoutingConfigError, match="KeyError"): + route("analytics.finance.revenue", TrouveType.TABLE, routing) + + def test_callable_reads_environment_variable(self, monkeypatch): + import os + + monkeypatch.setenv("CLAIR_USER", "obaddour") + + def routing(database_name, schema_name, table_name): + user = os.environ["CLAIR_USER"].upper() + return f"{database_name}_{user}.{schema_name}.{table_name}" + + result = route("analytics.finance.revenue", TrouveType.TABLE, routing) + assert result == "analytics_OBADDOUR.finance.revenue" + + +class TestTypedRoutingValidation: + """route() validates every rule kind, not schema_isolation alone.""" + + def test_database_override_rejects_an_invalid_identifier(self): + routing = _db_override("my-dev-db") + with pytest.raises(InvalidRoutingConfigError, match="invalid database_name"): + route("analytics.finance.revenue", TrouveType.TABLE, routing) + + def test_database_override_accepts_a_valid_identifier(self): + routing = _db_override("MY_DEV_DB") + assert route("a.b.c", TrouveType.TABLE, routing) == "MY_DEV_DB.b.c" + + +class TestDescribeRouting: + def test_describes_none(self): + assert describe_routing(None) == "none" + + def test_describes_database_override(self): + assert "OMER_DEV" in describe_routing(_db_override("OMER_DEV")) + + def test_describes_schema_isolation(self): + description = describe_routing(_schema_isolation("DEV", "obaddour")) + assert "DEV.obaddour" in description + + def test_describes_a_named_function_by_its_source(self): + def dev_routing(database_name, schema_name, table_name): + return f"{database_name}_dev.{schema_name}.{table_name}" + + description = describe_routing(dev_routing) + assert "_dev" in description + + def test_description_stays_on_one_line(self): + def dev_routing(database_name, schema_name, table_name): + return ( + f"{database_name}_dev" + f".{schema_name}.{table_name}" + ) + + assert "\n" not in describe_routing(dev_routing) + + class TestDetectRoutingCollisions: def test_no_collision_returns_empty(self): result = detect_routing_collisions({ diff --git a/tests/unit/test_scaffold.py b/tests/unit/test_scaffold.py index bb50548..8c73803 100644 --- a/tests/unit/test_scaffold.py +++ b/tests/unit/test_scaffold.py @@ -50,11 +50,17 @@ def test_creates_environments_yml(self, tmp_path: Path) -> None: environments_path = fake_home / ".clair" / "environments.yml" assert environments_path.exists() + def test_creates_routing_file(self, tmp_path: Path) -> None: + project_dir = tmp_path / "my_project" + scaffold_project(project_dir, **DEFAULT_SOURCE_ARGS, home_dir=tmp_path / "home") + + assert (project_dir / "__routing__.py").exists() + def test_returns_all_paths_as_created(self, tmp_path: Path) -> None: results = _run_scaffold(tmp_path) - # 1 project file + 1 environments.yml - assert len(results) == 2 + # 1 source Trouve + 1 __routing__.py + 1 environments.yml + assert len(results) == 3 assert all(status == "created" for status, _ in results) @@ -74,7 +80,17 @@ def test_environments_yml_has_dev_environment(self, tmp_path: Path) -> None: assert "dev:" in content assert "account:" in content assert "externalbrowser" in content - assert " routing:" not in content # routing omitted by default; shown as comment only + # Routing lives in the project __routing__.py, never in environments.yml. + assert " routing:" not in content + + def test_routing_file_defines_a_routing_table(self, tmp_path: Path) -> None: + project_dir = tmp_path / "proj" + scaffold_project(project_dir, **DEFAULT_SOURCE_ARGS, home_dir=tmp_path / "home") + + content = (project_dir / "__routing__.py").read_text() + assert "routing = {" in content + assert "CLAIR_USER" in content + assert "database_name, schema_name, table_name" in content class TestDoesNotOverwriteExistingFiles: diff --git a/tests/unit/test_validate_cli.py b/tests/unit/test_validate_cli.py new file mode 100644 index 0000000..e73bc67 --- /dev/null +++ b/tests/unit/test_validate_cli.py @@ -0,0 +1,162 @@ +"""Tests for the `clair validate` command.""" + +from __future__ import annotations + +import textwrap +from pathlib import Path + +import pytest +from click.testing import CliRunner + +from clair.cli.main import cli + + +@pytest.fixture +def project_with_trouves(tmp_path: Path) -> Path: + """Build a project with one SOURCE Trouve and two TABLE Trouves.""" + project_dir = tmp_path / "proj" + (project_dir / "source" / "raw").mkdir(parents=True) + (project_dir / "source" / "raw" / "orders.py").write_text( + "from clair import Trouve, TrouveType\n\n" + "trouve = Trouve(type=TrouveType.SOURCE)\n" + ) + for schema_name in ("finance", "reports"): + (project_dir / "analytics" / schema_name).mkdir(parents=True) + (project_dir / "analytics" / schema_name / "revenue.py").write_text( + "from clair import Trouve, TrouveType\n\n" + 'trouve = Trouve(type=TrouveType.TABLE, sql="SELECT 1 AS x")\n' + ) + return project_dir + + +def _write_routing(project_dir: Path, body: str) -> None: + (project_dir / "__routing__.py").write_text(textwrap.dedent(body)) + + +def _run_validate(project_dir: Path, *args: str): + return CliRunner().invoke(cli, ["validate", "--project", str(project_dir), *args]) + + +class TestValidateSucceeds: + def test_no_routing_file_passes(self, project_with_trouves: Path): + result = _run_validate(project_with_trouves) + assert result.exit_code == 0 + assert "Every routed name is valid" in result.output + + def test_valid_callable_rule_passes(self, project_with_trouves: Path): + _write_routing(project_with_trouves, ''' + routing = { + "dev": lambda database_name, schema_name, table_name: ( + f"{database_name}_dev.{schema_name}.{table_name}" + ), + } + ''') + result = _run_validate(project_with_trouves) + assert result.exit_code == 0 + assert "Every routed name is valid" in result.output + + def test_output_names_the_environment_and_rule(self, project_with_trouves: Path): + _write_routing(project_with_trouves, ''' + routing = { + "dev": lambda database_name, schema_name, table_name: ( + f"{database_name}_dev.{schema_name}.{table_name}" + ), + } + ''') + result = _run_validate(project_with_trouves) + assert "environment: dev" in result.output + # The rule description shows the source, not the word "lambda" alone. + assert "_dev" in result.output + + def test_counts_only_routable_trouves(self, project_with_trouves: Path): + # 2 TABLE Trouves route. The SOURCE Trouve never routes. + result = _run_validate(project_with_trouves) + assert "Trouves to route: 2" in result.output + + +class TestValidateFails: + def test_invalid_identifier_fails(self, project_with_trouves: Path): + _write_routing(project_with_trouves, ''' + routing = { + "dev": lambda database_name, schema_name, table_name: ( + f"{database_name}-dev.{schema_name}.{table_name}" + ), + } + ''') + result = _run_validate(project_with_trouves) + assert result.exit_code == 1 + assert "invalid database_name" in result.output + + def test_reports_every_bad_trouve_not_only_the_first(self, project_with_trouves: Path): + _write_routing(project_with_trouves, ''' + routing = { + "dev": lambda database_name, schema_name, table_name: ( + f"{database_name}-dev.{schema_name}.{table_name}" + ), + } + ''') + result = _run_validate(project_with_trouves) + assert "analytics.finance.revenue" in result.output + assert "analytics.reports.revenue" in result.output + assert "2 problems found" in result.output + + def test_collision_fails(self, project_with_trouves: Path): + _write_routing(project_with_trouves, ''' + routing = { + "dev": lambda database_name, schema_name, table_name: ( + f"DEV.shared.{table_name}" + ), + } + ''') + result = _run_validate(project_with_trouves) + assert result.exit_code == 1 + assert "DEV.shared.revenue" in result.output + assert "analytics.finance.revenue" in result.output + assert "analytics.reports.revenue" in result.output + + def test_rule_that_raises_fails(self, project_with_trouves: Path, monkeypatch): + monkeypatch.delenv("CLAIR_USER", raising=False) + _write_routing(project_with_trouves, ''' + import os + + routing = { + "dev": lambda database_name, schema_name, table_name: ( + f"{database_name}_{os.environ['CLAIR_USER']}" + f".{schema_name}.{table_name}" + ), + } + ''') + result = _run_validate(project_with_trouves) + assert result.exit_code == 1 + assert "CLAIR_USER" in result.output + + def test_broken_routing_file_fails(self, project_with_trouves: Path): + _write_routing(project_with_trouves, "routing = {\n") + result = _run_validate(project_with_trouves) + assert result.exit_code == 1 + + +class TestUnnamedEnvironmentWarning: + def test_absent_environment_warns(self, project_with_trouves: Path): + _write_routing(project_with_trouves, ''' + routing = {"dev": None} + ''') + result = _run_validate(project_with_trouves, "--env", "typo") + assert "does not name the environment 'typo'" in result.output + + def test_explicit_none_does_not_warn(self, project_with_trouves: Path): + """A "prod": None entry is a decision, so it must stay quiet.""" + _write_routing(project_with_trouves, ''' + routing = {"dev": None, "prod": None} + ''') + result = _run_validate(project_with_trouves, "--env", "prod") + assert result.exit_code == 0 + assert "does not name the environment" not in result.output + + def test_env_var_selects_the_environment(self, project_with_trouves: Path, monkeypatch): + monkeypatch.setenv("CLAIR_ENV", "staging") + _write_routing(project_with_trouves, ''' + routing = {"staging": None} + ''') + result = _run_validate(project_with_trouves) + assert "environment: staging" in result.output From 1460574acbe5825080f2da0ee6748b3c95b89d9a Mon Sep 17 00:00:00 2001 From: OmerBaddour Date: Sun, 2 Aug 2026 17:51:37 -0400 Subject: [PATCH 2/4] docs: record the v0 backwards compatibility stance --- CLAUDE.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index c01fee8..fd19f13 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -42,6 +42,15 @@ The worktree shares git history with the main repo but has its own `.venv/`. Alw - Use `database_name` instead of `database`, `schema_name` instead of `schema`, `table_name` instead of `table` - Address git merge conflicts by pulling main, resolving conflicts, and pushing. Favour simplicity over clean commit history — PRs are squash-merged anyway. +## Backwards compatibility + +The major version is 0. While the major version stays 0, clair does not keep backwards compatibility. clair has no users at this time, so the best system design wins against a stable interface. + +- Change a public name, a file format, or a function signature when the change makes the system better. +- Do not add a deprecation shim, an alias for an old name, or a migration path. +- Delete the old code path. Do not keep it beside the new one. +- Name each behaviour change in the pull request description. + ## Comments: Simplified Technical English Write all code comments and docstrings in Simplified Technical English (ASD-STE100). STE is a controlled writing standard that makes technical text clear and unambiguous for readers who do not speak English as a first language. From bd153331a1408f647a97aa53091ce585a3d4210b Mon Sep 17 00:00:00 2001 From: OmerBaddour Date: Sun, 2 Aug 2026 18:16:07 -0400 Subject: [PATCH 3/4] fix: satisfy the ty type checker in validate and in the routing tests The validate command now keeps the (logical name, type) pair, not the Trouve. The type checker cannot narrow trouve.compiled across a list comprehension, and the name is the only part that the collision report needs. The tests that exercise a runtime error now say so in a way that the type checker accepts: model_validate for an absent field, setattr for a frozen model, and model_dump to read a field of a user subclass. --- src/clair/cli/main.py | 15 ++++++++------- tests/unit/test_project_routing.py | 9 ++++++--- tests/unit/test_routing.py | 5 +++-- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/src/clair/cli/main.py b/src/clair/cli/main.py index 44facb4..1cdc1c3 100644 --- a/src/clair/cli/main.py +++ b/src/clair/cli/main.py @@ -385,9 +385,12 @@ def validate(project: str, env: str | None) -> None: sys.exit(1) routing = project_routing.entry - routable = [ - trouve for trouve in discovered - if trouve.compiled and trouve.type != TrouveType.SOURCE + # Keep the (logical name, type) pair, not the Trouve. The name is the only + # part that the collision report needs, and here it is never None. + routable: list[tuple[str, TrouveType]] = [ + (trouve.compiled.logical_name, trouve.type) + for trouve in discovered + if trouve.compiled is not None and trouve.type != TrouveType.SOURCE ] click.echo(f"\n environment: {env_name}") @@ -403,10 +406,8 @@ def validate(project: str, env: str | None) -> None: collisions: list[tuple[str, list[str]]] = [] if not problems: logical_to_routed = { - trouve.compiled.logical_name: route( - trouve.compiled.logical_name, trouve.type, routing - ) - for trouve in routable + logical_name: route(logical_name, trouve_type, routing) + for logical_name, trouve_type in routable } collisions = detect_routing_collisions(logical_to_routed) for routed_target, logical_sources in collisions: diff --git a/tests/unit/test_project_routing.py b/tests/unit/test_project_routing.py index 6a26faf..4b0fe9b 100644 --- a/tests/unit/test_project_routing.py +++ b/tests/unit/test_project_routing.py @@ -61,7 +61,8 @@ def test_loads_an_entry(self, routing_project: Path): ''') result = load_project_routing(routing_project, "dev") assert result.entry is not None - assert result.entry.database_name == "OMER_DEV" + # The entry is a user subclass, so read its own field through the model. + assert result.entry.model_dump()["database_name"] == "OMER_DEV" assert route("a.b.c", TrouveType.TABLE, result.entry) == "OMER_DEV.b.c" def test_an_empty_table_gives_passthrough(self, routing_project: Path): @@ -177,10 +178,12 @@ def test_an_edit_reloads_the_file(self, routing_project: Path): routing = RoutingTable(entries=[DatabaseOverride(database_name="FIRST")]) ''') first = load_project_routing(routing_project, "dev") - assert first.entry.database_name == "FIRST" + assert first.entry is not None + assert first.entry.model_dump()["database_name"] == "FIRST" _write_with_prelude(routing_project, ''' routing = RoutingTable(entries=[DatabaseOverride(database_name="SECOND")]) ''') second = load_project_routing(routing_project, "dev") - assert second.entry.database_name == "SECOND" + assert second.entry is not None + assert second.entry.model_dump()["database_name"] == "SECOND" diff --git a/tests/unit/test_routing.py b/tests/unit/test_routing.py index 048a3e1..bf0bbc1 100644 --- a/tests/unit/test_routing.py +++ b/tests/unit/test_routing.py @@ -35,7 +35,7 @@ def test_str_joins_the_three_names(self): def test_address_is_frozen(self): address = TrouveAddress.parse("a.b.c") with pytest.raises(ValidationError): - address.database_name = "other" + setattr(address, "database_name", "other") # noqa: B010 def test_address_is_hashable(self): assert len({TrouveAddress.parse("a.b.c"), TrouveAddress.parse("a.b.c")}) == 1 @@ -84,8 +84,9 @@ def test_a_subclass_keeps_its_own_fields(self): assert entry.database_name == "OMER_DEV" def test_pydantic_validates_a_subclass_field(self): + """An absent database_name is an error, not a silent default.""" with pytest.raises(ValidationError): - DatabaseOverrideRouting(environment_name="dev") + DatabaseOverrideRouting.model_validate({"environment_name": "dev"}) class TestRoutingTable: From 0357f95f3928a3b375f552044c042c9b0f9d38f5 Mon Sep 17 00:00:00 2001 From: OmerBaddour Date: Sun, 2 Aug 2026 18:21:27 -0400 Subject: [PATCH 4/4] chore: delete two duplicate memory files macOS made a copy of each file with " 2" in the name. A git add -A command then put the copies in the branch. The original files stay. --- .claude/memory/project_design_invariants 2.md | 27 ------------- .../project_docs_are_the_source_of_truth 2.md | 38 ------------------- 2 files changed, 65 deletions(-) delete mode 100644 .claude/memory/project_design_invariants 2.md delete mode 100644 .claude/memory/project_docs_are_the_source_of_truth 2.md diff --git a/.claude/memory/project_design_invariants 2.md b/.claude/memory/project_design_invariants 2.md deleted file mode 100644 index 97fbaca..0000000 --- a/.claude/memory/project_design_invariants 2.md +++ /dev/null @@ -1,27 +0,0 @@ ---- -name: project_design_invariants -description: Seven design rules that clair never breaks — read before you propose a new feature or a refactor -metadata: - type: project ---- - -These rules control all clair design work. A change that breaks one of them is wrong, -even if the code operates correctly. - -1. No Jinja. SQL is a plain Python f-string. -2. No YAML for configuration. YAML holds credentials only, in `~/.clair/environments.yml`. - All other configuration is Python. -3. The file path gives `database.schema.table`. Trouve files are three levels below the - project root. -4. `clair compile` makes no connection to Snowflake. It is a local operation only. -5. Validation is eager. An invalid Trouve raises at construction time, not at run time. -6. Shared SQL logic is a normal Python function, which you import normally. -7. All warehouse access goes through the `WarehouseAdapter` ABC in - `src/clair/adapters/base.py`. The runner must not import `SnowflakeAdapter` directly. - -**Why:** These rules are the product position against dbt and SQLMesh. Python-native, no -template language, no configuration language, full IDE support. - -**How to apply:** Before you write a new module or change an interface, compare your design -to this list. For feature behaviour that these rules do not cover, read `site_docs/` -(see [[project_docs_are_the_source_of_truth]]). diff --git a/.claude/memory/project_docs_are_the_source_of_truth 2.md b/.claude/memory/project_docs_are_the_source_of_truth 2.md deleted file mode 100644 index be2d989..0000000 --- a/.claude/memory/project_docs_are_the_source_of_truth 2.md +++ /dev/null @@ -1,38 +0,0 @@ ---- -name: project_docs_are_the_source_of_truth -description: Read site_docs/docs/ to learn clair behaviour — do not read src/ first, and do not copy docs content into memory -metadata: - type: project ---- - -`site_docs/docs/` is the source of truth for clair behaviour. It is Markdown, it is small -(approximately 9000 words), and `grep` finds content in it quickly. - -**Why:** Memory notes about features became wrong. An old note sent agents to -`src/clair/auth/environments.py`, a path that does not exist. The documentation is the -better source because users read it, so an error there gets a report. - -**But the documentation also rots.** The pandas guide, the landing page and the README -documented a `PandasTrouve` class that was never built, while the API reference correctly -documented the `df_fn` field that was. The pages came from a design spec, and nobody -changed them when the implementation took a different shape. mkdocs does not execute the -examples, so CI did not catch it. - -**Therefore: the code is the final authority.** Read the documentation first for -orientation, then confirm any API detail against `src/` or `example_projects/` before you -depend on it. When the two disagree, the code wins and the page is a bug. - -**How to apply:** - -- To learn what a feature does, `grep` `site_docs/docs/` first. Then confirm the exact API - against `src/` or a project in `example_projects/`. -- Search for the field name, not only the class name. The pandas feature was invisible to a - search for `PandasTrouve`, because the real name is `df_fn`. -- Map for orientation: `concepts/` (Trouve, DAG, project layout, environments), - `guides/` (routing, incrementality, tests, selectors, pandas, per-database config), - `cli/` (one page for each subcommand), `reference/` (API for Trouve, Column, RunConfig, - Tests). -- When you change behaviour, change the matching page in `site_docs/docs/` in the same PR. - Do not add a memory note that repeats the page. -- Write a memory note only for what the documentation cannot hold: a design rule - (see [[project_design_invariants]]) or a correction the user gave you.