From 5f7dadfe5e08a0dd23c75c89e5d232b7278214e4 Mon Sep 17 00:00:00 2001 From: Adrian Ehrsam Date: Fri, 17 Jul 2026 14:55:22 +0200 Subject: [PATCH] Use the Docker API instead of shelling out to podman; fix schema-apply retry and dependency parsing - container.py: replace subprocess calls to the podman CLI with the docker package's client (docker.from_env(), falling back to Podman's rootful/rootless socket). Works against a real Docker daemon or Podman transparently, and needs no CLI binary or CI setup step - ubuntu-latest's preinstalled Docker daemon just works. - schema.py apply_schema(): loop the failed-file retry until a full pass makes no more progress, instead of a single extra pass in original order - a multi-level dependency chain (e.g. view -> view -> table) could still raise on an item whose dependency hadn't had its own retry yet. - schema.py _get_sql_deps(): pass error_level=IGNORE to sqlglot.parse() so one statement it can't fully parse (e.g. a schema-qualified DROP TRIGGER ... ON schema.table) doesn't lose every other statement's dependency info in the same file: fall back further to a regex scan if sqlglot still raises. This only feeds ordering, not execution, so an approximation is an acceptable fallback. --- .github/workflows/python-test.yml | 2 - README.md | 15 +++-- pgdevkit/testdb/container.py | 101 +++++++++++++++++++----------- pgdevkit/testdb/schema.py | 45 ++++++++++++- pyproject.toml | 1 + skills/pgdevkit/SKILL.md | 4 +- tests/testdb/conftest.py | 14 ++++- tests/testdb/test_container.py | 12 ++-- uv.lock | 29 +++++++++ 9 files changed, 165 insertions(+), 58 deletions(-) diff --git a/.github/workflows/python-test.yml b/.github/workflows/python-test.yml index e10c7d3..a59f3d5 100644 --- a/.github/workflows/python-test.yml +++ b/.github/workflows/python-test.yml @@ -24,8 +24,6 @@ jobs: uses: actions/setup-python@v5 with: python-version: ${{ matrix.python-version }} - - name: Ensure podman is installed - run: command -v podman || (sudo apt-get update && sudo apt-get install -y podman) - name: Install uv run: curl -LsSf https://astral.sh/uv/install.sh | sh - name: Install project dependencies diff --git a/README.md b/README.md index 2b3de24..0e21d4f 100644 --- a/README.md +++ b/README.md @@ -70,15 +70,20 @@ CLI: `pgdb testdb up|reset|run-sql|status|shell|clean`. Container connection defaults (`localhost:54322`, `postgres`/`testpwd`) can be overridden with `PGDEVKIT_TESTDB_HOST`, `PGDEVKIT_TESTDB_PORT`, -`PGDEVKIT_TESTDB_USER`, `PGDEVKIT_TESTDB_PASSWORD`. Before touching -podman/docker, pgdevkit first checks (with a short timeout) whether Postgres +`PGDEVKIT_TESTDB_USER`, `PGDEVKIT_TESTDB_PASSWORD`. Before touching the +Docker API, pgdevkit first checks (with a short timeout) whether Postgres is already reachable at that address and skips container management if so. Set `PGDEVKIT_SKIP_CONTAINER=1` to always assume it's already there and skip that check too. -To point at a local Postgres install instead of the podman container — -useful when podman isn't available, or you'd rather use peer authentication -as the current OS user — set `PGDEVKIT_TESTDB_HOST` to the unix socket +Container management goes through the Docker API (the `docker` package, +`docker.from_env()`, falling back to Podman's rootful/rootless socket) — it +works against a real Docker daemon or Podman transparently, no CLI binary +required either way. + +To point at a local Postgres install instead of the container — useful when +neither is available, or you'd rather use peer authentication as the +current OS user — set `PGDEVKIT_TESTDB_HOST` to the unix socket directory (e.g. `/var/run/postgresql`) and `PGDEVKIT_TESTDB_PASSWORD=""`. The role named by `PGDEVKIT_TESTDB_USER` must exist and match your OS user (`CREATE ROLE SUPERUSER LOGIN;`) and `pg_hba.conf` must allow `peer` diff --git a/pgdevkit/testdb/container.py b/pgdevkit/testdb/container.py index 2319f23..cad0e93 100644 --- a/pgdevkit/testdb/container.py +++ b/pgdevkit/testdb/container.py @@ -1,23 +1,53 @@ from __future__ import annotations import os -import subprocess import time +import docker +import docker.errors import psycopg from . import constants +# Candidate Docker-API-compatible socket URLs tried after plain +# docker.from_env() (which only looks at DOCKER_HOST / the default Docker +# socket) fails to connect -- covers rootful and rootless Podman, which +# speaks the same API but doesn't always advertise itself via DOCKER_HOST. +_FALLBACK_SOCKET_URLS = [ + f"unix://{os.environ['XDG_RUNTIME_DIR']}/podman/podman.sock" if os.environ.get("XDG_RUNTIME_DIR") else None, + "unix:///run/podman/podman.sock", +] -def _podman(*args: str, check: bool = True) -> subprocess.CompletedProcess: - return subprocess.run(["podman", *args], capture_output=True, text=True, check=check) + +def _client() -> docker.DockerClient: + """A Docker-API client, working against a real Docker daemon or a + Podman one (Podman exposes the same API over its own socket) -- callers + never need to know or care which one is actually running.""" + try: + client = docker.from_env() + client.ping() + return client + except Exception: # noqa: BLE001 + pass + for base_url in _FALLBACK_SOCKET_URLS: + if base_url is None: + continue + try: + client = docker.DockerClient(base_url=base_url) + client.ping() + return client + except Exception: # noqa: BLE001 + continue + raise RuntimeError( + "Could not reach a Docker-compatible API. Set DOCKER_HOST, or make sure " + "Docker or Podman's API socket is running." + ) def _available(timeout: float = 3.0) -> bool: """Quick check (short timeout) for whether Postgres is already reachable at HOST:PORT, so a database started outside pgdevkit's control (or the - container from a previous run) doesn't trigger another podman/docker - lifecycle call.""" + container from a previous run) doesn't trigger another Docker API call.""" # libpq's connect_timeout is whole seconds; anything below 1 means "wait # indefinitely" instead of a short timeout, so it's clamped up to 1. connect_timeout = max(1, round(timeout)) @@ -29,32 +59,24 @@ def _available(timeout: float = 3.0) -> bool: return False -def _container_status() -> str | None: - """Return 'running', 'exited', etc., or None if the container doesn't exist.""" - result = _podman( - "inspect", constants.CONTAINER_NAME, "--format", "{{.State.Status}}", check=False - ) - if result.returncode != 0: - return None - return result.stdout.strip() - - -def _create_container() -> None: - result = _podman( - "run", "-d", - "--name", constants.CONTAINER_NAME, - "-p", f"{constants.PORT}:5432", - "-e", f"POSTGRES_USER={constants.USER}", - "-e", f"POSTGRES_PASSWORD={constants.PASSWORD}", - constants.IMAGE, - "postgres", *constants.PG_SPEED_FLAGS, - check=False, - ) - if result.returncode != 0 and "already in use" in result.stderr: - _podman("start", constants.CONTAINER_NAME) - return - if result.returncode != 0: - raise RuntimeError(f"podman run failed: {result.stderr}") +def _create_container(client: docker.DockerClient) -> None: + try: + client.containers.run( + constants.IMAGE, + name=constants.CONTAINER_NAME, + detach=True, + ports={"5432/tcp": constants.PORT}, + environment={ + "POSTGRES_USER": constants.USER, + "POSTGRES_PASSWORD": constants.PASSWORD, + }, + command=["postgres", *constants.PG_SPEED_FLAGS], + ) + except docker.errors.APIError as e: + if getattr(e, "status_code", None) == 409 or "already in use" in str(e): + client.containers.get(constants.CONTAINER_NAME).start() + return + raise RuntimeError(f"Starting the {constants.CONTAINER_NAME} container failed: {e}") from e def _wait_ready(timeout: float = 30.0) -> None: @@ -73,17 +95,20 @@ def _wait_ready(timeout: float = 30.0) -> None: def ensure_container() -> None: """Idempotently ensure the shared pgdevkit-postgres container is running - and accepting connections. Never touches podman/docker if Postgres is + and accepting connections. Never touches the Docker API if Postgres is already reachable, or if PGDEVKIT_SKIP_CONTAINER says to assume it is.""" if os.environ.get("PGDEVKIT_SKIP_CONTAINER"): return if _available(): return - status = _container_status() - if status == "running": - return - if status is not None: - _podman("start", constants.CONTAINER_NAME) + client = _client() + try: + container = client.containers.get(constants.CONTAINER_NAME) + except docker.errors.NotFound: + container = None + if container is not None: + if container.status != "running": + container.start() else: - _create_container() + _create_container(client) _wait_ready() diff --git a/pgdevkit/testdb/schema.py b/pgdevkit/testdb/schema.py index 3b9bb99..a0bf797 100644 --- a/pgdevkit/testdb/schema.py +++ b/pgdevkit/testdb/schema.py @@ -63,8 +63,31 @@ def _strip_layer_prefix(schema_name: str) -> str: } +_DECLARE_RE = re.compile( + r"CREATE\s+(?:OR\s+REPLACE\s+)?(?:TABLE|VIEW|FUNCTION|PROCEDURE|TYPE|SCHEMA)\s+(\w+\.\w+)", re.IGNORECASE +) +_DEPEND_RE = re.compile(r"(?:FROM|JOIN|INTO|UPDATE|TABLE|ON)\s+(\w+\.\w+)", re.IGNORECASE) + + +def _get_sql_deps_regex_fallback(sql: str) -> set[str]: + """Crude regex scan used when sqlglot can't parse a statement even with + error_level=IGNORE. This only feeds dependency *ordering* (which file to + apply first), not execution, so an imprecise-but-safe approximation here + is fine.""" + declares = set(_DECLARE_RE.findall(sql)) + deps = set(_DEPEND_RE.findall(sql)) + return deps - declares + + def _get_sql_deps(sql: str) -> set[str]: - exprs = sqlglot.parse(sql, dialect="postgres") + try: + # error_level=IGNORE lets sqlglot recover from statements it can't + # fully parse (e.g. a schema-qualified `DROP TRIGGER ... ON + # schema.table`) and keep going, instead of raising and losing every + # other statement's dependency info in the same file. + exprs = sqlglot.parse(sql, dialect="postgres", error_level=sqlglot.ErrorLevel.IGNORE) + except Exception: # noqa: BLE001 + return _get_sql_deps_regex_fallback(sql) deps: set[str] = set() for e in exprs: if e is None: @@ -206,5 +229,21 @@ async def _apply(file: Path, sql: str) -> None: logger.warning("Error executing %s (will retry): %s", file, e) failures.append((file, sql)) - for file, sql in failures: - await _apply(file, sql) + # Retry the whole failed set, not just once: a single extra pass in + # original order can still raise on an item whose dependency is later + # in the same list and hasn't had its own retry yet. Every CREATE here + # is idempotent, so looping until a full pass makes no progress + # converges on any resolvable ordering without re-doing finished work. + while failures: + still_failing: list[tuple[Path, str]] = [] + last_error: Exception | None = None + for file, sql in failures: + try: + await _apply(file, sql) + except Exception as e: # noqa: BLE001 + still_failing.append((file, sql)) + last_error = e + if len(still_failing) == len(failures): + assert last_error is not None + raise last_error + failures = still_failing diff --git a/pyproject.toml b/pyproject.toml index f0ecbf3..8e8d334 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -16,6 +16,7 @@ description = "A helper for developing with Postgres" readme = "README.md" requires-python = ">=3.14" dependencies = [ + "docker>=7.1.0", "psycopg[binary]>=3.2.0", "sqlglot>=30.11.0", ] diff --git a/skills/pgdevkit/SKILL.md b/skills/pgdevkit/SKILL.md index f7a79f7..8c0e03a 100644 --- a/skills/pgdevkit/SKILL.md +++ b/skills/pgdevkit/SKILL.md @@ -73,7 +73,7 @@ def _testdb_env(): os.environ[key] = value ``` -`ensure_testdb()` starts the shared `pgdevkit-postgres` container if needed (via `podman`), creates a database scoped to this project+branch (so different worktrees/branches never collide), and applies every `.sql` file under `database_dir` in dependency order, seeding any `.test_data.json` sidecar files. +`ensure_testdb()` starts the shared `pgdevkit-postgres` container if needed (via the Docker API — works against a real Docker daemon or Podman's socket, no CLI binary required), creates a database scoped to this project+branch (so different worktrees/branches never collide), and applies every `.sql` file under `database_dir` in dependency order, seeding any `.test_data.json` sidecar files. **`migrations/` is never applied here, on purpose.** If the test schema is missing something, that's a sign the base `tables/`/`views`/... file has drifted behind a migration that was only ever run manually against a real database — fix the base file, don't add migration-replay to `apply_schema()` (tried once, reverted: a migration can't be judged "safe to re-run" from its SQL text alone — see `docs/database-layout.md`'s Migrations section). @@ -88,7 +88,7 @@ def _testdb_env(): ### CI -Podman needs to be available on the runner (`apt-get install -y podman` on `ubuntu-latest` if not preinstalled) — `ensure_testdb()`/`ensure_container()` shell out to it directly. There's no `PGDEVKIT_SKIP_CONTAINER` + service-container escape hatch wired through every fixture yet — if a project's CI can't run podman, it needs its own workaround for now. +`ensure_testdb()`/`ensure_container()` talk to whatever Docker-compatible API is reachable (`DOCKER_HOST`, the default Docker socket, or Podman's socket as a fallback) — `ubuntu-latest`'s preinstalled Docker daemon just works, no setup step needed. There's no `PGDEVKIT_SKIP_CONTAINER` + service-container escape hatch wired through every fixture yet — if a project's CI has neither Docker nor Podman reachable, it needs its own workaround for now. --- diff --git a/tests/testdb/conftest.py b/tests/testdb/conftest.py index c8c6260..790bf5c 100644 --- a/tests/testdb/conftest.py +++ b/tests/testdb/conftest.py @@ -1,15 +1,25 @@ from __future__ import annotations import os -import shutil import subprocess from pathlib import Path from typing import Callable import pytest + +def _has_container_runtime() -> bool: + from pgdevkit.testdb.container import _client + + try: + _client() + return True + except Exception: # noqa: BLE001 + return False + + requires_podman = pytest.mark.skipif( - shutil.which("podman") is None, reason="podman is not installed" + not _has_container_runtime(), reason="no Docker-compatible API reachable" ) FIXTURES = Path(__file__).parent / "fixtures" / "database" diff --git a/tests/testdb/test_container.py b/tests/testdb/test_container.py index 754a573..2993bdb 100644 --- a/tests/testdb/test_container.py +++ b/tests/testdb/test_container.py @@ -32,10 +32,10 @@ def test_ensure_container_skips_podman_when_already_available(monkeypatch): monkeypatch.setattr(container, "_available", lambda timeout=1.0: True) def _fail(*a, **k): - raise AssertionError("must not shell out to podman when already available") + raise AssertionError("must not touch the Docker API when already available") - monkeypatch.setattr(container, "_podman", _fail) - ensure_container() # must return without calling podman + monkeypatch.setattr(container, "_client", _fail) + ensure_container() # must return without calling the Docker API def test_ensure_container_skips_everything_when_skip_env_set(monkeypatch): @@ -45,8 +45,8 @@ def _fail(*a, **k): raise AssertionError("must not run when PGDEVKIT_SKIP_CONTAINER is set") monkeypatch.setattr(container, "_available", _fail) - monkeypatch.setattr(container, "_podman", _fail) - ensure_container() # must return without checking availability or calling podman + monkeypatch.setattr(container, "_client", _fail) + ensure_container() # must return without checking availability or calling the Docker API def _admin_dsn() -> str: @@ -78,7 +78,7 @@ def test_ensure_container_is_idempotent(): def test_create_container_falls_back_to_start_when_name_in_use(): ensure_container() # container already exists under constants.CONTAINER_NAME - _create_container() # "podman run" will fail with "already in use"; must fall back to "podman start" + _create_container(container._client()) # run will conflict; must fall back to start() with psycopg.connect(_admin_dsn()) as con: with con.cursor() as cur: diff --git a/uv.lock b/uv.lock index 7385f1c..108689d 100644 --- a/uv.lock +++ b/uv.lock @@ -191,6 +191,20 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/c2/e6/f60198ea8d9dfa15fff9ed4ca02ce362f6eadd9ba757dcc50634c4257b63/cryptography-49.0.0-cp39-abi3-win_amd64.whl", hash = "sha256:026ac7423e6fa66872d3bf889be5974507da3944f866f704fa200eadacd00001", size = 3785547, upload-time = "2026-06-12T20:02:26.847Z" }, ] +[[package]] +name = "docker" +version = "7.2.0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "pywin32", marker = "sys_platform == 'win32'" }, + { name = "requests" }, + { name = "urllib3" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/88/7f/731ff914b0255d3d065f45fd4e626d4b8c95dbcbaada049f337a6ac16410/docker-7.2.0.tar.gz", hash = "sha256:cebb93773d334f778e023a7ee352a8d6e13ab1bd3b863a4d4a59dec897df43ac", size = 118731, upload-time = "2026-07-09T14:53:46.39Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/75/23/529140fe1aab80fc6992f93a706deec709140a6397439139a054e1515c45/docker-7.2.0-py3-none-any.whl", hash = "sha256:a3f45fdeb9165e2d25d9a1d02ddf3bc70fb572cf5ebbf9b58558c22caf29b71f", size = 148775, upload-time = "2026-07-09T14:53:45.224Z" }, +] + [[package]] name = "idna" version = "3.18" @@ -270,6 +284,7 @@ name = "pgdevkit" version = "0.2.2" source = { editable = "." } dependencies = [ + { name = "docker" }, { name = "psycopg", extra = ["binary"] }, { name = "sqlglot" }, ] @@ -301,6 +316,7 @@ test = [ [package.metadata] requires-dist = [ { name = "azure-identity", marker = "extra == 'azure'", specifier = ">=1.19.0" }, + { name = "docker", specifier = ">=7.1.0" }, { name = "psycopg", extras = ["binary"], specifier = ">=3.2.0" }, { name = "psycopg-pool", marker = "extra == 'db'", specifier = ">=3.3.0" }, { name = "pydantic", marker = "extra == 'db'", specifier = ">=2.0" }, @@ -513,6 +529,19 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/0b/d7/1959b9648791274998a9c3526f6d0ec8fd2233e4d4acce81bbae76b44b2a/python_dotenv-1.2.2-py3-none-any.whl", hash = "sha256:1d8214789a24de455a8b8bd8ae6fe3c6b69a5e3d64aa8a8e5d68e694bbcb285a", size = 22101, upload-time = "2026-03-01T16:00:25.09Z" }, ] +[[package]] +name = "pywin32" +version = "312" +source = { registry = "https://pypi.org/simple" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/fc/2b/1f3cded5822fd49c02f40544cbb5f58c7cfd6b1694869fd476cb6170ee97/pywin32-312-cp314-cp314-win32.whl", hash = "sha256:a77a90fbb6881238d2ca9c6fd797b25817f3768fe78d214a90137ff055a75f5b", size = 6468928, upload-time = "2026-06-04T07:49:43.188Z" }, + { url = "https://files.pythonhosted.org/packages/21/82/3bf86d2e2808902013132e1ce905a7da0da53790f3836c64bf44d55e24f3/pywin32-312-cp314-cp314-win_amd64.whl", hash = "sha256:a4dd3a848290ef724347b19f301045831d8e802fa4464f491b98b1e0a081432e", size = 7024157, upload-time = "2026-06-04T07:49:45.34Z" }, + { url = "https://files.pythonhosted.org/packages/a4/0e/73f6d6800b4f27655abd9e9f6aaeaefcddb2b946e4674efa2bab184a7f7b/pywin32-312-cp314-cp314-win_arm64.whl", hash = "sha256:9fce94568364e0155e6dfb781ac5d95903be8baf28670632beab1b523f300daa", size = 6839598, upload-time = "2026-06-04T07:49:47.613Z" }, + { url = "https://files.pythonhosted.org/packages/eb/61/caa39686032d2ebdd04ff0ab5cbe163126c0066d98e00c9018646e42393b/pywin32-312-cp315-cp315-win32.whl", hash = "sha256:5c1fbe4a937a73ae9297384a3da38518cbc694c68ad8a809b2e19acd350f03ed", size = 6471159, upload-time = "2026-06-04T07:49:50.035Z" }, + { url = "https://files.pythonhosted.org/packages/0f/cd/7e1de64a4a6f69c04214169657ccab0d93a670ea50e35eb8f489d7378249/pywin32-312-cp315-cp315-win_amd64.whl", hash = "sha256:c2f03a0f73f804a13c2735b99392b0cd426bb4f2c4d0178e5ac966a0f21618d5", size = 7025293, upload-time = "2026-06-04T07:49:54.857Z" }, + { url = "https://files.pythonhosted.org/packages/23/ed/4532e9388e65fa16b46776ef47ad631a64eda1631884488af707666350ed/pywin32-312-cp315-cp315-win_arm64.whl", hash = "sha256:a8597d28f267b39074aef51fa593530082b39cbe5a074226096857b1fed2dfb9", size = 6840337, upload-time = "2026-06-04T07:49:57.531Z" }, +] + [[package]] name = "requests" version = "2.34.2"