Skip to content

Commit

Permalink
ref(py): Replace usage of croniter with cronsim
Browse files Browse the repository at this point in the history
We're replacing croniter with cronsim as it's bit more of a modern
library. This replaces the single usage outsode of the cron monitors
feature of sentry.

See GH-80208
  • Loading branch information
evanpurkhiser committed Nov 6, 2024
1 parent 935e2ba commit 6955235
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 19 deletions.
21 changes: 12 additions & 9 deletions src/sentry/api/helpers/deprecation.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,14 @@
import functools
import logging
from collections.abc import Callable
from datetime import datetime, timedelta, timezone
from datetime import datetime, timedelta
from typing import Concatenate, ParamSpec, TypeVar

import isodate
from croniter import croniter
from cronsim import CronSim, CronSimError
from django.conf import settings
from django.http.response import HttpResponseBase
from django.utils import timezone
from isodate.isoerror import ISO8601Error
from rest_framework.request import Request
from rest_framework.response import Response
Expand Down Expand Up @@ -66,17 +67,19 @@ def _should_be_blocked(deprecation_date: datetime, now: datetime, key: str) -> b
logger.exception("Invalid ISO8601 format for blackout duration")
return False

if not croniter.is_valid(brownout_cron):
try:
# Move back one minute so we can iterate once and determine if now
# matches the cron schedule.
iter = CronSim(brownout_cron, now - timedelta(minutes=1))
except CronSimError:
logger.error("Invalid crontab for blackout schedule")
return False

# return True if now exactly matches the crontab
if croniter.match(brownout_cron, now):
if next(iter) == now.replace(second=0, microsecond=0):
return True

# If not, check if now is within BROWNOUT_DURATION of the last brownout time
iter = croniter(brownout_cron, now)
brownout_start = iter.get_prev(datetime)
# If not, check if now is within `brownout_duration` of the last brownout time
brownout_start = next(CronSim(brownout_cron, now, reverse=True))
return brownout_start <= now < brownout_start + brownout_duration
return False

Expand Down Expand Up @@ -131,7 +134,7 @@ def endpoint_method(
if is_self_hosted() and not settings.ENVIRONMENT == "development":
return func(self, request, *args, **kwargs)

now = datetime.now(timezone.utc)
now = timezone.now()

if now > deprecation_date and _should_be_blocked(deprecation_date, now, key):
response: HttpResponseBase = Response(GONE_MESSAGE, status=GONE)
Expand Down
20 changes: 10 additions & 10 deletions tests/sentry/helpers/test_deprecation.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from datetime import datetime, timedelta

import isodate
from croniter import croniter
from cronsim import CronSim
from django.http.request import HttpRequest
from django.http.response import HttpResponse
from rest_framework.response import Response
Expand All @@ -18,7 +18,7 @@

replacement_api = "replacement-api"
test_date = datetime.fromisoformat("2020-01-01T00:00:00+00:00:00")
timeiter = croniter("0 12 * * *", test_date)
timeiter = CronSim("0 12 * * *", test_date)
default_duration = timedelta(minutes=1)

custom_cron = "30 1 * * *"
Expand Down Expand Up @@ -86,7 +86,7 @@ def test_after_deprecation_date(self):
with freeze_time(test_date):
self.assert_allowed_request("GET")

brownout_start = timeiter.get_next(datetime)
brownout_start = next(timeiter)
with freeze_time(brownout_start):
self.assert_denied_request("GET")

Expand Down Expand Up @@ -119,13 +119,13 @@ def test_default_key(self):
options.delete("api.deprecation.brownout-cron")
options.delete("api.deprecation.brownout-duration")

custom_time_iter = croniter(custom_cron, test_date)
custom_time_iter = CronSim(custom_cron, test_date)
custom_duration_timedelta = isodate.parse_duration(custom_duration)
old_brownout_start = timeiter.get_next(datetime)
old_brownout_start = next(timeiter)
with freeze_time(old_brownout_start):
self.assert_allowed_request("GET")

new_brownout_start = custom_time_iter.get_next(datetime)
new_brownout_start = next(custom_time_iter)
with freeze_time(new_brownout_start):
self.assert_denied_request("GET")

Expand All @@ -141,19 +141,19 @@ def test_custom_key(self):
with self.settings(
SENTRY_SELF_HOSTED=False,
):
old_brownout_start = timeiter.get_next(datetime)
old_brownout_start = next(timeiter)
with freeze_time(old_brownout_start):
self.assert_denied_request("POST")

register("override-cron", default=custom_cron)
register("override-duration", default=custom_duration)
custom_time_iter = croniter(custom_cron, test_date)
custom_time_iter = CronSim(custom_cron, test_date)
custom_duration_timedelta = isodate.parse_duration(custom_duration)

with freeze_time(old_brownout_start):
self.assert_allowed_request("POST")

new_brownout_start = custom_time_iter.get_next(datetime)
new_brownout_start = next(custom_time_iter)
with freeze_time(new_brownout_start):
self.assert_denied_request("POST")

Expand All @@ -162,7 +162,7 @@ def test_custom_key(self):
self.assert_allowed_request("POST")

def test_bad_schedule_format(self):
brownout_start = timeiter.get_next(datetime)
brownout_start = next(timeiter)
with freeze_time(brownout_start):
with (
self.settings(SENTRY_SELF_HOSTED=False),
Expand Down

0 comments on commit 6955235

Please sign in to comment.