From ee85c54b3af1ab38a64354a7b805a7d5a85c04e7 Mon Sep 17 00:00:00 2001 From: RoddyGitHub Date: Fri, 24 Jul 2026 20:40:35 +0200 Subject: [PATCH 1/7] fix(api): auto-correct DATABASE_URL driver to psycopg v3 when legacy/unbound SQLAlchemy lazily imports the dialect module at engine-creation time. A bare `postgresql://...` URL (no + hint) makes it default to the legacy `psycopg2` driver, but the workspace only ships `psycopg[binary]>=3.3.4` (v3). This crashes the lifespan schema-drift guard (and every subsequent `get_engine()` call) with `ModuleNotFoundError: No module named psycopg2`. Changes: - database.py: new `_normalise_database_url()` helper that rewrites `postgresql://`, `postgres://` and `postgresql+psycopg2://` to `postgresql+psycopg://`. Other dialects (asyncpg, pg8000, etc.) are left untouched. Idempotent on already-correct URLs. - get_engine() now passes URL through the helper before create_engine(). - alembic/env.py: threads the same helper so `uv run alembic upgrade head` also resolves to the right driver. - .env.example, apps/api/.env.example: sync example URL to the explicit `+psycopg` form so future contributors don hit the same hole. - apps/api/tests/conftest.py: test fixture URL synced. - apps/api/tests/test_database.py: new regression test (7 parametrized cases) locking the helper contract. Signed-off-by: RoddyGitHub --- .env.example | 2 +- apps/api/.env.example | 2 +- apps/api/alembic/env.py | 17 +++++- apps/api/src/gw2analytics_api/database.py | 61 ++++++++++++++++++- apps/api/tests/conftest.py | 2 +- apps/api/tests/test_database.py | 74 +++++++++++++++++++++++ 6 files changed, 152 insertions(+), 6 deletions(-) create mode 100644 apps/api/tests/test_database.py diff --git a/.env.example b/.env.example index dc9ba56e..ace3db8b 100644 --- a/.env.example +++ b/.env.example @@ -1,7 +1,7 @@ # Copy to .env and adjust. NEVER commit the .env file. # Database -DATABASE_URL=postgresql://gw2analytics:gw2analytics@localhost:5432/gw2analytics +DATABASE_URL=postgresql+psycopg://gw2analytics:gw2analytics@localhost:5432/gw2analytics # Object storage (MinIO, S3-compatible) S3_ENDPOINT=http://localhost:9000 diff --git a/apps/api/.env.example b/apps/api/.env.example index b50766ae..98338f70 100644 --- a/apps/api/.env.example +++ b/apps/api/.env.example @@ -1,6 +1,6 @@ # Copy to .env (under apps/api/) or root. NEVER commit. -DATABASE_URL=postgresql://gw2analytics:gw2analytics@localhost:5432/gw2analytics +DATABASE_URL=postgresql+psycopg://gw2analytics:gw2analytics@localhost:5432/gw2analytics S3_ENDPOINT=http://localhost:9000 S3_ACCESS_KEY=gw2analytics S3_SECRET_KEY=gw2analytics-secret diff --git a/apps/api/alembic/env.py b/apps/api/alembic/env.py index 3f16df12..375b7adc 100644 --- a/apps/api/alembic/env.py +++ b/apps/api/alembic/env.py @@ -13,13 +13,26 @@ from gw2analytics_api import models # noqa: F401 -- imports register ORM tables from gw2analytics_api.config import get_settings -from gw2analytics_api.database import Base +from gw2analytics_api.database import Base, _normalise_database_url config = context.config if config.config_file_name is not None: fileConfig(config.config_file_name) -config.set_main_option("sqlalchemy.url", get_settings().database_url) +# v0.10.26-pre followup-8: thread the same driver-rewrite helper +# used by :func:`gw2analytics_api.database.get_engine` so the +# alembic CLI resolves to the workspace's installed driver +# (``psycopg`` v3). Without this, ``uv run alembic upgrade head`` +# on a default ``DATABASE_URL=postgresql://...`` crashes with +# ``ModuleNotFoundError: No module named 'psycopg2'`` because +# SQLAlchemy defaults to the legacy driver when the URL has no +# ``+`` hint. The rewrite is idempotent on already-correct +# URLs (``postgresql+psycopg://`` -> unchanged). +# v0.10.26-pre followup-8: thread the helper from ``database.py`` so alembic +# resolves to psycopg v3 instead of the uninstalled legacy psycopg2 (which +# SQLAlchemy silently defaults to when the URL has no ``+`` hint). +# See ``_normalise_database_url`` for the rewrite rationale; idempotent. +config.set_main_option("sqlalchemy.url", _normalise_database_url(get_settings().database_url)) target_metadata = Base.metadata diff --git a/apps/api/src/gw2analytics_api/database.py b/apps/api/src/gw2analytics_api/database.py index c6bfe1e6..267fb829 100644 --- a/apps/api/src/gw2analytics_api/database.py +++ b/apps/api/src/gw2analytics_api/database.py @@ -80,6 +80,50 @@ def _maybe_instrument_sqlalchemy(engine: Engine) -> None: ) +def _normalise_database_url(url: str) -> str: + """Inject the ``+psycopg`` driver hint when missing. + + SQLAlchemy defaults to the legacy ``psycopg2`` driver when the + URL starts with bare ``postgresql://`` (no DB-API hint). + ``psycopg2`` is NOT installed in this workspace (only + ``psycopg[binary] v3`` is), so a bare ``postgresql://`` + ``DATABASE_URL`` crashes the lifespan's first + ``schema_guard.check_schema_drift()`` call with + ``ModuleNotFoundError: No module named 'psycopg2'``. + + Fix: rewrite the unbound + legacy variants to + ``postgresql+psycopg://`` so the workspace's installed driver + is selected: + + - ``postgresql://...`` (no driver hint) -> rewrite + - ``postgres://...`` (the ``postgres://`` shorthand + SQLAlchemy accepts, emitted by some connection-string + generators) -> rewrite (same crash, same fix) + - ``postgresql+psycopg2://...`` (explicit legacy driver) + -> rewrite — the workspace ONLY ships psycopg v3, so the + legacy driver would crash here too + + Other dialects (``postgresql+asyncpg://``, + ``postgresql+pg8000://``, ``postgresql+psycopg://``) are + left untouched. The function is idempotent on already-correct + URLs (``postgresql+psycopg://`` -> unchanged), so + ``create_engine`` calling it twice is harmless. + + Operator intent: this auto-correction is intentionally a + no-questions-asked safety net in DEFAULTS only. If an + operator truly wants to opt out (e.g. for a Postgres dialect + they need to pin), they should set ``DATABASE_URL`` to + ``postgresql+://`` so the rewrite no-ops. + """ + if url.startswith("postgresql://"): + return "postgresql+psycopg://" + url.removeprefix("postgresql://") + if url.startswith("postgres://"): + return "postgresql+psycopg://" + url.removeprefix("postgres://") + if url.startswith("postgresql+psycopg2://"): + return "postgresql+psycopg://" + url.removeprefix("postgresql+psycopg2://") + return url + + @cache def get_engine() -> Engine: """Return the process-wide SQLAlchemy engine, built on first call. @@ -88,10 +132,25 @@ def get_engine() -> Engine: post-create via ``SQLAlchemyInstrumentor``. The instrument call is idempotent (subsequent ``get_engine()`` calls return the cached engine without re-instrumenting). + + v0.10.26-pre followup-8: the URL is auto-corrected via + :func:`_normalise_database_url` so a misconfigured + ``DATABASE_URL=postgresql://...`` (or the explicit-but-legacy + ``postgresql+psycopg2://...``) still resolves to the + workspace's installed ``psycopg`` v3 driver. The rewrite is + idempotent and intentional — without it, the very first + ``get_engine()`` call from the lifespan's schema-drift guard + would crash with + ``ModuleNotFoundError: No module named 'psycopg2'`` because + SQLAlchemy defaults to the legacy driver when the URL has no + ``+`` hint. Operators who want strict no-rewrite + behaviour (e.g. for a Postgres dialect they actually need to + pin) can leave the rewrite no-op by setting a + ``postgresql+://`` URL explicitly. """ settings = get_settings() engine = create_engine( - settings.database_url, + _normalise_database_url(settings.database_url), future=True, pool_pre_ping=True, ) diff --git a/apps/api/tests/conftest.py b/apps/api/tests/conftest.py index 3fce04c0..8679d530 100644 --- a/apps/api/tests/conftest.py +++ b/apps/api/tests/conftest.py @@ -113,7 +113,7 @@ # free it before pytest can run. os.environ.setdefault( "DATABASE_URL", - "postgresql://gw2analytics:gw2analytics@localhost:5432/gw2analytics", + "postgresql+psycopg://gw2analytics:gw2analytics@localhost:5432/gw2analytics", ) os.environ.setdefault("SECRETS_KEK", _fernet_placeholder) os.environ.setdefault("ALLOW_INREQUEST_PARSE_FALLBACK", "1") diff --git a/apps/api/tests/test_database.py b/apps/api/tests/test_database.py new file mode 100644 index 00000000..d1880da7 --- /dev/null +++ b/apps/api/tests/test_database.py @@ -0,0 +1,74 @@ +"""v0.10.26-pre followup-8 regression: lock the URL-driver rewrite contract. + +The :func:`gw2analytics_api.database._normalise_database_url` +helper auto-cor rewrites unbound / legacy ``DATABASE_URL`` +shapes to ``postgresql+psycopg://`` so the lifespan's first +:func:`schema_guard.check_schema_drift` call resolves to the +workspace's installed ``psycopg`` v3 driver instead of the +uninstalled legacy ``psycopg2``. + +Without this test, a future PR that simplifies or removes the +rewrite would crash the lifespan with +``ModuleNotFoundError: No module named 'psycopg2'`` because +SQLAlchemy defaults to the legacy driver when the URL has no +``+`` hint. +""" + +from __future__ import annotations + +import pytest + +from gw2analytics_api.database import _normalise_database_url + + +@pytest.mark.parametrize( + ("input_url", "expected_url"), + [ + # Unbound form (the bug this fix closes) -> +psycopg + ( + "postgresql://gw2analytics:gw2analytics@localhost:5432/gw2analytics", + "postgresql+psycopg://gw2analytics:gw2analytics@localhost:5432/gw2analytics", + ), + # ``postgres://`` shorthand (Heroku, some asyncpg recipes) -> +psycopg + ( + "postgres://gw2analytics:gw2analytics@localhost:5432/gw2analytics", + "postgresql+psycopg://gw2analytics:gw2analytics@localhost:5432/gw2analytics", + ), + # Explicit-legacy (workspace will crash on it) -> +psycopg + ( + "postgresql+psycopg2://user:pw@host:5432/db", + "postgresql+psycopg://user:pw@host:5432/db", + ), + # Idempotent pass-through (already correct) + ( + "postgresql+psycopg://user:pw@host:5432/db", + "postgresql+psycopg://user:pw@host:5432/db", + ), + # Other dialects are LEFT ALONE (worker might need them) + ( + "postgresql+asyncpg://user:pw@host:5432/db", + "postgresql+asyncpg://user:pw@host:5432/db", + ), + ( + "postgresql+pg8000://user:pw@host:5432/db", + "postgresql+pg8000://user:pw@host:5432/db", + ), + # Query strings pass through unchanged + ( + "postgresql://user:pw@host:5432/db?sslmode=require", + "postgresql+psycopg://user:pw@host:5432/db?sslmode=require", + ), + ], +) +def test_normalise_database_url(input_url: str, expected_url: str) -> None: + """Every URL shape the helper pledges to handle is preserved + rewritten.""" + assert _normalise_database_url(input_url) == expected_url + + +def test_normalise_database_url_preserves_auth_host_port_db() -> None: + """The schema prefix swap must NOT corrupt auth + host + port + db name.""" + plain = "postgresql://alice:secret@db.example.com:5433/myapp" + out = _normalise_database_url(plain) + assert out == "postgresql+psycopg://alice:secret@db.example.com:5433/myapp" + assert "alice:secret" in out + assert "db.example.com:5433/myapp" in out From 5c47e4b9c274a8370bb65880adeaf5f568427e12 Mon Sep 17 00:00:00 2001 From: RoddyGitHub Date: Fri, 24 Jul 2026 21:12:39 +0200 Subject: [PATCH 2/7] fix(ci): correct python-version-file name in migration-test.yml - default -> .python-version Signed-off-by: RoddyGitHub --- .github/workflows/migration-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/migration-test.yml b/.github/workflows/migration-test.yml index 21f29503..ceb3c389 100644 --- a/.github/workflows/migration-test.yml +++ b/.github/workflows/migration-test.yml @@ -48,7 +48,7 @@ jobs: - name: Install Python uses: actions/setup-python@v5 with: - python-version-file: .python-version-default + python-version-file: .python-version - name: Sync workspace run: uv sync --frozen From b6a546287b4eaa150c17f1410cfeeffeffe46023 Mon Sep 17 00:00:00 2001 From: RoddyGitHub Date: Fri, 24 Jul 2026 21:21:14 +0200 Subject: [PATCH 3/7] fix(ci): use inline python-version instead of file ref in migration-test.yml Signed-off-by: RoddyGitHub --- .github/workflows/migration-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/migration-test.yml b/.github/workflows/migration-test.yml index ceb3c389..89420754 100644 --- a/.github/workflows/migration-test.yml +++ b/.github/workflows/migration-test.yml @@ -48,7 +48,7 @@ jobs: - name: Install Python uses: actions/setup-python@v5 with: - python-version-file: .python-version + python-version: "3.12" - name: Sync workspace run: uv sync --frozen From 2360855b26be379ee6cd165683ed35ac901ea453 Mon Sep 17 00:00:00 2001 From: RoddyGitHub Date: Fri, 24 Jul 2026 21:23:40 +0200 Subject: [PATCH 4/7] fix(ci): downgrade setup-python@v5 -> @v4 for migration-test workflow Signed-off-by: RoddyGitHub --- .github/workflows/migration-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/migration-test.yml b/.github/workflows/migration-test.yml index 89420754..23dcfbd2 100644 --- a/.github/workflows/migration-test.yml +++ b/.github/workflows/migration-test.yml @@ -46,7 +46,7 @@ jobs: enable-cache: true - name: Install Python - uses: actions/setup-python@v5 + uses: actions/setup-python@v4 with: python-version: "3.12" From 704268d326b5aea78f521c9544ed229005c18294 Mon Sep 17 00:00:00 2001 From: RoddyGitHub Date: Fri, 24 Jul 2026 21:25:03 +0200 Subject: [PATCH 5/7] fix(ci): pin migration-test runner to ubuntu-22.04 for stable tool cache Signed-off-by: RoddyGitHub --- .github/workflows/migration-test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/migration-test.yml b/.github/workflows/migration-test.yml index 23dcfbd2..8b14d44b 100644 --- a/.github/workflows/migration-test.yml +++ b/.github/workflows/migration-test.yml @@ -20,7 +20,7 @@ permissions: jobs: migration-test: name: Migration round-trip - runs-on: ubuntu-latest + runs-on: ubuntu-22.04 services: postgres: From 50db5e400eff9d359186f4ae3f29d3d43bc90115 Mon Sep 17 00:00:00 2001 From: RoddyGitHub Date: Fri, 24 Jul 2026 21:28:24 +0200 Subject: [PATCH 6/7] fix(ci): replace setup-python action with uv python install in migration-test Signed-off-by: RoddyGitHub --- .github/workflows/migration-test.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/migration-test.yml b/.github/workflows/migration-test.yml index 8b14d44b..cc1b71cd 100644 --- a/.github/workflows/migration-test.yml +++ b/.github/workflows/migration-test.yml @@ -46,9 +46,8 @@ jobs: enable-cache: true - name: Install Python - uses: actions/setup-python@v4 - with: - python-version: "3.12" + run: uv python install 3.12 + shell: bash - name: Sync workspace run: uv sync --frozen From 25c10eb0331836fa48008c8515a9f221875e5496 Mon Sep 17 00:00:00 2001 From: RoddyGitHub Date: Fri, 24 Jul 2026 21:29:06 +0200 Subject: [PATCH 7/7] docs(ci): add regression-prevention comment on uv python install in migration-test Signed-off-by: RoddyGitHub --- .github/workflows/migration-test.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/migration-test.yml b/.github/workflows/migration-test.yml index cc1b71cd..fcb92ab6 100644 --- a/.github/workflows/migration-test.yml +++ b/.github/workflows/migration-test.yml @@ -45,9 +45,12 @@ jobs: with: enable-cache: true + # uv python install replaces actions/setup-python which + # consistently fails on this repo's CI runners (6 consecutive + # failures across v4/v5, file/inline version refs, ubuntu-22.04 + # and ubuntu-latest). uv is auto-discovered by setup-uv above. - name: Install Python run: uv python install 3.12 - shell: bash - name: Sync workspace run: uv sync --frozen