fastapi-sqlalchemy-pg-catalog: minimal repro sample for keploy/integrations#193#102
Open
AkashKumar7902 wants to merge 10 commits into
Open
fastapi-sqlalchemy-pg-catalog: minimal repro sample for keploy/integrations#193#102AkashKumar7902 wants to merge 10 commits into
AkashKumar7902 wants to merge 10 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a new minimal reproduction sample (fastapi-sqlalchemy-pg-catalog) demonstrating the Keploy Postgres v3 simple-query ClassCatalog replay mismatch described in keploy/integrations#193, using FastAPI + SQLAlchemy 2.x + psycopg2 + Postgres 13.
Changes:
- Introduces a small FastAPI app that runs
Base.metadata.create_all()during lifespan startup and exposes/healthand/projects. - Adds Docker Compose + Postgres init SQL to ensure the
projecttable exists at record time (so thepg_classprobe is exercised andCREATE TABLEis skipped/never recorded). - Adds a
flow.shscript and README with local record/replay instructions.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| fastapi-sqlalchemy-pg-catalog/README.md | Documents the bug, local reproduction steps, and sample layout. |
| fastapi-sqlalchemy-pg-catalog/init.sql | Initializes the project table (and seeds a row) to force the intended record-time behavior. |
| fastapi-sqlalchemy-pg-catalog/flow.sh | Drives minimal HTTP traffic for record/replay runs. |
| fastapi-sqlalchemy-pg-catalog/docker-compose.yml | Defines Postgres + app services with healthcheck and env-templated isolation knobs. |
| fastapi-sqlalchemy-pg-catalog/app/requirements.txt | Pins FastAPI/uvicorn/SQLAlchemy/psycopg2-binary versions for the repro. |
| fastapi-sqlalchemy-pg-catalog/app/main.py | Minimal FastAPI + SQLAlchemy app that triggers the pg_class probe during startup. |
| fastapi-sqlalchemy-pg-catalog/app/Dockerfile | Container build for the app service. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ations#193 FastAPI + SQLAlchemy 2.x + psycopg2 + Postgres 13 sample built to exercise the v3 dispatcher's simple-query ClassCatalog branch. The sample's init.sql pre-creates the `project` table so SQLAlchemy's Base.metadata.create_all skips CREATE TABLE at record time -- the shape the dispatcher bug requires. Used by the keploy/integrations Woodpecker lane sqlalchemy-pg-catalog-postgres to assert that recorded `type: query` mocks with `class: CATALOG` are consulted by the simple-query path, not just the extended-query path. Signed-off-by: Akash Kumar <meakash7902@gmail.com>
AkashKumar7902
force-pushed
the
add-fastapi-sqlalchemy-pg-catalog
branch
from
904f1ee to
61a61e6
Compare
… review flow.sh: - set -Eeuo pipefail so curl/other failures actually fail the script. - Track readiness explicitly; exit 1 with a clear message if the app never reaches /health within READY_TIMEOUT_S (default 60s) instead of silently falling through and proceeding against a dead app. - curl -fsS for the readiness probe and the endpoint calls so HTTP failures propagate as exit codes (the previous `curl -sS ... && echo` shape silenced non-2xx responses). init.sql: - Replace `INSERT ... ON CONFLICT DO NOTHING` with `INSERT ... SELECT WHERE NOT EXISTS`. The model has no UNIQUE constraint on name, so ON CONFLICT had nothing to fire on; this form is genuinely idempotent on re-runs against an existing volume. Refs Copilot review on #102. Signed-off-by: Akash Kumar <meakash7902@gmail.com>
Addresses two Copilot review nits on #102: * README.md: the local-repro snippet hardcoded `--container-name pg-catalog-repro-app`, but the compose file uses `${APP_CONTAINER:-pg-catalog-repro-app}` so a user who overrides APP_CONTAINER (e.g. to isolate concurrent runs) would have keploy point at a non-existent container. Both keploy invocations now thread `${APP_CONTAINER:-pg-catalog-repro-app}` so they pick up the same env override compose sees. Also switched the deprecated camelCase `--apiTimeout`/`--disableMockUpload` to the kebab forms the v3 CLI actually registers (`--api-timeout`; mock-upload flag dropped — not registered on v3-dev `test`). * main.py: SQL echo was unconditional. It is INTENTIONALLY on by default for this sample (the whole point is to surface SQLAlchemy's pg_class probe + create_all behaviour in the app log so a reader can correlate it with the keploy agent log), but the noise is real for unrelated investigations. Gated behind SQL_ECHO env var with a comment explaining why it's on by default. Signed-off-by: Akash Kumar <meakash7902@gmail.com>
…-safe create_all Two nits from the Copilot review: * DATABASE_URL: `os.environ["DATABASE_URL"]` raised a bare KeyError at import time, which surfaces as a noisy traceback in container logs with no hint about what's missing. Switched to os.getenv with an explicit RuntimeError that names the env var and gives a sample URL format. * lifespan: `Base.metadata.create_all(engine)` is synchronous psycopg2 I/O running inside an async lifespan, blocking uvicorn's event loop until pg_class probe + any CREATE TABLE round-trips complete. Switched to `await asyncio.to_thread(...)` so the loop stays responsive. For this minimal repro the difference is small, but the pattern is the right FastAPI shape for any startup that touches a sync DB driver. Signed-off-by: Akash Kumar <meakash7902@gmail.com>
…te_engine SQLAlchemy 2.x defaults to the future-2.0 behaviour; passing `future=True` is redundant and can trip a deprecation warning in some 2.x point releases. Dropped to keep the sample free of incidental warning noise that would distract from the dispatcher-bug repro. Refs Copilot review on #102. Signed-off-by: Akash Kumar <meakash7902@gmail.com>
…utdown Add try/finally around the lifespan yield so engine.dispose() runs at shutdown. Releases pooled psycopg2 connections cleanly across repeated start/stop cycles (local repro loops, CI lane reruns), which otherwise leak half-open postgres connections. Refs Copilot review on #102. Signed-off-by: Akash Kumar <meakash7902@gmail.com>
…y/finally Copilot noted that the previous try/finally only covered the yield, so a failure in create_all (the *exact* failure mode this repro demonstrates: pre-fix keploy makes create_all raise psycopg2.DatabaseError) wouldn't reach the finally and the engine pool would leak. Moved the startup logging + create_all call inside the try block; finally now runs regardless of where the failure occurs. Signed-off-by: Akash Kumar <meakash7902@gmail.com>
…lly matters
Copilot noted /docker-entrypoint-initdb.d scripts only run on first
database initialization, so the previous comment's framing ('re-run
against an existing volume') was misleading. Reworded to call out
the actual scenario: this is a single-shot insert on a clean volume,
and NOT EXISTS is defensive coverage for stale-volume reuse.
Signed-off-by: Akash Kumar <meakash7902@gmail.com>
… runs on first init Copilot's repeated note: /docker-entrypoint-initdb.d scripts run only on first database init. On a stale data volume the script doesn't run at all, so the NOT EXISTS guard couldn't help anyway. Simplified to a plain INSERT and updated the comment to tell readers to recreate the volume (docker compose down -v) if they want a deterministic repro. Signed-off-by: Akash Kumar <meakash7902@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ClassCatalogbranch via SQLAlchemy'sBase.metadata.create_allpg_class probe.keploy/integrationsWoodpecker lane.woodpecker/sqlalchemy-pg-catalog-postgres.yml(companion PR in keploy/integrations) to falsify the bug fixed there.Why this sample exists
keploy/integrations#193 — the v3 dispatcher's simple-Query
ClassCatalogbranch indispatchBySQLHashskips the recorded mock lookup and goes straight to the synthetic catalog engine. The extended-Query branch inrunEngineForPortaldoes the right thing (probe recorded mock first, fall back to synthetic on miss). The two paths diverge on the same SQL.SQLAlchemy 2.x with psycopg2 routes
Base.metadata.create_all'spg_catalog.pg_classprobe via the simple-Query protocol (psycopg2 client-side substitutes%(param)splaceholders and sends as a single SQL string, no Bind/Execute). At record time, with the table already present, the probe returns 1 row andCREATE TABLEis skipped. The recorder saves the probe astype: query, class: CATALOGwithrows=[("project",)] / CommandComplete="SELECT 1".At replay, the pre-fix dispatcher ignores that recorded mock, the synthetic catalog engine returns
rows: 0, cc: "SELECT 0", SQLAlchemy interprets "table missing", issues an unrecordedCREATE TABLE project (...), the transactional engine misses, and the app worker dies withpsycopg2.DatabaseError: keploy-pg-v3: no recorded invocation matched. Every HTTP testcase that follows fails with404because uvicorn is dead.What's in here
The compose is env-templated (
APP_CONTAINER,DB_CONTAINER,APP_HOST_PORT,COMPOSE_NET) so the integrations 3-cell matrix can isolate concurrent cells on one Docker daemon. All four have sane defaults that match the local repro recipe in the README.Verification
Local end-to-end repro confirmed against Keploy 3.5.4 (current main carries the bug):
mocks.yamlcontains theclass: CATALOGmock with bind values[project, r, p, f, v, m, pg_catalog]— the exact shape the issue documents.executeSimple dispatch class=CATALOG / catalog: pattern matched pg_class / catalog: response built rows=0chain quoted in #193, followed byexecuteSimple dispatch class=DDL CREATE TABLE projectandtransactional: no invocation matched (candidates=0). App log carriespsycopg2.DatabaseError: keploy-pg-v3: no recorded invocation matchedandApplication startup failed. Exiting.→exited with code 3.Test plan
docker compose buildworksdocker compose up -d && bash flow.sh(baseline, no keploy) — both endpoints return 200keploy recordproduces 3 testcases + the expectedclass: CATALOGmock inmocks.yamlkeploy testagainst pre-fix dispatcher: 0/3 pass with the documented cascade signaturekeploy testagainst the fixed dispatcher: 3/3 pass (validated in the integrations lane PR)References
fix-pg-v3-catalog-simple-dispatch