Improved timezones handling in next_run

[SijmenHuizenga]

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 to self.at_time_zone and then converted to local time. In the next condition, the code uses datetime.datetime.now(), which fetches the current time in the local timezone. If the system's local timezone is different from self.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.

[coveralls]

coverage: 99.474% (-0.3%) from 99.726% when pulling 418693be717ad78aa7ff8fa8568c6794be9a80f3 on SijmenHuizenga:timezone-fix-simplify into 073dbc6e5f8254d27c656f88a5062cca2cdfc4a2 on dbader:master.

[sosdarko]

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. When I want to test if execution of the job is correct, I put some close time as scheduled time, but due to this bug, it is scheduled for next day. I found the reason and modify code in _schedule_next_run so it takes into account time zone set when schedule was defined. That's around lines where you ask is it the first run (last_run is None), and compare self.at_time with now.time(). This comparison doesn't take into account 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 :)

[SijmenHuizenga]

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?

[sosdarko]

Hi @SijmenHuizenga , yes I can do both, should I take your last commit? Is it this one: https://github.com/dbader/schedule/pull/604/commits/e7e8be6e20062a03c8b0157099a443ab041f3bf9

[SijmenHuizenga]

@sosdarko Amazing! Yes, the last one is the one to take: https://github.com/dbader/schedule/pull/604/commits/51d762e16f7fca41e5ebe54414667c9f55e13f66

[sosdarko]

next_time is calculated right, but in execution I got this:

init.py", line 768, in _schedule_next_run if not self.last_run or (self.next_run - self.last_run) > self.period:

TypeError: can't subtract offset-naive and offset-aware datetimes

[SijmenHuizenga]

@sosdarko good find, the unit tests apparently do not cover timezone usage with multiple runs :sweat:. Will fix this later today or tomorrow.