From 243561804eaa6f49e424566637cae25ec177fcf7 Mon Sep 17 00:00:00 2001 From: Juan Miguel Contreras <19253629+juanmicl@users.noreply.github.com> Date: Thu, 3 Sep 2026 13:31:50 +0200 Subject: [PATCH 1/5] feat(cli): project hook sqlpush.py (dsn/metadata/chain-dir defaults) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Any verb run with a sqlpush.py in the CWD loads it by path (spec_from_file_location, never by module name) and uses it as the source of defaults: get_dsn() for --dsn/--ref-dsn, get_metadata() for the module:attribute positional (now optional on diff/check/push/revision — the object flows straight into the api calls, no import happens), CHAIN_DIR for --dir. Precedence: explicit flag > hook > $DATABASE_URL/current default. Missing or raising members fail as typed HookError (SqlpushError family) naming the file and member — exit 1 via main(), no traceback. The CWD is APPENDED to sys.path on discovery, never inserted at the front: a file named sqlpush.py must not shadow the installed package (pinned by a subprocess test replicating the console-script sys.path shape). Without a hook, behavior is identical to 0.5.1 (pinned). hook.py is pure stdlib + SqlpushError. --- CHANGELOG.md | 18 +++ src/sqlpush/cli.py | 98 +++++++++++----- src/sqlpush/hook.py | 84 ++++++++++++++ tests/test_hook.py | 270 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 444 insertions(+), 26 deletions(-) create mode 100644 src/sqlpush/hook.py create mode 100644 tests/test_hook.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 75f48ce..63eaccf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,24 @@ the project follows [Semantic Versioning](https://semver.org/). ## [Unreleased] +### Added + +- Project hook `sqlpush.py`: any verb run with a `sqlpush.py` in the + CWD discovers it (the alembic `env.py` / pytest `conftest.py` + pattern; loaded by path, never by module name) and uses it for + defaults — `get_dsn()` for `--dsn`/`--ref-dsn`, `get_metadata()` + for the `module:attribute` positional (now optional on + `diff`/`check`/`push`/`revision`), `CHAIN_DIR` for `--dir`. + Precedence is explicit flag > hook > `$DATABASE_URL`/current + default; a hook that is missing a needed member, or whose + `get_dsn()`/`get_metadata()` raises, fails as a typed + `SqlpushError` naming the file and the member (exit 1, no + traceback). The CWD is appended — never prepended — to `sys.path`, + so the hook file can never shadow the installed `sqlpush` package. + Without a hook every verb behaves exactly as before: `uv run + sqlpush revision -m "change"` from the project root now needs no + `--dsn`, no `module:attribute` and no PYTHONPATH. + ## [0.5.1] - 2026-09-02 ### Added diff --git a/src/sqlpush/cli.py b/src/sqlpush/cli.py index 1824dd8..f26e1c9 100644 --- a/src/sqlpush/cli.py +++ b/src/sqlpush/cli.py @@ -24,6 +24,7 @@ from sqlpush import api from sqlpush.core.render import render +from sqlpush.hook import hook_chain_dir, hook_dsn, hook_metadata, load_project_hook from sqlpush.types import ( CheckResult, MetadataImportError, @@ -38,8 +39,10 @@ SchemaOpt = Annotated[list[str] | None, typer.Option("--schema")] ExcludeOpt = Annotated[list[str] | None, typer.Option("--exclude")] # Path-typed options need the Annotated form: B008 (call in default) only -# exempts typer.Option for non-Path annotations -DirOpt = Annotated[Path, typer.Option("--dir")] +# exempts typer.Option for non-Path annotations. None default: lets the +# verb tell "user passed --dir" apart from "use hook/default" resolution. +DirOpt = Annotated[Path | None, typer.Option("--dir")] +DEFAULT_CHAIN_DIR = Path("migrations/versions") def _load_metadata(spec: str): @@ -53,7 +56,28 @@ def _load_metadata(spec: str): raise MetadataImportError(f"cannot import {spec!r}: {exc}") from exc -def _engine(dsn: str | None): +def _metadata(metadata_spec: str | None, hook): + """Resolve the metadata source: flag > hook.get_metadata() > usage error. + + The hook path returns the OBJECT itself (no module:attribute import + happens); the positional path is untouched. + """ + if metadata_spec is not None: + return _load_metadata(metadata_spec) + if hook is not None: + return hook_metadata(hook) + typer.secho( + "metadata spec required: pass module:attribute or add get_metadata() to ./sqlpush.py", + fg="red", + err=True, + ) + raise typer.Exit(code=2) + + +def _engine(dsn: str | None, hook=None): + # precedence: explicit flag > hook.get_dsn() > $DATABASE_URL > error + if dsn is None and hook is not None: + dsn = hook_dsn(hook) dsn = dsn or os.environ.get("DATABASE_URL") if not dsn: typer.secho("no --dsn and no DATABASE_URL set", fg="red", err=True) @@ -66,6 +90,13 @@ def _engine(dsn: str | None): raise typer.Exit(code=1) from exc +def _resolve_dir(out_dir: Path | None, hook) -> Path: + """Resolve --dir: explicit flag > hook.CHAIN_DIR > current default.""" + if out_dir is not None: + return out_dir + return Path(hook_chain_dir(hook, str(DEFAULT_CHAIN_DIR))) + + def _emit_json(plan) -> None: typer.echo(json.dumps(plan.to_json_dict(), indent=2)) @@ -81,7 +112,7 @@ def _risk_summary(p) -> None: @app.command() def diff( - metadata_spec: str = typer.Argument(..., help="module:metadata"), + metadata_spec: str | None = typer.Argument(None, help="module:metadata"), dsn: str | None = typer.Option(None), json_output: bool = typer.Option(False, "--json"), verbose: bool = typer.Option(False, "--verbose"), @@ -89,8 +120,9 @@ def diff( schema: SchemaOpt = None, exclude: ExcludeOpt = None, ): - md = _load_metadata(metadata_spec) - engine = _engine(dsn) + hook = load_project_hook() + md = _metadata(metadata_spec, hook) + engine = _engine(dsn, hook) try: p = api.plan(md, engine, schemas=schema, exclude=exclude or ()) if json_output: @@ -106,7 +138,7 @@ def diff( @app.command() def check( - metadata_spec: str = typer.Argument(...), + metadata_spec: str | None = typer.Argument(None), dsn: str | None = typer.Option(None), json_output: bool = typer.Option(False, "--json"), verbose: bool = typer.Option(False, "--verbose"), @@ -114,8 +146,9 @@ def check( schema: SchemaOpt = None, exclude: ExcludeOpt = None, ): - md = _load_metadata(metadata_spec) - engine = _engine(dsn) + hook = load_project_hook() + md = _metadata(metadata_spec, hook) + engine = _engine(dsn, hook) try: # plan ONCE and derive everything from that single object: a # second plan could observe a different DB state than the one @@ -140,7 +173,7 @@ def check( @app.command() def push( - metadata_spec: str = typer.Argument(...), + metadata_spec: str | None = typer.Argument(None), dsn: str | None = typer.Option(None), json_output: bool = typer.Option(False, "--json"), allow_destructive: bool = typer.Option(False, "--allow-destructive"), @@ -155,8 +188,9 @@ def push( schema: SchemaOpt = None, exclude: ExcludeOpt = None, ): - md = _load_metadata(metadata_spec) - engine = _engine(dsn) + hook = load_project_hook() + md = _metadata(metadata_spec, hook) + engine = _engine(dsn, hook) try: try: report = api.push( @@ -223,25 +257,35 @@ def push( @app.command() def revision( - metadata_spec: str = typer.Argument(..., help="module:metadata"), - ref_dsn: str = typer.Option(..., "--ref-dsn"), + metadata_spec: str | None = typer.Argument(None, help="module:metadata"), + ref_dsn: str | None = typer.Option(None, "--ref-dsn"), message: str | None = typer.Option(None, "--message", "-m"), - out_dir: DirOpt = Path("migrations/versions"), + out_dir: DirOpt = None, no_concurrently: bool = typer.Option(False, "--no-concurrently"), schema: SchemaOpt = None, exclude: ExcludeOpt = None, ): """Generate the next migration file from models vs the reference DB.""" - md = _load_metadata(metadata_spec) - # required --ref-dsn (no DATABASE_URL fallback): the reference DB is a - # different database from the push target — conflating them silently - # would chain against the wrong head + hook = load_project_hook() + md = _metadata(metadata_spec, hook) + # --ref-dsn: flag > hook.get_dsn(); NO DATABASE_URL fallback (the + # reference DB is a different database from the push target — + # conflating them silently would chain against the wrong head) + if ref_dsn is None and hook is not None: + ref_dsn = hook_dsn(hook) + if ref_dsn is None: + typer.secho( + "--ref-dsn is required (or add get_dsn() to ./sqlpush.py)", + fg="red", + err=True, + ) + raise typer.Exit(code=2) engine = _engine(ref_dsn) try: path = api.revision( md, engine, - out_dir=out_dir, + out_dir=_resolve_dir(out_dir, hook), message=message, concurrently=not no_concurrently, schemas=schema, @@ -260,14 +304,15 @@ def migrate( advisory_wait: float = typer.Option(30.0, "--advisory-wait"), lock_timeout: float = typer.Option(5.0, "--lock-timeout"), statement_timeout: float | None = typer.Option(None, "--statement-timeout"), - out_dir: DirOpt = Path("migrations/versions"), + out_dir: DirOpt = None, ): """Replay pending migration files (gates + checksum bookkeeping).""" - engine = _engine(dsn) + hook = load_project_hook() + engine = _engine(dsn, hook) try: report = api.migrate( engine, - chain_dir=out_dir, + chain_dir=_resolve_dir(out_dir, hook), allow_destructive=allow_destructive, advisory_wait=advisory_wait, lock_timeout=lock_timeout, @@ -290,12 +335,13 @@ def migrate( def stamp( dsn: str | None = typer.Option(None), force: bool = typer.Option(False, "--force"), - out_dir: DirOpt = Path("migrations/versions"), + out_dir: DirOpt = None, ): """Adopt an existing DB: register chain files without executing SQL.""" - engine = _engine(dsn) + hook = load_project_hook() + engine = _engine(dsn, hook) try: - report = api.stamp(engine, chain_dir=out_dir, force=force) + report = api.stamp(engine, chain_dir=_resolve_dir(out_dir, hook), force=force) finally: engine.dispose() typer.echo( diff --git a/src/sqlpush/hook.py b/src/sqlpush/hook.py new file mode 100644 index 0000000..9b7adc8 --- /dev/null +++ b/src/sqlpush/hook.py @@ -0,0 +1,84 @@ +# src/sqlpush/hook.py +"""Project hook: ``sqlpush.py`` in the CWD. + +The alembic ``env.py`` / pytest ``conftest.py`` pattern: when a +``sqlpush.py`` exists next to where the user runs the CLI, it becomes +the source of defaults for metadata, DSN and chain dir. Explicit flags +always win (flag > hook > env/default); without the hook every verb +behaves exactly as before. + +Loading is BY PATH (``spec_from_file_location``), never by module +name: a file named ``sqlpush.py`` in the CWD would shadow the +installed package if the CWD came first on ``sys.path`` — so the CWD +is APPENDED (never ``insert(0)``), guaranteeing the package wins for +normal imports while the hook itself is only ever loaded explicitly. + +Only typed :class:`SqlpushError`-family errors escape this module, +always naming the file and the member involved. +""" + +from __future__ import annotations + +import importlib.util +import os +import sys +from pathlib import Path +from types import ModuleType + +from sqlpush.types import SqlpushError + +HOOK_FILENAME = "sqlpush.py" + + +class HookError(SqlpushError): + """The project hook is broken or incomplete — names file + member.""" + + +def load_project_hook() -> ModuleType | None: + """Load ``./sqlpush.py`` by path if present; ``None`` when absent. + + On success the CWD is appended to ``sys.path`` (never inserted at + the front — see the module docstring for the shadowing rationale). + Any import-time failure is re-typed as :class:`HookError`. + """ + path = Path.cwd() / HOOK_FILENAME + if not path.is_file(): + return None + sys.path.append(os.getcwd()) + spec = importlib.util.spec_from_file_location("sqlpush_project_hook", path) + if spec is None or spec.loader is None: + raise HookError(f"{HOOK_FILENAME}: cannot load (invalid module spec)") + module = importlib.util.module_from_spec(spec) + try: + spec.loader.exec_module(module) + except Exception as exc: + raise HookError(f"{HOOK_FILENAME}: import raised: {exc}") from exc + return module + + +def hook_dsn(hook: ModuleType) -> str: + """Call ``get_dsn()`` LAZILY — only when a verb actually needs it.""" + getter = getattr(hook, "get_dsn", None) + if getter is None: + raise HookError(f"{HOOK_FILENAME}: missing get_dsn()") + try: + return getter() + except Exception as exc: + raise HookError(f"{HOOK_FILENAME}: get_dsn() raised: {exc}") from exc + + +def hook_metadata(hook: ModuleType): + """Call ``get_metadata()`` LAZILY — returns the populated MetaData object.""" + getter = getattr(hook, "get_metadata", None) + if getter is None: + raise HookError(f"{HOOK_FILENAME}: missing get_metadata()") + try: + return getter() + except Exception as exc: + raise HookError(f"{HOOK_FILENAME}: get_metadata() raised: {exc}") from exc + + +def hook_chain_dir(hook: ModuleType | None, default: str) -> str: + """Read ``CHAIN_DIR`` as a module attribute; ``default`` when unset.""" + value = getattr(hook, "CHAIN_DIR", None) if hook is not None else None + return default if value is None else value diff --git a/tests/test_hook.py b/tests/test_hook.py new file mode 100644 index 0000000..3c59946 --- /dev/null +++ b/tests/test_hook.py @@ -0,0 +1,270 @@ +# tests/test_hook.py — project hook (sqlpush.py) discovery, precedence, +# typed errors and the package-shadowing pin. DB-free unless marked. +from __future__ import annotations + +import os +import subprocess +import sys +from pathlib import Path + +import pytest +from sqlalchemy import text +from typer.testing import CliRunner + +import sqlpush.cli +from sqlpush.cli import app +from sqlpush.types import MigrateReport, Plan, SqlpushError + +runner = CliRunner() +DSN = os.environ.get( + "SQLPUSH_TEST_DSN", "postgresql+psycopg://sqlpush:sqlpush@localhost:5433/sqlpush_test" +) +HOOK_DSN = "postgresql+psycopg://u:p@localhost:1/hookunit" +ENV_DSN = "postgresql+psycopg://u:p@localhost:1/envunit" +FLAG_DSN = "postgresql+psycopg://u:p@localhost:1/flagunit" + +# a parseable-but-never-connected DSN: create_engine is faked in the +# DB-free tests; only the string value matters +FULL_HOOK = ( + "from sqlalchemy import Column, Integer, MetaData, Table\n" + f"HOOK_DSN = {HOOK_DSN!r}\n" + "def get_dsn():\n" + " return HOOK_DSN\n" + "def get_metadata():\n" + " md = MetaData()\n" + " Table('hook_marker_tbl', md, Column('id', Integer, primary_key=True))\n" + " return md\n" + "CHAIN_DIR = 'migrations/chain'\n" +) + + +class _FakeEngine: + def dispose(self) -> None: + pass + + +def _capture_engine(monkeypatch, seen: dict) -> None: + def _create(dsn, **kw): + seen["dsn"] = dsn + return _FakeEngine() + + monkeypatch.setattr(sqlpush.cli, "create_engine", _create) + + +# --- the hook resolves dsn/metadata/dir ------------------------------------- + + +def test_hook_resolves_metadata_and_dsn_for_check(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / "sqlpush.py").write_text(FULL_HOOK) + seen: dict = {} + _capture_engine(monkeypatch, seen) + + def fake_plan(md, engine, **kw): + seen["md"] = md + return Plan() # clean → exit 0 + + monkeypatch.setattr(sqlpush.cli.api, "plan", fake_plan) + # NO --dsn, NO module:attribute positional, NO DATABASE_URL + monkeypatch.delenv("DATABASE_URL", raising=False) + r = runner.invoke(app, ["check"]) + assert r.exit_code == 0, r.output + assert seen["dsn"] == HOOK_DSN + # the metadata OBJECT came from the hook's get_metadata() + assert "hook_marker_tbl" in seen["md"].tables + + +def test_hook_chain_dir_and_flag_precedence_for_migrate(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / "sqlpush.py").write_text(FULL_HOOK) + seen: dict = {} + _capture_engine(monkeypatch, seen) + monkeypatch.setattr( + sqlpush.cli.api, + "migrate", + lambda *a, **k: seen.__setitem__("dir", k.get("chain_dir")) or MigrateReport(), + ) + r = runner.invoke(app, ["migrate"]) + assert r.exit_code == 0, r.output + assert seen["dir"] == Path("migrations/chain") # hook CHAIN_DIR default + explicit = tmp_path / "explicit" / "dir" + r2 = runner.invoke(app, ["migrate", "--dir", str(explicit)]) + assert r2.exit_code == 0 + assert seen["dir"] == explicit # explicit --dir flag beats the hook + + +def test_hook_revision_ref_dsn_and_chain_dir(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / "sqlpush.py").write_text(FULL_HOOK) + seen: dict = {} + _capture_engine(monkeypatch, seen) + monkeypatch.setattr( + sqlpush.cli.api, + "revision", + lambda md, engine, **k: seen.__setitem__("out_dir", k.get("out_dir")) or Path("x.sql"), + ) + # no --ref-dsn, no positional, no -m — everything from the hook + r = runner.invoke(app, ["revision"]) + assert r.exit_code == 0, r.output + assert seen["dsn"] == HOOK_DSN + assert seen["out_dir"] == Path("migrations/chain") + + +# --- precedence: flag > hook > env ------------------------------------------ + + +def test_dsn_precedence_flag_beats_hook_beats_env(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / "sqlpush.py").write_text(FULL_HOOK) + monkeypatch.setenv("DATABASE_URL", ENV_DSN) + seen: dict = {} + _capture_engine(monkeypatch, seen) + monkeypatch.setattr(sqlpush.cli.api, "plan", lambda md, engine, **kw: Plan()) + r = runner.invoke(app, ["check"]) + assert r.exit_code == 0 + assert seen["dsn"] == HOOK_DSN # hook > $DATABASE_URL + r2 = runner.invoke(app, ["check", "--dsn", FLAG_DSN]) + assert r2.exit_code == 0 + assert seen["dsn"] == FLAG_DSN # explicit flag > hook + + +def test_no_hook_env_fallback_unchanged(tmp_path, monkeypatch): + # without a hook: env var behaves exactly as today (flag > env) + monkeypatch.chdir(tmp_path) # no sqlpush.py here + monkeypatch.setenv("DATABASE_URL", ENV_DSN) + seen: dict = {} + _capture_engine(monkeypatch, seen) + monkeypatch.setattr(sqlpush.cli.api, "plan", lambda md, engine, **kw: Plan()) + monkeypatch.syspath_prepend(str(tmp_path)) + (tmp_path / "models.py").write_text("from sqlalchemy import MetaData\nmetadata = MetaData()\n") + r = runner.invoke(app, ["check", "models:metadata"]) + assert r.exit_code == 0 + assert seen["dsn"] == ENV_DSN + + +# --- shadowing pin: the package always wins --------------------------------- + + +def test_hook_never_shadows_package(tmp_path): + # spec point 2: a file named sqlpush.py in the CWD must never shadow + # the installed package. The CLI APPENDS the CWD to sys.path (never + # insert(0)); replicated here in a fresh subprocess (console-script + # shape: the CWD is not already at sys.path[0]). + (tmp_path / "sqlpush.py").write_text("def get_dsn():\n return 'postgresql://x'\n") + code = ( + "import sys, os\n" + "sys.path[:] = [p for p in sys.path if p not in ('', os.getcwd())]\n" + "from sqlpush.hook import load_project_hook\n" + "hook = load_project_hook()\n" + "assert hook is not None, 'hook must be discovered'\n" + "assert os.getcwd() in sys.path, 'cwd appended'\n" + "assert sys.path[-1] == os.getcwd(), 'appended LAST, never first'\n" + "import sqlpush\n" + "print(sqlpush.__file__)\n" + ) + # check=False: the returncode assert below is the failure signal + proc = subprocess.run( + [sys.executable, "-c", code], cwd=tmp_path, capture_output=True, text=True, check=False + ) + assert proc.returncode == 0, proc.stderr + resolved = Path(proc.stdout.strip()).resolve() + assert resolved != (tmp_path / "sqlpush.py").resolve() + assert resolved.name == "__init__.py" and resolved.parent.name == "sqlpush" + + +# --- typed errors: names the file and the member ----------------------------- + + +def test_hook_missing_get_dsn_errors_typed(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / "sqlpush.py").write_text("CHAIN_DIR = 'x'\n") # no get_dsn + r = runner.invoke(app, ["migrate"]) + assert r.exit_code == 1 + assert isinstance(r.exception, SqlpushError) + assert "sqlpush.py: missing get_dsn()" in str(r.exception) + + +def test_hook_missing_get_metadata_errors_typed(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / "sqlpush.py").write_text("def get_dsn():\n return 'postgresql://x'\n") + r = runner.invoke(app, ["check"]) # needs metadata, hook has none + assert r.exit_code == 1 + assert isinstance(r.exception, SqlpushError) + assert "sqlpush.py: missing get_metadata()" in str(r.exception) + + +def test_hook_get_dsn_raises_typed_with_cause(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / "sqlpush.py").write_text("def get_dsn():\n raise RuntimeError('boom-dsn')\n") + r = runner.invoke(app, ["migrate"]) + assert r.exit_code == 1 + assert isinstance(r.exception, SqlpushError) + assert "sqlpush.py: get_dsn() raised:" in str(r.exception) + assert "boom-dsn" in str(r.exception) + assert isinstance(r.exception.__cause__, RuntimeError) # not swallowed + + +def test_hook_get_metadata_raises_typed(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / "sqlpush.py").write_text( + "def get_dsn():\n return 'postgresql://x'\n" + "def get_metadata():\n raise RuntimeError('boom-md')\n" + ) + r = runner.invoke(app, ["check"]) + assert r.exit_code == 1 + assert isinstance(r.exception, SqlpushError) + assert "sqlpush.py: get_metadata() raised:" in str(r.exception) + assert "boom-md" in str(r.exception) + + +def test_hook_import_failure_typed(tmp_path, monkeypatch): + monkeypatch.chdir(tmp_path) + (tmp_path / "sqlpush.py").write_text("def broken(:\n") # syntax error + r = runner.invoke(app, ["migrate"]) + assert r.exit_code == 1 + assert isinstance(r.exception, SqlpushError) + assert "sqlpush.py" in str(r.exception) + + +def test_no_hook_missing_metadata_spec_exit_2(tmp_path, monkeypatch): + # backwards compat: no hook, no positional → same class of failure + # as today's missing-argument usage error (exit 2), message points + # at both remedies + monkeypatch.chdir(tmp_path) + monkeypatch.delenv("DATABASE_URL", raising=False) + r = runner.invoke(app, ["check", "--dsn", "postgresql+psycopg://u:p@localhost:1/x"]) + assert r.exit_code == 2 + assert "module:attribute" in r.stderr + assert "sqlpush.py" in r.stderr + + +# --- end-to-end against the dev DB ------------------------------------------- + +DSN_HOOK = ( + "from sqlalchemy import Column, Integer, MetaData, String, Table\n" + f"DSN = {DSN!r}\n" + "def get_dsn():\n" + " return DSN\n" + "def get_metadata():\n" + " md = MetaData()\n" + " Table('hook_hero', md, Column('id', Integer, primary_key=True), " + "Column('name', String(50)))\n" + " return md\n" +) + + +@pytest.mark.pg +def test_hook_end_to_end_check_clean(tmp_path, monkeypatch, pg_engine): + # the full contract on a real DB: hook resolves dsn AND metadata, + # zero CLI inputs, in-sync schema → check exits 0 + monkeypatch.chdir(tmp_path) + (tmp_path / "sqlpush.py").write_text(DSN_HOOK) + with pg_engine.begin() as conn: + conn.execute(text("DROP TABLE IF EXISTS hook_hero")) + conn.execute(text("CREATE TABLE hook_hero (id integer PRIMARY KEY, name varchar(50))")) + try: + r = runner.invoke(app, ["check"]) + assert r.exit_code == 0, r.output + finally: + with pg_engine.begin() as conn: + conn.execute(text("DROP TABLE IF EXISTS hook_hero")) From 4948e368ad9d509070496277bf1965b91df59227 Mon Sep 17 00:00:00 2001 From: Juan Miguel Contreras <19253629+juanmicl@users.noreply.github.com> Date: Thu, 3 Sep 2026 13:33:04 +0200 Subject: [PATCH 2/5] docs(readme): Project hook section Complete sqlpush.py example (get_metadata/get_dsn/CHAIN_DIR), the precedence table (flag > hook > env/default, including which inputs each verb resolves), the typed-error contract, and the sys.path shadowing note (append, never prepend). The knobs table now marks --ref-dsn as required only without a hook. --- README.md | 50 ++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 8c0f047..0bfeb8c 100644 --- a/README.md +++ b/README.md @@ -167,6 +167,47 @@ run under `psql`. Schema change and data backfill ship as one file. The [chain guide](docs/the-chain.md) covers the format, the gates and the workflows. +## Project hook + +Drop a `sqlpush.py` in your repo root (the alembic `env.py` / +pytest `conftest.py` pattern) and the CLI stops needing flags, +specs and env vars: + +```python +# sqlpush.py — in your repo root +def get_metadata(): # REQUIRED for diff/check/push/revision + from myapp.models import metadata + return metadata # -> a populated MetaData + +def get_dsn() -> str: # REQUIRED for every verb + from myapp.settings import DATABASE_URL + return DATABASE_URL # -> a full psycopg DSN + +CHAIN_DIR = "migrations/chain" # OPTIONAL — default for --dir +``` + +With that file in place, `uv run sqlpush revision -m "change"` just +works: no `--dsn`, no `module:attribute`, no credentials on the +command line, no PYTHONPATH. Inputs resolve with a fixed precedence: + +| input | explicit flag | `sqlpush.py` | fallback | +| --- | --- | --- | --- | +| `--dsn` / `--ref-dsn` | wins | `get_dsn()` | `$DATABASE_URL` (not for `--ref-dsn`) | +| `module:attribute` (diff/check/push/revision) | wins | `get_metadata()` | usage error | +| `--dir` (revision/migrate/stamp) | wins | `CHAIN_DIR` | `migrations/versions` | + +A hook that is missing a member a verb needs — or whose +`get_dsn()`/`get_metadata()` raises — fails with a typed error naming +the file and the member (`sqlpush.py: missing get_dsn()`), never a +traceback. Without a `sqlpush.py`, every verb behaves exactly as +before. + +One deliberate detail: on discovering the hook, sqlpush **appends** +your CWD to `sys.path` instead of prepending it. A file named +`sqlpush.py` would otherwise shadow the installed package the moment +you ran the CLI from your repo root; appending guarantees the real +package always wins, and the hook is loaded by path only. + ## Exit codes | verb | 0 | 1 | 2 | 3 | @@ -194,13 +235,14 @@ The knobs, per verb: | verb | flags | | --- | --- | | `push` | `--allow-destructive` `--safe-only` `--no-lock` `--lock-timeout` `--advisory-wait` `--no-concurrently` `--statement-timeout` | -| `revision` | `--ref-dsn` (required) `-m/--message` `--no-concurrently` `--dir` | +| `revision` | `--ref-dsn` (required without a hook) `-m/--message` `--no-concurrently` `--dir` | | `migrate` | `--allow-destructive` `--advisory-wait` `--lock-timeout` `--statement-timeout` `--dir` | | `stamp` | `--force` `--dir` | -Every verb except `revision` takes `--dsn` (or `$DATABASE_URL`). -`revision` requires `--ref-dsn`, with no env fallback: the reference -DB is a different database from the push target. `diff`, `check`, +Every verb except `revision` takes `--dsn` (or `$DATABASE_URL`, or +the project hook's `get_dsn()`). `revision` takes `--ref-dsn` — or +the hook — with no env fallback: the reference DB is a different +database from the push target. `diff`, `check`, `push` and `revision` also take repeatable `--schema` / `--exclude`. Timeouts are seconds; a `lock_timeout` bounds how long a statement waits on a lock before failing, `statement_timeout` bounds each From 01829bbfdcbed23a632a4f1856c1f582e0b9761e Mon Sep 17 00:00:00 2001 From: Juan Miguel Contreras <19253629+juanmicl@users.noreply.github.com> Date: Thu, 3 Sep 2026 13:34:08 +0200 Subject: [PATCH 3/5] style(readme): canonical ruff format for the hook code block ruff 0.16 formats Python fences inside Markdown; the Project hook example needed two-space comment spacing and blank lines between top-level defs. --- README.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 0bfeb8c..bdc4159 100644 --- a/README.md +++ b/README.md @@ -175,15 +175,19 @@ specs and env vars: ```python # sqlpush.py — in your repo root -def get_metadata(): # REQUIRED for diff/check/push/revision +def get_metadata(): # REQUIRED for diff/check/push/revision from myapp.models import metadata - return metadata # -> a populated MetaData -def get_dsn() -> str: # REQUIRED for every verb + return metadata # -> a populated MetaData + + +def get_dsn() -> str: # REQUIRED for every verb from myapp.settings import DATABASE_URL - return DATABASE_URL # -> a full psycopg DSN -CHAIN_DIR = "migrations/chain" # OPTIONAL — default for --dir + return DATABASE_URL # -> a full psycopg DSN + + +CHAIN_DIR = "migrations/chain" # OPTIONAL — default for --dir ``` With that file in place, `uv run sqlpush revision -m "change"` just From 458d7c878b846fe8004754a17864ec610618d900 Mon Sep 17 00:00:00 2001 From: Juan Miguel Contreras <19253629+juanmicl@users.noreply.github.com> Date: Thu, 3 Sep 2026 13:39:47 +0200 Subject: [PATCH 4/5] docs(readme): hook DSN fallback precise (only without a hook) --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index bdc4159..554a031 100644 --- a/README.md +++ b/README.md @@ -196,7 +196,7 @@ command line, no PYTHONPATH. Inputs resolve with a fixed precedence: | input | explicit flag | `sqlpush.py` | fallback | | --- | --- | --- | --- | -| `--dsn` / `--ref-dsn` | wins | `get_dsn()` | `$DATABASE_URL` (not for `--ref-dsn`) | +| `--dsn` / `--ref-dsn` | wins | `get_dsn()` | `$DATABASE_URL`, only without a hook and never for `--ref-dsn` | | `module:attribute` (diff/check/push/revision) | wins | `get_metadata()` | usage error | | `--dir` (revision/migrate/stamp) | wins | `CHAIN_DIR` | `migrations/versions` | From 41b3deddb64dc2f59be269f9b23a9c2901cf79a8 Mon Sep 17 00:00:00 2001 From: Juan Miguel Contreras <19253629+juanmicl@users.noreply.github.com> Date: Thu, 3 Sep 2026 13:43:31 +0200 Subject: [PATCH 5/5] test(hook): pin revision env-isolation, hook laziness, fresh-import shadowing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three review nits as test pins: - revision env-isolation: $DATABASE_URL set + no hook + no --ref-dsn must exit 2 with the remedy, and create_engine must never see the env DSN — guards the cli.py ordering (ref_dsn checked None before _engine) against a silent refactor. Mutation-checked: removing the guard so _engine falls back to the env var fails the pin. - shadowing: the subprocess script now drops sys.modules['sqlpush'] and re-imports — the fresh PATH-ORDER resolution (CWD appended last) must still find the installed package, proving the original assert was not a sys.modules cache short-circuit. - laziness: migrate with a hook whose get_metadata RAISES (get_dsn fine, empty chain dir) is a clean idle run — get_metadata() is never called for verbs that do not need it. --- tests/test_hook.py | 63 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 59 insertions(+), 4 deletions(-) diff --git a/tests/test_hook.py b/tests/test_hook.py index 3c59946..a5d058e 100644 --- a/tests/test_hook.py +++ b/tests/test_hook.py @@ -142,6 +142,26 @@ def test_no_hook_env_fallback_unchanged(tmp_path, monkeypatch): assert seen["dsn"] == ENV_DSN +def test_revision_env_isolation_no_hook_no_ref_dsn(tmp_path, monkeypatch): + # pin: $DATABASE_URL must NEVER leak into the reference DSN. With + # no hook and no --ref-dsn, revision refuses with the remedy even + # though the env var is set — today this holds because cli.py + # checks ref_dsn is None BEFORE _engine is ever consulted; a + # refactor that reorders resolution would silently chain against + # the push target's env DSN (the wrong head). + monkeypatch.chdir(tmp_path) # no sqlpush.py + monkeypatch.setenv("DATABASE_URL", ENV_DSN) + monkeypatch.syspath_prepend(str(tmp_path)) + (tmp_path / "models.py").write_text("from sqlalchemy import MetaData\nmetadata = MetaData()\n") + seen: dict = {} + _capture_engine(monkeypatch, seen) # engine must never be created + r = runner.invoke(app, ["revision", "models:metadata"]) + assert r.exit_code == 2 + assert "--ref-dsn" in r.stderr + assert "sqlpush.py" in r.stderr # the remedy names the hook too + assert "dsn" not in seen # the env var never reached create_engine + + # --- shadowing pin: the package always wins --------------------------------- @@ -149,7 +169,10 @@ def test_hook_never_shadows_package(tmp_path): # spec point 2: a file named sqlpush.py in the CWD must never shadow # the installed package. The CLI APPENDS the CWD to sys.path (never # insert(0)); replicated here in a fresh subprocess (console-script - # shape: the CWD is not already at sys.path[0]). + # shape: the CWD is not already at sys.path[0]). The second half + # drops the package from sys.modules and re-imports: with the CWD + # appended LAST the fresh PATH-ORDER resolution must still find the + # package — proving it is not a sys.modules cache short-circuit. (tmp_path / "sqlpush.py").write_text("def get_dsn():\n return 'postgresql://x'\n") code = ( "import sys, os\n" @@ -161,15 +184,21 @@ def test_hook_never_shadows_package(tmp_path): "assert sys.path[-1] == os.getcwd(), 'appended LAST, never first'\n" "import sqlpush\n" "print(sqlpush.__file__)\n" + "del sys.modules['sqlpush']\n" + "import sqlpush\n" + "print(sqlpush.__file__)\n" ) # check=False: the returncode assert below is the failure signal proc = subprocess.run( [sys.executable, "-c", code], cwd=tmp_path, capture_output=True, text=True, check=False ) assert proc.returncode == 0, proc.stderr - resolved = Path(proc.stdout.strip()).resolve() - assert resolved != (tmp_path / "sqlpush.py").resolve() - assert resolved.name == "__init__.py" and resolved.parent.name == "sqlpush" + resolved_lines = [ln for ln in proc.stdout.strip().splitlines() if ln] + assert len(resolved_lines) == 2 # cached import + fresh re-import + for line in resolved_lines: + resolved = Path(line).resolve() + assert resolved != (tmp_path / "sqlpush.py").resolve() + assert resolved.name == "__init__.py" and resolved.parent.name == "sqlpush" # --- typed errors: names the file and the member ----------------------------- @@ -268,3 +297,29 @@ def test_hook_end_to_end_check_clean(tmp_path, monkeypatch, pg_engine): finally: with pg_engine.begin() as conn: conn.execute(text("DROP TABLE IF EXISTS hook_hero")) + + +@pytest.mark.pg +def test_hook_laziness_get_metadata_never_called_for_migrate(tmp_path, monkeypatch, pg_engine): + # laziness pin: migrate never needs metadata, so get_metadata() is + # never called — a hook whose get_metadata RAISES still migrates + # cleanly (empty chain dir → idle run: versions table ensured, + # nothing applied). If resolution were eager, this would die with + # "sqlpush.py: get_metadata() raised". + monkeypatch.chdir(tmp_path) + (tmp_path / "sqlpush.py").write_text( + f"DSN = {DSN!r}\n" + "def get_dsn():\n" + " return DSN\n" + "def get_metadata():\n" + " raise RuntimeError('get_metadata must not be called')\n" + ) + (tmp_path / "chain").mkdir() # empty: legitimate idle migrate + try: + r = runner.invoke(app, ["migrate", "--dir", str(tmp_path / "chain")]) + assert r.exit_code == 0, r.output + assert "applied: 0" in r.output + finally: + # the idle run ensures the versions table in the shared dev DB + with pg_engine.begin() as conn: + conn.execute(text("DROP TABLE IF EXISTS sqlpush_versions"))