-
Notifications
You must be signed in to change notification settings - Fork 5
🚚(backend) move healthcheck route to api/version/ #348
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughHealthcheck endpoints were moved to a versioned API path; Docker Compose healthchecks updated to call Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Docker as Docker Healthcheck
participant Web as Web Server (nginx/gunicorn)
participant WSGI as QuietWSGIRequestHandler
participant Django as URL Router
participant View as Health View
Docker->>Web: GET /api/v1.0/healthz/
activate Web
Web->>WSGI: deliver request
WSGI->>Django: route request
Django->>View: call health view
View-->>Django: 200 OK (health status)
Django-->>WSGI: response
WSGI-->>Web: response to client
deactivate Web
Note over WSGI: suppress log if path.rstrip("/") == "/api/v1.0/healthz" (exact match)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used📓 Path-based instructions (1)src/backend/**/*.py📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
Files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
🔇 Additional comments (2)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Pylint (3.3.8)src/backend/messages/wsgi.pyThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
compose.yaml (1)
105-148
: Parameterize health-check URL with API_VERSIONHardcoding
/api/v1.0/healthz/
risks going out-of-sync withsettings.API_VERSION
, leading to failing health checks when the version changes. Please reference the environment variable (or reuse the same value) to keep compose aligned with Django settings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
compose.yaml
(2 hunks)src/backend/messages/urls.py
(1 hunks)src/backend/messages/wsgi.py
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/backend/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
src/backend/**/*.py
: Follow Django/PEP 8 style with a 100-character line limit
Use descriptive, snake_case names for variables and functions
Use Django ORM for database access; avoid raw SQL unless necessary for performance
Use Django’s built-in user model and authentication framework
Prefer try-except blocks to handle exceptions in business logic and views
Log expected and unexpected actions with appropriate log levels
Capture and report exceptions to Sentry; use capture_exception() for custom errors
Do not log sensitive information (tokens, passwords, financial/health data, PII)
Files:
src/backend/messages/urls.py
src/backend/messages/wsgi.py
src/backend/**/urls.py
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
Define clear, RESTful URL patterns using Django’s URL dispatcher
Files:
src/backend/messages/urls.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: docker-publish-socks-proxy / docker-build-push
- GitHub Check: check-api-state
- GitHub Check: build-front
- GitHub Check: lint-back
- GitHub Check: test-back
🔇 Additional comments (1)
src/backend/messages/urls.py (1)
20-23
: Ensure API version format matches compose health checksThe new route delegates to
settings.API_VERSION
, butcompose.yaml
hardcodesv1.0
. Ifsettings.API_VERSION
differs (e.g.,v1
), the Docker health checks will 404. Please confirm the setting or parameterize the compose file so both stay in sync.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/backend/messages/wsgi.py (1)
22-22
: Consider using settings.API_VERSION to avoid hardcoding.The AI summary mentions that Django routing constructs the health route from
settings.API_VERSION
. Hardcoding/api/v1.0/healthz
here could lead to inconsistencies if the API version changes in settings but this path is not updated.If
settings.API_VERSION
is available, apply this diff to construct the path dynamically:+from django.conf import settings + # pylint: disable=all # Suppress access logs for healthcheck route try: import django.core.servers.basehttp class QuietWSGIRequestHandler(django.core.servers.basehttp.WSGIRequestHandler): def log_message(self, format, *args): path = getattr(self, "path", "") parsed_path = path.split("?", 1)[0].rstrip("/") - if parsed_path == "/api/v1.0/healthz": + if parsed_path == f"/api/{settings.API_VERSION}/healthz": return super().log_message(format, *args)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/backend/messages/wsgi.py
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/backend/**/*.py
📄 CodeRabbit inference engine (.cursor/rules/django-python.mdc)
src/backend/**/*.py
: Follow Django/PEP 8 style with a 100-character line limit
Use descriptive, snake_case names for variables and functions
Use Django ORM for database access; avoid raw SQL unless necessary for performance
Use Django’s built-in user model and authentication framework
Prefer try-except blocks to handle exceptions in business logic and views
Log expected and unexpected actions with appropriate log levels
Capture and report exceptions to Sentry; use capture_exception() for custom errors
Do not log sensitive information (tokens, passwords, financial/health data, PII)
Files:
src/backend/messages/wsgi.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: docker-publish-socks-proxy / docker-build-push
- GitHub Check: build-front
- GitHub Check: test-back
Purpose
Move healthcheck to /api/version so that it can be used in scalingo
Summary by CodeRabbit