You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit b82495e
Browse filesBrowse the repository at this point in the historyBrowse files
perf: turn pool_pre_ping off by default and rely on postgres_retry (#88)
* test: cover handlers against lost connections and the pre-ping default
The handler tests terminate the backend a request session holds, the endpoint tests terminate an idle pooled connection on each engine. Both fail until the handlers retry inside a Transaction and pre-ping is off.
* perf: turn pool_pre_ping off by default and rely on postgres_retry
Every handler is decorated with postgres_retry and runs its queries inside a request-scoped db_retry.Transaction, committing before it leaves the block. Repositories switch to auto_commit=False so the handler owns the whole transaction. Requires db-retry 0.8.0.
* refactor: move each handler's database work into a use case
app/use_cases/ holds one dataclass per operation, retried with postgres_retry and running inside its Transaction, as in chat-app. ioc splits into Database, Repositories and UseCases groups, and handlers only call a use case and convert the result.
* test: replace the per-use-case connection-loss table with one test
Every use case has the same retry and Transaction shape, so ListDecksUseCase stands in for all of them. It runs against a connection lost while held by the session and one lost while idle in the pool. It only reads, so the committed fixture data and the truncation go away.
* test: drop the connection-loss test
db-retry already tests a pooled connection closed while idle being retried through a Transaction against a real Postgres. The template test only ran the same library code through a trivial use case.
Copy file name to clipboardExpand all lines: AGENTS.md
+37-7Lines changed: 37 additions & 7 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -30,27 +30,57 @@ CI (`.github/workflows/main.yml`) runs `ruff format --check`, `ruff check --no-f
30
30
31
31
`app/__main__.py` boots Granian pointing at `app.application:build_app` (factory). `build_app()` in `app/application.py`:
32
32
33
-
1. Creates a `modern_di.Container` with the `Dependencies` group from `app/ioc.py`.
33
+
1. Creates a `modern_di.Container` with `ioc.ALL_GROUPS` from `app/ioc.py`.
34
34
2. Builds a `FastAPIBootstrapper` from `settings.api_bootstrapper_config`, injecting the SQLAlchemy OpenTelemetry instrumentor.
35
35
3.`modern_di_fastapi.setup_di(app, container)` wires DI scopes onto the FastAPI app.
36
36
4. Includes one `ROUTER` per resource (`app.api.decks.ROUTER`, `app.api.cards.ROUTER`) under `/api` via `include_routers`. Add a new resource by creating `app/api/<name>.py` with its own `APIRouter`-based `ROUTER` and registering it there.
37
37
5. Registers `DuplicateKeyError` → 422 handler from `app/exceptions.py`.
38
38
39
39
### DI scopes (modern-di)
40
40
41
-
`app/ioc.py` defines providers:
41
+
`app/ioc.py` splits providers into three groups, collected in `ALL_GROUPS`.
-`database_read_engine` — singleton-ish read `AsyncEngine` built from `DB_REPLICA_DSN`, or from `DB_DSN` when it is unset. It runs in `AUTOCOMMIT` (no BEGIN/ROLLBACK per request, and the pre-ping is a bare `;`) and sets `default_transaction_read_only=on`, so Postgres rejects writes on it. Each statement sees its own snapshot, so a multi-query read (e.g. `fetch_with_cards`) can see a write that commits in between. Without a replica it is a second pool on the primary, so a process can hold up to `2 × db_pool_size` connections.
45
+
-`database_read_engine` — singleton-ish read `AsyncEngine` built from `DB_REPLICA_DSN`, or from `DB_DSN` when it is unset. It runs in `AUTOCOMMIT` (no BEGIN/ROLLBACK per request) and sets `default_transaction_read_only=on`, so Postgres rejects writes on it. Each statement sees its own snapshot, so a multi-query read (e.g. `fetch_with_cards`) can see a write that commits in between. Without a replica it is a second pool on the primary, so a process can hold up to `2 × db_pool_size` connections.
44
46
-`dynamic_engine` — `Scope.REQUEST`; `choose_sa_engine` returns the read engine for `GET`/`HEAD` requests, the primary otherwise (including when there is no request). GET handlers cannot write, and a read right after a write may see replica lag.
45
47
-`session` — `Scope.REQUEST`, bound to `dynamic_engine`, finalized by `close_session`.
46
-
-`decks_repository`, `cards_repository` — `Scope.REQUEST`, depend on `session`, configured with `auto_commit=True` (commit happens at session close, not per call).
48
+
-`transaction` — `Scope.REQUEST`, a `db_retry.Transaction` over `session`. Leaving its block rolls back anything uncommitted and closes the session.
49
+
50
+
`Repositories` (`Scope.REQUEST`): `decks_repository` and `cards_repository`, on `Database.session` with `auto_commit=False`. The use case commits through `transaction.commit()`.
51
+
52
+
`UseCases` (`Scope.REQUEST`): one provider per use case in `app/use_cases/`. modern-di fills each dataclass field by its type, so a use case needs no `kwargs`.
53
+
54
+
Endpoints inject a use case with `FromDI(UseCase)` from `modern_di_fastapi`, call it, and convert the result to a schema. They never touch repositories or the session. Add new providers to the matching group rather than constructing services manually in routes.
55
+
56
+
### Use cases and lost connections
57
+
58
+
`pool_pre_ping` is off by default (`DB_POOL_PRE_PING`). A pooled connection that died while idle is handled by `db_retry.postgres_retry` instead, which saves a round trip on every checkout. That only works if the retried function owns its whole transaction, so every use case has this shape:
- Every query runs inside `async with self.transaction:`. A query before the block leaves the session in a failed transaction on a lost connection, and the retry then fails with `PendingRollbackError`.
75
+
- Commit before leaving the block, reads included, and before any early return. Otherwise `__aexit__` rolls back, which expires the loaded rows.
76
+
- A retry runs the whole use case again. If the connection drops during `COMMIT`, the write may already have landed, so a non-idempotent write such as `CreateDeckUseCase` can be applied twice.
47
77
48
-
Endpoints inject repositories with `FromDI(Repository)`from `modern_di_fastapi`. Add new providers to `Dependencies` rather than constructing services manually in routes.
78
+
`app/use_cases/` is exempt from ruff's `TC` rules because modern-di reads the dataclass field annotations at runtime.
49
79
50
80
### Database layer
51
81
52
82
-`app/models.py` — `BigIntAuditBase` from `advanced_alchemy` (auto `id`, `created_at`, `updated_at`). The module aliases `orm_registry.metadata` onto `orm.DeclarativeBase.metadata` so Alembic autogenerate sees both. New models go here.
53
-
-`app/repositories.py` — Subclass `SQLAlchemyAsyncRepositoryService[Model]` with a nested `BaseRepository(SQLAlchemyAsyncRepository[Model])`. Routes use the service methods (`list`, `get_one_or_none`, `create`, `update`, `create_many`, `upsert_many`).
83
+
-`app/repositories.py` — Subclass `SQLAlchemyAsyncRepositoryService[Model]` with a nested `BaseRepository(SQLAlchemyAsyncRepository[Model])`. Use cases call the service methods (`get_many`, `get_one`, `create`, `update`, `create_many`, `upsert_many`).
54
84
-`app/resources/db.py` — engine factories live here and are DI creators: modern-di resolves their parameter annotations at runtime, so `pyproject.toml` exempts `app/resources/` from ruff's `TC` rules. `create_session` passes `join_transaction_mode="create_savepoint"`. This is inert in production (the session binds to an engine) but enables the test rollback pattern below: when a test binds the session to a connection already in a transaction, the session owns its own savepoint so the outer transaction survives commits — do not "fix" it.
55
85
-`migrations/env.py` swaps the asyncpg driver for the sync `postgresql` driver and uses `app.models.METADATA` as `target_metadata`.
56
86
@@ -63,7 +93,7 @@ Endpoints inject repositories with `FromDI(Repository)` from `modern_di_fastapi`
63
93
`tests/conftest.py` provides the test isolation pattern — read it before adding fixtures:
64
94
65
95
-`app` fixture builds a fresh app via `LifespanManager`.
66
-
-`db_session` opens a connection, begins a transaction, and **overrides `Dependencies.dynamic_engine`** with the connection itself, so reads and writes share one rolled-back transaction. Each session built against that connection uses `join_transaction_mode="create_savepoint"`, so `auto_commit` releases the session's own savepoint while the outer transaction is rolled back at teardown — each test starts clean.
96
+
-`db_session` opens a connection, begins a transaction, and **overrides `Database.dynamic_engine`** with the connection itself, so reads and writes share one rolled-back transaction. Each session built against that connection uses `join_transaction_mode="create_savepoint"`, so `transaction.commit()` releases the session's own savepoint while the outer transaction is rolled back at teardown — each test starts clean.
67
97
-`set_async_session_in_base_sqlalchemy_factory` wires `db_session` into `SQLAlchemyFactory.__async_session__` so `polyfactory` factories in `tests/factories.py` (`DeckModelFactory`, `CardModelFactory`) persist via the rolled-back session. Test modules that use these factories opt in with `pytestmark = [pytest.mark.usefixtures("set_async_session_in_base_sqlalchemy_factory")]`.
68
98
69
99
`pytest.ini_options` sets `asyncio_mode = "auto"` — async tests do not need `@pytest.mark.asyncio`. Coverage runs by default with `--cov-fail-under=100`, so `just test` fails unless tests cover every line of new code.
0 commit comments