-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Crons: croniter
library does not correctly compute DST transition for some schedules
#66868
Comments
I've begun investigating this. My initial investigation looks like this actually has to do with the |
I'm investigating moving us over to the |
Here's some test code I've written to see how many cron expressions are parsable from datetime import datetime, UTC
from cronsim import CronSim
from croniter.croniter import croniter
def validate_cron_expressions(file_path):
valid_count = 0
invalid_expressions = []
dt = datetime.now(UTC).replace(second=0, microsecond=0)
print(f'It is {dt}')
print('')
try:
with open(file_path, "r") as file:
for line_num, expression in enumerate(file, start=1):
cron_expr = expression.strip()
try:
# Attempt to parse and simulate the cron expression
cron_sim = CronSim(cron_expr, dt)
cron_iter = croniter(cron_expr, dt)
valid_count += 1
except Exception as e:
# Store invalid expressions with line number and error
invalid_expressions.append((line_num, cron_expr, str(e)))
# Print results
print(f"Total valid expressions: {valid_count}")
if invalid_expressions:
print("\nInvalid expressions:")
for line_num, cron_expr, error in invalid_expressions:
print(f"Line {line_num}: '{cron_expr}' - Error: {error}")
else:
print("No invalid expressions found.")
except FileNotFoundError:
print("The specified file was not found.")
# Replace 'cron_expressions.txt' with your actual file path
validate_cron_expressions("crons.txt") For all our currently existing (~ 7K unique cron expressions) here's what cronsim can't parse
|
Okay. Since we're running out of time until the DST end time (which is mostly inconsequential, since the computed time ends up just being an hour late) we were going to see if we could hack-in something in to skip misses during the standard time → daylight savings time transtion. However, turns out we've already missed all those transitions in the southern hemisphere. Here's some chat-gpt code that finds all the DST transitions starting 30 days back and looking ahead 60 days. from datetime import datetime, timedelta
from zoneinfo import ZoneInfo, available_timezones
def find_upcoming_dst_transitions():
# Set the time window (next 7 days)
now = datetime.now() - timedelta(days=30)
upcoming_transitions = []
print(f'Start time is {now}')
print('-----')
# Check each timezone
for timezone in available_timezones():
tz = ZoneInfo(timezone)
# Start with today at midnight in the specified timezone
dt = now.replace(hour=0, minute=0, second=0, microsecond=0, tzinfo=tz)
one_month = dt + timedelta(days=60)
# Loop through each hour in the next week to detect transitions
while dt < one_month:
# Check if the DST offset changes between this hour and the next
next_hour = dt + timedelta(hours=1)
if dt.dst() != next_hour.dst():
# Record the timezone and the exact transition time
transition_type = "IN to DST" if next_hour.dst() > dt.dst() else "OUT of DST"
upcoming_transitions.append(
(timezone, next_hour, transition_type))
break # No need to check further within this timezone
dt = next_hour
return upcoming_transitions
# Example usage:
upcoming_dst_transitions = find_upcoming_dst_transitions()
upcoming_dst_transitions.sort(key=lambda k: k[2])
for tz, transition_time, transition_type in upcoming_dst_transitions:
print(f"{transition_type}: {tz} at {transition_time}") For the transition into DST, we already missed all of these
And again, leaving DST isn't as important, since it just means we say the next checkin can happen an hour later than it should. Going forward I am going to spend some time trying to migrate us to cronsim since that is a much more modern implementation of what we need. |
As reported here:
What's wrong
What is happening is schedules such as
0 5 * * *
compute the wrongnext_expected_checkin
during DST transitions. This is a bug in thecroniter
library we're using.Effect on customers
When DST starts we will compute next expected check-in an hour early, we then create a missed check-in for the monitor an hour early.
When DST ends we compute the next expected check-in an hour late. In this scenario we may not completely fail, since a customers check-in will simply come early (and because right now we don't treat early/late check-ins differently #44393 this is OK) it may re-compute the next check-in time to be the next correct occurrence.
After the first incorrect computation the next computation of the expected check-in time will be correct.
Upstream bug
This is a bug in croniter.
There are a number of reports to the library and it's forks:
Some work was done here to attempt to fix it, but it was never merged https://github.com/kiorky/croniter-fork/pull/34
It appears the original problem was introduced here taichino/croniter#92 Which incorrectly introduced this bug.
The text was updated successfully, but these errors were encountered: