Observability: Sentry Cron #780
Conversation
📝 WalkthroughWalkthroughSentry monitoring was added to the evaluation cron endpoint and a new Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/core/config.py`:
- Around line 159-163: Add a lower-bound validator to CRON_INTERVAL_MINUTES so
zero/negative values are rejected: change the declaration of
CRON_INTERVAL_MINUTES in config.py to use Pydantic Field with default=5 and ge=1
(i.e., Field(default=5, ge=1)) and add the required import for Field from
pydantic at the top of the file; update any type hints that reference
CRON_INTERVAL_MINUTES if necessary to keep it as int.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 731b3b13-b9db-462f-bc48-fb11acc7ffe7
📒 Files selected for processing (2)
backend/app/api/routes/cron.pybackend/app/core/config.py
| # Evaluation cron invocation interval (minutes). In staging/production the | ||
| # endpoint is triggered by AWS EventBridge on this cadence; locally it can | ||
| # be driven by scripts/python/invoke-cron.py. The Sentry cron monitor reads | ||
| # this same value so its expected schedule stays aligned with the trigger. | ||
| CRON_INTERVAL_MINUTES: int = 5 |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What are the valid value constraints for Sentry Python SDK crons MonitorConfig interval schedules with type interval value unit minute?
💡 Result:
For Sentry Python SDK crons MonitorConfig with schedule type "interval", unit "minute", the "value" is a positive integer representing the number of minutes between executions. No explicit upper or lower bounds are documented in official sources, but practical constraints apply: Sentry limits check-ins to 6 per minute per monitor environment, implying a minimum practical value of 10 minutes (to allow for checkin_margin). Examples use values like 3 or 10 minutes. The value must be a non-negative integer (likely >0), as indicated by type definitions and SDK code references.
Citations:
- 1: https://develop.sentry.dev/sdk/telemetry/check-ins/
- 2: https://docs.sentry.io/platforms/javascript/guides/connect/crons/
- 3: https://docs.sentry.io/platforms/java/crons/
- 4: Enable Seconds interval for Cron monitor schedules getsentry/sentry#71333
- 5: https://github.com/getsentry/sentry-python/blob/master/sentry_sdk/_types.py
🏁 Script executed:
# Find usage of CRON_INTERVAL_MINUTES
rg "CRON_INTERVAL_MINUTES" --type py -B 2 -A 2Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 1774
🏁 Script executed:
# Check the config file for imports and current validation patterns
head -50 backend/app/core/config.pyRepository: ProjectTech4DevAI/kaapi-backend
Length of output: 1586
🏁 Script executed:
# Check if Field is already imported in config.py
rg "from pydantic import" backend/app/core/config.py -A 10Repository: ProjectTech4DevAI/kaapi-backend
Length of output: 322
Add lower bound validation to CRON_INTERVAL_MINUTES to prevent invalid Sentry monitor config.
CRON_INTERVAL_MINUTES is environment-overridable but currently accepts any integer value. Setting it to 0 or negative values would create invalid Sentry monitor schedule configuration (used as interval "value" and in "max_runtime" calculation). Sentry requires positive interval values.
Apply Field(default=5, ge=1) with the necessary import addition.
Suggested fix
from pydantic import (
EmailStr,
+ Field,
HttpUrl,
PostgresDsn,
computed_field,
model_validator,
)
@@
- CRON_INTERVAL_MINUTES: int = 5
+ CRON_INTERVAL_MINUTES: int = Field(default=5, ge=1)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Evaluation cron invocation interval (minutes). In staging/production the | |
| # endpoint is triggered by AWS EventBridge on this cadence; locally it can | |
| # be driven by scripts/python/invoke-cron.py. The Sentry cron monitor reads | |
| # this same value so its expected schedule stays aligned with the trigger. | |
| CRON_INTERVAL_MINUTES: int = 5 | |
| # Evaluation cron invocation interval (minutes). In staging/production the | |
| # endpoint is triggered by AWS EventBridge on this cadence; locally it can | |
| # be driven by scripts/python/invoke-cron.py. The Sentry cron monitor reads | |
| # this same value so its expected schedule stays aligned with the trigger. | |
| CRON_INTERVAL_MINUTES: int = Field(default=5, ge=1) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend/app/core/config.py` around lines 159 - 163, Add a lower-bound
validator to CRON_INTERVAL_MINUTES so zero/negative values are rejected: change
the declaration of CRON_INTERVAL_MINUTES in config.py to use Pydantic Field with
default=5 and ge=1 (i.e., Field(default=5, ge=1)) and add the required import
for Field from pydantic at the top of the file; update any type hints that
reference CRON_INTERVAL_MINUTES if necessary to keep it as int.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
2 participants
Summary
Target issue is #776
Description