Skip to content

Fix/155 health check redis host - #384

Open
devikaviju wants to merge 7 commits into
ascherj:mainfrom
devikaviju:fix/155-health-check-redis-host
Open

Fix/155 health check redis host#384
devikaviju wants to merge 7 commits into
ascherj:mainfrom
devikaviju:fix/155-health-check-redis-host

Conversation

@devikaviju

Copy link
Copy Markdown

Summary

The Redis probe read config fields that don't exist, so /health reported Redis as down even when it was fine. This points the probe at the field Settings actually defines.

Issue

Closes #155

Changes

  • api/routes/health.py: replaced redis.Redis(host=settings.redis_host, port=settings.redis_port, db=0, ...) with redis.Redis.from_url(settings.redis_url, decode_responses=True). Settings defines redis_url, not redis_host/redis_port.
  • Import ordering in the same file was applied automatically by the project's pre-commit hooks.
  • tests/unit/test_health.py: new file, 4 unit tests for the Redis probe.

Testing

  • Unit tests pass (make test-unit) — see note below
  • Integration tests pass (make test-integration) — not run
  • Linter passes (make lint) — see note below
  • Type checker passes (make typecheck) — see note below
  • New/updated tests cover the changes

Verified manually against a running stack:

Before: {"dependencies":{"postgres":"unhealthy","redis":"unhealthy","vector_db":"healthy"}}

After: {"dependencies":{"postgres":"unhealthy","redis":"healthy","vector_db":"healthy"}}

The new tests fail 2/4 against the pre-fix code and pass 4/4 after.

Notes for Reviewers

Pre-existing failures. make test-unit was 53 failed / 375 passed before my changes and 53 failed / 379 passed after — the same 53 failures, plus my 4 new passes. make lint reports 182 pre-existing errors, all in tests/unit/ files I didn't touch. make typecheck fails on missing library stubs (PyPDF2, jose, passlib, rank_bm25) before reaching my code. My new test file passes ruff and black cleanly. None of these are affected by this change.

Why the bug was invisible. The probe's broad except Exception caught the AttributeError and set Redis to "unhealthy," so a config bug looked like an outage. Captured during the test run: redis_health_check_failed error="'Settings' object has no attribute 'redis_host'"

Out of scope. Postgres also reports unhealthy, from a different cause — await db.execute("SELECT 1") needs text() under SQLAlchemy 2.0. Left alone to keep this scoped to #155.

@salman-khan03 salman-khan03 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it correct

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 references settings.redis_host, which does not exist on Settings

2 participants