Feature/heartbit endpoint#28
Merged
Merged
Conversation
Contributor
Руководство для ревьюера (свернуто для небольших PR)Руководство для ревьюераДобавляет отдельный эндпоинт Диаграмма последовательности для Docker health check с использованием нового эндпоинта /healthsequenceDiagram
participant Docker
participant App
Docker->>App: GET /health
App->>App: health_checker(request)
App-->>Docker: {status: ok}
Изменения на уровне файлов
Подсказки и командыВзаимодействие с Sourcery
Настройка работыЗайдите в свою панель управления, чтобы:
Получение помощи
Original review guide in EnglishReviewer's guide (collapsed on small PRs)Reviewer's GuideAdds a dedicated /health endpoint for application health checks and updates Docker-related configuration to use it and improve container behavior. Sequence diagram for Docker health check using the new /health endpointsequenceDiagram
participant Docker
participant App
Docker->>App: GET /health
App->>App: health_checker(request)
App-->>Docker: {status: ok}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey — я нашёл одну проблему и оставил несколько общих замечаний:
- Эндпоинт
/healthограничен по частоте до5/minutes, в то время как Docker healthcheck запускается каждые 10 секунд с повторами, что легко может превысить лимит и привести к ложным состояниям «unhealthy». Подумайте о том, чтобы убрать лимитер для этого эндпоинта или ослабить лимит, приведя его в соответствие с частотой healthcheck. - Поскольку
/healthпредназначен только для проверки «живости» контейнера, возможно, стоит убедиться, что он остаётся максимально лёгким (без лишнего middleware или тяжёлых зависимостей) и всегда явно возвращает статус 200 OK для более понятной интеграции с оркестраторами.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `/health` endpoint is rate-limited to `5/minutes` while the Docker healthcheck runs every 10 seconds with retries, which can easily exceed the limit and cause false unhealthy states; consider removing the limiter for this endpoint or loosening the limit to align with the healthcheck frequency.
- Since `/health` is intended only for a container liveness check, you might want to ensure it stays as lightweight as possible (no extra middleware or heavy dependencies) and returns an explicit 200 OK status for clearer integration with orchestration tools.
## Individual Comments
### Comment 1
<location path="main.py" line_range="65-66" />
<code_context>
}
+
+@app.get("/health")
+@limiter.limit("5/minutes")
+def health_checker(request: Request):
+ return {"status": "ok"}
</code_context>
<issue_to_address>
**issue (bug_risk):** Rate limiting the `/health` endpoint conflicts with the healthcheck frequency and can mark the container unhealthy.
The Docker healthcheck runs every 10 seconds (up to 6 calls/min), but this endpoint is limited to `5/minutes`. After the limit is hit, healthchecks will get 429s and may mark the container unhealthy. For health probes, consider removing the limit or using a much higher/independent limit so operational checks are never throttled.
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The
/healthendpoint is rate-limited to5/minuteswhile the Docker healthcheck runs every 10 seconds with retries, which can easily exceed the limit and cause false unhealthy states; consider removing the limiter for this endpoint or loosening the limit to align with the healthcheck frequency. - Since
/healthis intended only for a container liveness check, you might want to ensure it stays as lightweight as possible (no extra middleware or heavy dependencies) and returns an explicit 200 OK status for clearer integration with orchestration tools.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `/health` endpoint is rate-limited to `5/minutes` while the Docker healthcheck runs every 10 seconds with retries, which can easily exceed the limit and cause false unhealthy states; consider removing the limiter for this endpoint or loosening the limit to align with the healthcheck frequency.
- Since `/health` is intended only for a container liveness check, you might want to ensure it stays as lightweight as possible (no extra middleware or heavy dependencies) and returns an explicit 200 OK status for clearer integration with orchestration tools.
## Individual Comments
### Comment 1
<location path="main.py" line_range="65-66" />
<code_context>
}
+
+@app.get("/health")
+@limiter.limit("5/minutes")
+def health_checker(request: Request):
+ return {"status": "ok"}
</code_context>
<issue_to_address>
**issue (bug_risk):** Rate limiting the `/health` endpoint conflicts with the healthcheck frequency and can mark the container unhealthy.
The Docker healthcheck runs every 10 seconds (up to 6 calls/min), but this endpoint is limited to `5/minutes`. After the limit is hit, healthchecks will get 429s and may mark the container unhealthy. For health probes, consider removing the limit or using a much higher/independent limit so operational checks are never throttled.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Coverage Report for CI Build 26686320878Coverage remained the same at 27.887%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
…ed by RateLimiting
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by Sourcery
Добавить отдельную конечную точку для проверки состояния сервиса и интегрировать её в конфигурацию проверки работоспособности.
Новые возможности:
/health, который возвращает простой статус OK для проверок «живости» (liveness checks).Улучшения:
/healthвместо URL документации.Original summary in English
Summary by Sourcery
Add a dedicated health check endpoint and integrate it into the service health configuration.
New Features:
Enhancements: