Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .env.example
Original file line number Diff line number Diff line change
@@ -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
Expand Down
10 changes: 6 additions & 4 deletions .github/workflows/migration-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ permissions:
jobs:
migration-test:
name: Migration round-trip
runs-on: ubuntu-latest
runs-on: ubuntu-22.04

services:
postgres:
Expand All @@ -45,10 +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
uses: actions/setup-python@v5
with:
python-version-file: .python-version-default
run: uv python install 3.12

- name: Sync workspace
run: uv sync --frozen
Expand Down
2 changes: 1 addition & 1 deletion apps/api/.env.example
Original file line number Diff line number Diff line change
@@ -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
Expand Down
17 changes: 15 additions & 2 deletions apps/api/alembic/env.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
# ``+<driver>`` 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 ``+<driver>`` 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

Expand Down
61 changes: 60 additions & 1 deletion apps/api/src/gw2analytics_api/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -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+<other-driver>://`` 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.
Expand All @@ -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
``+<driver>`` 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+<driver>://`` URL explicitly.
"""
settings = get_settings()
engine = create_engine(
settings.database_url,
_normalise_database_url(settings.database_url),
future=True,
pool_pre_ping=True,
)
Expand Down
2 changes: 1 addition & 1 deletion apps/api/tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
74 changes: 74 additions & 0 deletions apps/api/tests/test_database.py
Original file line number Diff line number Diff line change
@@ -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
``+<driver>`` 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
Loading