feat(config): add health check endpoint#3
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a dedicated health check endpoint at /health/ for container monitoring and updates Docker health check configurations to use this new endpoint. The changes migrate from checking the root URL to a proper health check endpoint that verifies database connectivity, supporting better infrastructure monitoring and orchestration.
Key changes:
- Adds
/health/endpoint that checks database connectivity and returns JSON status - Updates Docker and docker-compose health checks to use the new endpoint
- Removes unused
main.pyfile
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| config/views.py | Implements health check view with database connectivity verification |
| config/urls.py | Registers the health check endpoint at /health/ path |
| docker-compose.yml | Updates app service health check to use /health/ endpoint |
| Dockerfile | Updates HEALTHCHECK instruction to use /health/ endpoint |
| main.py | Removes unused standalone script file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
|
|
There was a problem hiding this comment.
The health check endpoint should be decorated with @csrf_exempt to allow health check requests without CSRF tokens. Health monitoring tools typically don't include CSRF tokens in their requests, which would cause this endpoint to fail with a 403 Forbidden error.
Add the decorator:
from django.views.decorators.csrf import csrf_exempt
@csrf_exempt
def health_check(request):| from django.views.decorators.csrf import csrf_exempt | |
| @csrf_exempt |
|
|
||
|
|
There was a problem hiding this comment.
The health check endpoint should restrict requests to GET method only. Health checks should not accept POST, PUT, DELETE, or other HTTP methods.
Consider using the @require_http_methods or @require_GET decorator:
from django.views.decorators.http import require_GET
@require_GET
def health_check(request):| from django.views.decorators.http import require_GET | |
| @require_GET |
| except Exception as e: | ||
| return JsonResponse({"status": "unhealthy", "error": str(e)}, status=503) |
There was a problem hiding this comment.
The bare except Exception catches all exceptions, including database operational errors that might reveal sensitive information. The error message str(e) could expose database credentials, connection strings, or internal system details in the health check response.
Consider logging the detailed error internally while returning a generic error message:
import logging
logger = logging.getLogger(__name__)
def health_check(request):
"""..."""
try:
# Check database connectivity
with connection.cursor() as cursor:
cursor.execute("SELECT 1")
return JsonResponse({"status": "healthy", "database": "connected"})
except Exception as e:
logger.error(f"Health check failed: {e}", exc_info=True)
return JsonResponse({"status": "unhealthy", "database": "disconnected"}, status=503)8d890cb to
afa9d87
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| @csrf_exempt |
There was a problem hiding this comment.
The @csrf_exempt decorator is redundant for GET requests. CSRF protection only applies to state-changing HTTP methods (POST, PUT, DELETE, etc.) in Django. GET requests are not subject to CSRF validation by default, so this decorator has no effect here. Consider removing it for cleaner code.
| @csrf_exempt |
| def health_check(request): | ||
| try: | ||
| with connection.cursor() as cursor: | ||
| cursor.execute("SELECT 1") |
There was a problem hiding this comment.
[nitpick] While executing "SELECT 1" verifies database connectivity, consider fetching the result with cursor.fetchone() to ensure the query completes fully. This provides a more thorough health check:
cursor.execute("SELECT 1")
cursor.fetchone()| cursor.execute("SELECT 1") | |
| cursor.execute("SELECT 1") | |
| cursor.fetchone() |
afa9d87 to
0b90e05
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from django.views.decorators.http import require_GET | ||
| import logging | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
There was a problem hiding this comment.
The health check endpoint should be exempted from CSRF protection. Health check endpoints are typically called by monitoring systems, load balancers, and Docker/Kubernetes health checkers that don't have CSRF tokens. While @require_GET prevents modifications, the CSRF middleware will still reject requests without proper tokens.
Add @csrf_exempt decorator before @require_GET:
from django.views.decorators.csrf import csrf_exempt
@csrf_exempt
@require_GET
def health_check(request):| from django.views.decorators.http import require_GET | |
| import logging | |
| logger = logging.getLogger(__name__) | |
| from django.views.decorators.http import require_GET | |
| from django.views.decorators.csrf import csrf_exempt | |
| import logging | |
| logger = logging.getLogger(__name__) | |
| @csrf_exempt |
No description provided.