Skip to content

Fix: wrap SELECT 1 in text() for SQLAlchemy 2.x compatibility (#154) - #446

Open
laurale31 wants to merge 12 commits into
ascherj:mainfrom
laurale31:fix/154-health-check-sqlalchemy-text
Open

Fix: wrap SELECT 1 in text() for SQLAlchemy 2.x compatibility (#154)#446
laurale31 wants to merge 12 commits into
ascherj:mainfrom
laurale31:fix/154-health-check-sqlalchemy-text

Conversation

@laurale31

@laurale31 laurale31 commented Jul 31, 2026

Copy link
Copy Markdown

Summary

Fixes the /health endpoint's Postgres check, which currently reports the database as unhealthy even when it's fully reachable. The root cause is that SQLAlchemy 2.x no longer accepts raw SQL strings passed directly to execute() — they must be explicitly wrapped in sqlalchemy.text(). This PR wraps the query so the health check accurately reflects the database's real status.

Issue

Closes #154

Changes

  • Imported text from sqlalchemy in api/routes/health.py
  • Wrapped the Postgres health check query: db.execute("SELECT 1")db.execute(text("SELECT 1"))
  • Added tests/unit/test_health.py with two tests covering the Postgres check's healthy and unhealthy paths

Testing

  • Unit tests pass (make test-unit) — see note below on pre-existing failures
  • Integration tests pass (make test-integration) — not applicable to this fix; no integration tests touch this route
  • Linter passes (make lint) — see note below on pre-existing failures
  • Type checker passes (make typecheck) — see note below on pre-existing failures
  • New/updated tests cover the changes

Manual verification:

  1. Run docker compose up -d to start Postgres and Redis.
  2. Run make run to start the app.
  3. Run curl http://localhost:8000/health — before this fix, the response shows "postgres": "unhealthy" with an ArgumentError in the server logs; after this fix, it shows "postgres": "healthy".
  4. Run pytest tests/unit/test_health.py -v to confirm the new unit tests pass (2 passed).

Screenshots / Demo

N/A — this is a backend API fix; verification steps above cover the change.

Notes for Reviewers

This PR only touches the Postgres check in health_check(). The Redis check in the same function is separately broken (see #155, an invalid settings.redis_host reference) — that's out of scope here and left untouched. In fact, it's relied upon in test_postgres_check_healthy_when_query_succeeds to confirm the Postgres check is correctly isolated from that failure (i.e., health_check() still raises HTTPException overall due to Redis, but the Postgres field in the payload correctly reports "healthy").

Pre-existing failures (confirmed via git stash baseline comparison before my changes):

  • ruff check api/routes/health.py: 1 pre-existing issue (B008, Depends in default arg) — present before my change, unrelated to it.
  • mypy api/routes/health.py: 11 pre-existing errors (missing type annotations, dict indexing on untyped object, Redis config type mismatches, and the Health check references settings.redis_host, which does not exist on Settings #155 redis_host/redis_port attribute errors) — all present before my change.
  • make test-unit: 53 pre-existing test failures across 16 files (e.g. test_pii_scrubber.py, test_relevance_scorer.py, test_tech_detector.py, test_resume_parser.py), corresponding to other known open issues on the tracker. My change adds 2 new passing tests and does not affect this failure count (375 → 377 passed, 53 failed both before and after my change).

My changes introduce no new lint, type-check, or test failures beyond this documented baseline.

@laurale31
laurale31 marked this pull request as ready for review July 31, 2026 02:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Health check DB probe passes a raw SQL string, which fails under SQLAlchemy 2.x

2 participants