Skip to content

Commit 4918893

Browse files
olivermeyerclaude
andcommitted
fix(database): normalise asyncpg to psycopg in get_url
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 1519ae3 commit 4918893

5 files changed

Lines changed: 88 additions & 15 deletions

File tree

ATTRIBUTIONS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,7 @@ SOFTWARE.
360360

361361
```
362362

363-
## aignostics-foundry-core (0.6.1) - MIT License
363+
## aignostics-foundry-core (0.6.2) - MIT License
364364

365365
🏭 Foundational infrastructure for Foundry components.
366366

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
# 4. DatabaseSettings URL attribute
2+
3+
Date: 2026-04-02
4+
5+
## Status
6+
7+
Accepted
8+
9+
## Context
10+
11+
The functionality of the `DatabaseSettings` class is inherited from Bridge. It has:
12+
- A `url` field to store the full database connection URL, including a driver prefix and database name
13+
- A `name` field to store just the database name for substitution (used for feature environments)
14+
- A `get_url()` method that processes the raw URL (optionally replacing the driver and database name) and is the
15+
intended API for consumers
16+
17+
This is confusing: it's not clear that the URL should contain a database name, nor that the driver in the URL might be
18+
replaced at runtime, nor that the `name` field is used for substitution. Additionally, callers might easily reach
19+
for `settings.url` instead of `settings.get_url()`, skipping the processing logic and causing subtle bugs.
20+
21+
### Alternatives
22+
23+
- Rename `url` to `_url` with `Field(alias="url")`: this would make it clear that it's an internal field,
24+
but is **not supported by Pydantic >= 2.1.0**. We therefore reject this option.
25+
- Refactor the class to have a `host`, `db_name` and optional `driver` attributes and always construct the URL at runtime.
26+
This is the preferred design, but is a more significant change which would delay delivery of the service template.
27+
We therefore reject this option **for now**.
28+
- **`raw_url: SecretStr = Field(alias="url")`**: pydantic-settings applies the dynamic
29+
`_env_prefix` only to field names, not to aliases. With prefix `TEST_DB_` and
30+
`Field(alias="url")`, pydantic-settings looks for the bare env var `URL` rather than
31+
`TEST_DB_URL`, breaking the env-var contract entirely. `validate_by_name=True` adds a lookup
32+
for `TEST_DB_RAW_URL` but never for `TEST_DB_URL`. `PrivateAttr` was also evaluated: it
33+
supports underscore names but cannot be populated from env vars by pydantic-settings, requiring
34+
a bridge field that still leaves the raw URL accessible.
35+
36+
## Decision
37+
38+
Keep `url: SecretStr` as the field name. Rely on docstrings, the class API design, and this
39+
decision record to guide callers toward `get_url()`.
40+
41+
## Consequences
42+
43+
- `get_url()` is the only correct way to consume the URL; `settings.url` remains accessible but
44+
is clearly documented as internal and unprocessed.
45+
- The `raw_url` rename and `_url` alias approaches are ruled out; this document records the
46+
constraints for future reference.
47+
- A long-term redesign (separate `host`, `db_name`, optional `driver` fields) remains the preferred
48+
direction but is deferred.

src/aignostics_foundry_core/AGENTS.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ This file provides an overview of all modules in `aignostics_foundry_core`, thei
1717
| **log** | Configurable loguru logging initialisation | `logging_initialize(filter_func=None, *, context=None)`, `LogSettings` (env-prefix configurable), `InterceptHandler` for stdlib-to-loguru bridging |
1818
| **sentry** | Configurable Sentry integration | `sentry_initialize(integrations, *, context=None)`, `SentrySettings` (env-prefix configurable), `set_sentry_user(user, role_claim)` for Auth0 user context |
1919
| **service** | FastAPI-injectable base service | `BaseService` ABC with `get_service()` (cached per-class FastAPI `Depends` factory), `key()`, and abstract `health()` / `info()` methods; concrete subclasses implement health checks and module info |
20-
| **database** | Async SQLAlchemy session management + DB settings | `DatabaseSettings` (`OpaqueSettings` subclass; env prefix defaults to `{ctx.env_prefix}DB_`; `get_url()` with optional database `name` substitution); `init_engine(db_url=None, pool_size=None, pool_max_overflow=None, pool_timeout=None)` — all params optional, fall back to active context when `None`; `dispose_engine()`, `get_db_session()` (FastAPI dependency), `execute_with_session(func, …)`, `cli_run_with_db(func, …, db_url=None)`, `cli_run_with_engine(func, …, db_url=None)`, `with_engine` dual-mode decorator (supports `@with_engine`, `@with_engine()`, `@with_engine(db_url=…)`); auto-resets engine after `fork()` |
20+
| **database** | Async SQLAlchemy session management + DB settings | `DatabaseSettings` (`OpaqueSettings` subclass; env prefix defaults to `{ctx.env_prefix}DB_`; `url: SecretStr` (required; always access via `get_url()`, not directly); `get_url()` with optional database `name` substitution); `init_engine(db_url=None, pool_size=None, pool_max_overflow=None, pool_timeout=None)` — all params optional, fall back to active context when `None`; `dispose_engine()`, `get_db_session()` (FastAPI dependency), `execute_with_session(func, …)`, `cli_run_with_db(func, …, db_url=None)`, `cli_run_with_engine(func, …, db_url=None)`, `with_engine` dual-mode decorator (supports `@with_engine`, `@with_engine()`, `@with_engine(db_url=…)`); auto-resets engine after `fork()` |
2121
| **cli** | Typer CLI preparation utilities | `prepare_cli(cli, epilog, *, context=None)` — discovers and registers subcommands via `locate_implementations`, sets epilog recursively, installs `no_args_is_help` workaround; `no_args_is_help_workaround(ctx)` — raises `typer.Exit` when no subcommand is invoked |
2222
| **boot** | Application / library boot sequence | `boot(context, sentry_integrations, log_filter, show_cmdline)` — runs once per process: parses `--env` CLI args, initialises logging and Sentry, amends the SSL trust chain via *truststore* and *certifi*, and logs boot/shutdown messages |
2323
| **user_agent** | Parameterised HTTP user-agent string builder | `user_agent(project_name, version, repository_url)` — builds `{project_name}-python-sdk/{version} (…)` string including platform info, current test, and GitHub Actions run URL |
@@ -220,7 +220,7 @@ This file provides an overview of all modules in `aignostics_foundry_core`, thei
220220

221221
- **Purpose**: Provides a self-contained `OpaqueSettings` subclass that reads database connection parameters from env vars. The env prefix defaults to `{FoundryContext.env_prefix}DB_` when not supplied, enabling zero-boilerplate DB configuration once a `FoundryContext` is installed.
222222
- **Key Features**:
223-
- `DatabaseSettings(OpaqueSettings)` — fields: `url: SecretStr` (required), `pool_size: int = 10`, `pool_max_overflow: int = 10`, `pool_timeout: float = 30.0`, `name: str | None = None`
223+
- `DatabaseSettings(OpaqueSettings)` — fields: `url: SecretStr` (required; use `get_url()` — do not access `url` directly), `pool_size: int = 10`, `pool_max_overflow: int = 10`, `pool_timeout: float = 30.0`, `name: str | None = None`
224224
- `__init__(_env_prefix=None, **kwargs)` — when `_env_prefix` is `None`, lazy-imports `get_context` and uses `f"{ctx.env_prefix}DB_"` as the prefix (avoids a circular import at module load time)
225225
- `get_url() -> str` — returns the raw URL from the secret; if `name` is set, replaces the path component in the URL (e.g. `…/postgres``…/mydb`) while preserving scheme, host, port, query, and fragment
226226
- `model_config = SettingsConfigDict(extra="ignore")` — extra env vars are silently ignored

src/aignostics_foundry_core/database.py

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,14 @@ class DatabaseSettings(OpaqueSettings):
3636
3737
Environment variables (with default prefix ``{NAME}_DB_``):
3838
39-
* ``{PREFIX}URL`` — required; the full database connection URL
39+
* ``{PREFIX}URL`` — required; the full database connection URL. Access via :meth:`get_url` only.
4040
* ``{PREFIX}POOL_SIZE`` — optional; connection pool size (default ``10``)
4141
* ``{PREFIX}POOL_MAX_OVERFLOW`` — optional; maximum pool overflow (default ``10``)
4242
* ``{PREFIX}POOL_TIMEOUT`` — optional; pool checkout timeout in seconds (default ``30.0``)
4343
* ``{PREFIX}NAME`` — optional; override the database name in the URL path component
44+
45+
References:
46+
docs/decisions/0004-databasesettings-url-attribute.md
4447
"""
4548

4649
model_config = SettingsConfigDict(extra="ignore")
@@ -59,6 +62,10 @@ def __init__(
5962
) -> None:
6063
"""Initialise settings, deriving env prefix and env files from the active FoundryContext when not given.
6164
65+
TODO(oliverm): should this class have a `host` attribute WITHOUT the DB name, and construct the full URL as
66+
`host/name`? This would avoid the confusion between the `url` attribute and the `get_url()` method.
67+
See docs/decisions/0004-databasesettings-url-attribute.md.
68+
6269
Args:
6370
_env_prefix: Optional explicit environment variable prefix (e.g. ``"MYAPP_DB_"``).
6471
When ``None``, the prefix is derived from the active FoundryContext as
@@ -92,15 +99,16 @@ def __init__(
9299
super().__init__(_env_prefix=_env_prefix, **kwargs) # pyright: ignore[reportCallIssue]
93100

94101
def get_url(self) -> str:
95-
"""Return the database URL string, optionally substituting the database name.
102+
"""Return the database URL string, normalizing the driver and optionally substituting the database name.
96103
97-
When :attr:`name` is set, the path component of the URL is replaced with
98-
``/{name}``, leaving the scheme, host, port, query, and fragment unchanged.
104+
The method performs two transformations:
105+
1. Normalizes ``+asyncpg`` driver to ``+psycopg`` in the URL scheme
106+
2. When :attr:`name` is set, replaces the path component with ``/{name}``
99107
100108
Returns:
101-
The database URL as a plain string.
109+
The database URL as a plain string with normalized driver.
102110
"""
103-
raw = self.url.get_secret_value()
111+
raw = self.url.get_secret_value().replace("+asyncpg", "+psycopg")
104112
if self.name is None:
105113
return raw
106114
parsed = urllib.parse.urlparse(raw)

tests/aignostics_foundry_core/database_settings_test.py

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
# Constants (SonarQube S1192)
1313
POSTGRES_URL = "postgresql+asyncpg://user:pass@localhost:5432/postgres"
14+
POSTGRES_URL_PSYCOPG = "postgresql+psycopg://user:pass@localhost:5432/postgres"
1415
SQLITE_URL = "sqlite+aiosqlite:///test.db"
1516
WRONG_SQLITE_URL = "sqlite+aiosqlite:///wrong.db"
1617
MYAPP_ENV_PREFIX = "MYAPP_"
@@ -44,7 +45,7 @@ def _reset_context() -> Generator[None, None, None]: # pyright: ignore[reportUn
4445
def test_get_url_returns_plain_url_when_db_name_not_set() -> None:
4546
"""get_url() returns the raw secret value unchanged when database name is None."""
4647
settings = DatabaseSettings(_env_prefix="TEST_DB_", url=POSTGRES_URL)
47-
assert settings.get_url() == POSTGRES_URL
48+
assert settings.get_url() == POSTGRES_URL_PSYCOPG
4849

4950

5051
@pytest.mark.unit
@@ -61,10 +62,26 @@ def test_get_url_preserves_scheme_and_host() -> None:
6162
"""Scheme, host, and port are intact after name substitution."""
6263
settings = DatabaseSettings(_env_prefix="TEST_DB_", url=POSTGRES_URL, name="mydb")
6364
result = settings.get_url()
64-
assert result.startswith("postgresql+asyncpg://")
65+
assert result.startswith("postgresql+psycopg://")
6566
assert "localhost:5432" in result
6667

6768

69+
@pytest.mark.unit
70+
def test_get_url_normalises_asyncpg_to_psycopg() -> None:
71+
"""get_url() rewrites +asyncpg to +psycopg in the returned URL."""
72+
settings = DatabaseSettings(_env_prefix="TEST_DB_", url=POSTGRES_URL)
73+
result = settings.get_url()
74+
assert "+asyncpg" not in result
75+
assert "+psycopg" in result
76+
77+
78+
@pytest.mark.unit
79+
def test_get_url_leaves_non_asyncpg_schemes_unchanged() -> None:
80+
"""get_url() does not modify URLs that do not contain +asyncpg."""
81+
settings = DatabaseSettings(_env_prefix="TEST_DB_", url=SQLITE_URL)
82+
assert settings.get_url() == SQLITE_URL
83+
84+
6885
# ---------------------------------------------------------------------------
6986
# env-prefix resolution
7087
# ---------------------------------------------------------------------------
@@ -78,7 +95,7 @@ def test_default_env_prefix_reads_from_context(monkeypatch: pytest.MonkeyPatch)
7895
monkeypatch.setenv("MYAPP_DB_URL", POSTGRES_URL)
7996

8097
settings = DatabaseSettings()
81-
assert settings.get_url() == POSTGRES_URL
98+
assert settings.get_url() == POSTGRES_URL_PSYCOPG
8299

83100

84101
@pytest.mark.unit
@@ -90,7 +107,7 @@ def test_explicit_env_prefix_overrides_context(monkeypatch: pytest.MonkeyPatch)
90107
monkeypatch.setenv(CUSTOM_PREFIX_URL_ENV, POSTGRES_URL)
91108

92109
settings = DatabaseSettings(_env_prefix=CUSTOM_PREFIX)
93-
assert settings.get_url() == POSTGRES_URL
110+
assert settings.get_url() == POSTGRES_URL_PSYCOPG
94111

95112

96113
# ---------------------------------------------------------------------------
@@ -139,8 +156,8 @@ def test_db_name_reads_from_name_env_var(monkeypatch: pytest.MonkeyPatch) -> Non
139156

140157

141158
@pytest.mark.unit
142-
def test_url_is_masked_in_repr() -> None:
143-
"""repr(settings) / str(settings) does not expose the raw URL."""
159+
def test_raw_url_is_masked_in_repr() -> None:
160+
"""repr(settings) does not expose the secret value of raw_url."""
144161
settings = DatabaseSettings(_env_prefix="TEST_DB_", url=POSTGRES_URL)
145162
representation = repr(settings)
146163
assert "pass" not in representation

0 commit comments

Comments
 (0)