-
Notifications
You must be signed in to change notification settings - Fork 507
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
Do not support sub-minute cron intervals #2172
Do not support sub-minute cron intervals #2172
Conversation
@@ -374,7 +374,7 @@ def _get_humanized_interval(seconds): | |||
interval = int(seconds / divider) | |||
return (interval, unit) | |||
|
|||
return (1, "minute") |
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.
Before we just set everything that is under one minute to exactly one minute. Now We return the correct second interval.
if schedule_unit == "second": | ||
logger.warning( | ||
"Intervals shorter than one minute are not supported by Sentry Crons." | ||
) | ||
return {} |
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.
And when a second interval is returned, we print a warning and do not return a monitor config (this is the behavior the backend guys requested, because then they ignore the checkins)
is_supported_schedule = bool(monitor_config) | ||
if is_supported_schedule: |
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.
We now only create the "in_progress" checkin for supported schedule types (so if the schedule is sub-minute interval, the check in will not be sent)
Also because the headers are not set in case it is an unsupported type, the ok/failure checkins will also not be sent.
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.
lgtm
In Celery you can run task in intervals shorter than one minute. Sentry crons does not support this, so in a case where a sub-minute interval is specified, just print a warning and do not set a schedule configuration.
Fixes #2168