feat: health check endpoint and API versioning under /api/v1/#17
feat: health check endpoint and API versioning under /api/v1/#17sadykovIsmail merged 1 commit intomainfrom
Conversation
- Add health/ app with GET /api/health/ endpoint (no auth required)
- Returns {status: ok, database: ok} on 200
- Returns {status: degraded, database: unavailable} on 503 when DB is down
- Suitable for k8s liveness/readiness probes and load balancer health checks
- Add health tests: happy path, unauthenticated access, DB failure simulation
- Version all API routes under /api/v1/user/ and /api/v1/recipe/
- Keep unversioned /api/user/ and /api/recipe/ as deprecated aliases for backwards compat
- Register health app in INSTALLED_APPS
Closes #8
There was a problem hiding this comment.
Pull request overview
Adds a lightweight health check endpoint for load balancer / Kubernetes probes and introduces /api/v1/ routing for existing APIs while keeping legacy unversioned routes as aliases.
Changes:
- Added
GET /api/health/endpoint that performs a simple DB connectivity check and returns200/503. - Added test coverage for the new health endpoint behavior.
- Exposed all existing
userandreciperoutes under/api/v1/and kept/api/*as deprecated aliases.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| app/health/views.py | Implements DRF health check view with DB probe and status payload. |
| app/health/tests.py | Adds tests for health endpoint success, unauthenticated access, and DB-down behavior. |
| app/app/urls.py | Wires in /api/health/ and introduces /api/v1/ route prefixes plus legacy aliases. |
| app/app/settings.py | Registers the new health Django app. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| path('api/user/', include('user.urls')), | ||
| path('api/recipe/', include('recipe.urls')), |
There was a problem hiding this comment.
Including user.urls and recipe.urls twice will register the same URL namespace (user / recipe) multiple times (those modules define app_name). This triggers Django's non-unique namespace check and can make reverse('user:...') / reverse('recipe:...') ambiguous or resolve unpredictably. Use distinct namespaces for the deprecated aliases (e.g., include('user.urls', namespace='user-legacy') / include('recipe.urls', namespace='recipe-legacy')) so the primary user: / recipe: names reverse to the v1 routes without duplication.
| path('api/user/', include('user.urls')), | |
| path('api/recipe/', include('recipe.urls')), | |
| path('api/user/', include('user.urls', namespace='user-legacy')), | |
| path('api/recipe/', include('recipe.urls', namespace='recipe-legacy')), |
| 'core', | ||
| 'user', | ||
| 'recipe', | ||
| 'health', | ||
| ] |
There was a problem hiding this comment.
The PR/issue description mentions adding an API_VERSION setting, but no such setting is introduced. Consider adding something like API_VERSION = 'v1' in settings and using it to build the versioned URL prefixes (and related settings like SPECTACULAR_SETTINGS['SCHEMA_PATH_PREFIX']) to avoid hardcoding v1 in multiple places.
| from django.db import connection, OperationalError | ||
|
|
There was a problem hiding this comment.
This imports OperationalError from django.db, but elsewhere in the codebase the convention is from django.db.utils import OperationalError (e.g., core/management/commands/wait_for_db.py). Importing from django.db.utils is the canonical location and avoids surprises across Django versions/backends.
| from django.db import connection, OperationalError | |
| from django.db import connection | |
| from django.db.utils import OperationalError |
|
|
||
| return Response( | ||
| { | ||
| 'status': 'ok' if http_status == 200 else 'degraded', |
There was a problem hiding this comment.
The response sets 'status' based on http_status == 200, which hardcodes the numeric status code even though http_status is already a DRF constant. Prefer comparing against status.HTTP_200_OK (or deriving the overall status from db_status) to avoid magic numbers and keep the logic self-explanatory.
| 'status': 'ok' if http_status == 200 else 'degraded', | |
| 'status': 'ok' if http_status == status.HTTP_200_OK else 'degraded', |
|
|
||
| def test_health_check_db_down_returns_503(self): | ||
| """When the database is unreachable the endpoint returns 503.""" | ||
| from django.db import OperationalError |
There was a problem hiding this comment.
For consistency with the rest of the repo (e.g., core/tests/test_commands.py), import OperationalError from django.db.utils rather than django.db in this test as well.
| from django.db import OperationalError | |
| from django.db.utils import OperationalError |
Summary
Adds a production-essential health check endpoint and versions all API routes under
/api/v1/.Health Check
SELECT 1200when healthy,503when DB is unreachableAPI Versioning
All routes now available under
/api/v1/:/api/v1/user/→ user endpoints/api/v1/recipe/→ recipe endpointsOld
/api/user/and/api/recipe/kept as deprecated aliases so existing integrations don't break.Test plan
GET /api/health/returns 200 without authGET /api/v1/recipe/recipes/works with JWTGET /api/recipe/recipes/still works (backwards-compat)Closes #8