taichino / croniter

croniter is a python module to provide iteration for datetime object.
http://github.com/taichino/croniter
387 stars 105 forks source link

DST - croniter gets stuck iterating across DST change #137

Closed pacificsky closed 2 years ago

pacificsky commented 4 years ago

Hello! I wanted to report a DST issue with croniter that seems to be persistent across the recent 0.3.32 release (I noticed https://github.com/taichino/croniter/issues/90 was closed but I can repro this quite easily).

The basic issue seems to be that iterating across a DST boundary in a DST-aware time causes croniter to get stuck. Here's an example:

from croniter import croniter
from datetime import datetime, timedelta, timezone
from dateutil import tz

def iterate_across_dst_change():
    localtz = tz.gettz("America/Los_Angeles")
    start = datetime(year=2020, month=3, day=7, hour=21, tzinfo=localtz)
    # DST change happened at 2am on March 8 2020
    end = datetime(year=2020, month=3, day=8, hour=9, tzinfo=localtz)

    iter = croniter("0 */1 * * *", start)
    print(f"Starting at {start}")
    # Get step values as datetime objects
    for i in range(15):
        step = iter.get_next(ret_type=datetime)
        print(step)
    print(f"Finished at {end}")

if __name__ == "__main__":
    iterate_across_dst_change()

The expected output is for croniter to iterate and produce datetime values with hourly offsets. However this is the actual output:

Starting at 2020-03-07 21:00:00-08:00
2020-03-07 22:00:00-08:00
2020-03-07 23:00:00-08:00
2020-03-08 00:00:00-08:00
2020-03-08 01:00:00-08:00
2020-03-08 01:00:00-08:00
2020-03-08 01:00:00-08:00
2020-03-08 01:00:00-08:00
2020-03-08 01:00:00-08:00
2020-03-08 01:00:00-08:00
2020-03-08 01:00:00-08:00
2020-03-08 01:00:00-08:00
2020-03-08 01:00:00-08:00
2020-03-08 01:00:00-08:00
2020-03-08 01:00:00-08:00
2020-03-08 01:00:00-08:00
Finished at 2020-03-08 09:00:00-07:00

Croniter seems to get stuck at 01:00 and doesn't make forward progress. Given this, it seems all of the issues mentioned in https://github.com/taichino/croniter/issues/90 aren't fully resolved.

kiorky commented 4 years ago

During the one hour DST changes, we wont do anything good i think. Trigger before, or after.

lowell80 commented 4 years ago

@pacificsky, for whatever it's worth, if you use pytz.timezone() instead of datetime.tz.gettz() this example seems to work.

from croniter import croniter
from datetime import datetime, timedelta, timezone
import pytz

def iterate_across_dst_change():
    localtz = pytz.timezone("America/Los_Angeles")
    start = localtz.localize(datetime(year=2020, month=3, day=7, hour=21))
    # DST change happened at 2am on March 8 2020
    end = localtz.localize(datetime(year=2020, month=3, day=8, hour=9))

    iter = croniter("0 */1 * * *", start)
    print("Starting at {}".format(start))
    # Get step values as datetime objects
    for i in range(15):
        step = iter.get_next(ret_type=datetime)
        print(step)
    print("Finished at {}".format(end))

if __name__ == "__main__":
    iterate_across_dst_change()

Output:

Starting at 2020-03-07 21:00:00-08:00
2020-03-07 22:00:00-08:00
2020-03-07 23:00:00-08:00
2020-03-08 00:00:00-08:00
2020-03-08 01:00:00-08:00
2020-03-08 02:00:00-07:00
2020-03-08 03:00:00-07:00
2020-03-08 04:00:00-07:00
2020-03-08 05:00:00-07:00
2020-03-08 06:00:00-07:00
2020-03-08 07:00:00-07:00
2020-03-08 08:00:00-07:00
2020-03-08 09:00:00-07:00
2020-03-08 10:00:00-07:00
2020-03-08 11:00:00-07:00
2020-03-08 12:00:00-07:00
Finished at 2020-03-08 09:00:00-07:00

UPDATE: I was incorrectly passing it pytz timezones into tzinfo, hence the weird -07:53 timezone listed on the last line. I'd updated the example to use tz.localize() instead.

lowell80 commented 4 years ago

@kiorky, is using dateutil.tz supported by croniter? The readme just says you need a "TZ aware datetime" but doesn't name any specific packages. I think for python the 3 popular options are just pytz, dateutil, and tzlocal, right?

I noticed there are no unit test that currently use dateutil. Would you be open to a PRs that adds tests against that library? (Possibly that may help with issues like the one reported here, and possibly avoid future regressions.)

As far as tzlocal, I see that it's imported in the unittest script, but it doesn't seem to be used in any test. I haven't looked through the commit history, but I'm guessing the challenge with tzlocal is that unittesting on something you can't control makes it difficult to know what to expect the result to be.

kiorky commented 4 years ago

No problem if you do it :-), totally open to PR's

Remember to open them on my fork.

kiorky commented 2 years ago

see https://github.com/kiorky/croniter/issues/1