-
Notifications
You must be signed in to change notification settings - Fork 970
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
Improved timezones handling in next_run #604
Improved timezones handling in next_run #604
Conversation
I had a similar problem, but kind of opposite... I use every(day).at(...) and UTC times, but my machine is in different time zone. This is important not only for testing, but for restarts, because with this bug, when I restart process that runs schedule, I miss some of the scheduled executions. @SijmenHuizenga, thanks for rectifying this, hope that next version will be even better :) |
Thank you @sosdarko for explaining your use-case! I'm also looking forward to releasing a version that resolves this bug. Before I go forward with merging this, I want to be sure this fix resolves the the issues. Would you be willing to help test the fix in this pr? Either by running it as part of your program, or by writing a unit-test like the these ones that represents your situation? |
Hi @SijmenHuizenga , yes I can do both, should I take your last commit? Is it this one: e7e8be6 |
next_time is calculated right, but in execution I got this: init.py", line 768, in _schedule_next_run |
@sosdarko good find, the unit tests apparently do not cover timezone usage with multiple runs 😓. Will fix this later today or tomorrow. |
This improves how the timezone configured in
.at("18:00", "Europe/Amsterdam")
is processed.The problem is that the
self.next_run value
is localized toself.at_time_zone
and then converted to local time. In the next condition, the code usesdatetime.datetime.now()
, which fetches the current time in the local timezone. If the system's local timezone is different fromself.at_time_zone
, this can result in a discrepancy and cause the computation of self.next_run to be off by a day or more. This is what is happening in #603.This PR changes the timezone-handling approach to use localized datetimes throughout the whole
_schedule_next_run
function.PR created as draft since there are still a few untested cases that I would like to add tests for before merging. Especially in the area of combining
.at()
with weekdays,until()
and daylight-saving-time.