taichino / croniter

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

Possible error near DST? #90

Closed Bug-Spray closed 4 years ago

Bug-Spray commented 7 years ago

I am calculating the last date range based on a cron schedule. The functionality needs to be DST aware, and the cron schedule is specified in "local time", although the range values should be UTC for consistency throughout the rest of the app.

Here's a brief script to illustrate this behaviour

from datetime import datetime
import pytz
from croniter import croniter

def main():
    base = '00 00 * * 6'
    now = datetime(2017, 10, 3, 2, 07).replace(tzinfo=pytz.UTC)
    iter = localtz_iterator(now, base)
    last_window_utc(iter)

def localtz_iterator(now, base):
    tz = pytz.timezone('Australia/Sydney')  # usually coming from some other database field, not
    print 'initialising cron-iterator for {}'.format(tz)
    reference_localtz = now.astimezone(pytz.timezone('Australia/Sydney'))
    print 'local time is {} ({})'.format(reference_localtz, now)

    return croniter(base, reference_localtz)

def last_window_utc(iter):
    closing_marker = iter.get_prev(datetime)
    opening_marker = iter.get_prev(datetime)

    print 'opening record marker {} ({})'.format(opening_marker, opening_marker.astimezone(pytz.UTC))
    print 'closing record marker {} ({})'.format(closing_marker, closing_marker.astimezone(pytz.UTC))

    recalc_closing_marker = iter.get_next(datetime)
    assert closing_marker == recalc_closing_marker

    return (opening_marker, closing_marker)

if __name__ == '__main__':
    main()

So, my date range is expected to go from opening_marker to closing_marker - if the assertion is commented out, the closing marker is at 23:00 (i.e. 11pm) local time, even though the cron schedule is explicit that it should be at midnight. The recalc_closing_marker produces the correct value, by moving the marker forward once again...

All the same, thanks for an awesome module - this exactly fills the requirements I have in my project!

Bug-Spray commented 7 years ago

By tweaking the value of now in main(), I think I can show that this is linked to DST - moving one week forward to 10 Oct still produces the issue (where opening_marker and closing_marker are on either side of the DST change), moving two weeks forward, the assertion passes. Likewise, the assertion passes if I initialise with a datetime which is before the DST change over. It seems to occur when the internal marker moves across that DST boundary...

kbrose commented 6 years ago

Is this resolved by the new release (https://pypi.python.org/pypi/croniter/0.3.20)? (https://github.com/taichino/croniter/pull/92)

ashb commented 5 years ago

I'm not sure if this is the same issue, or another one, but having a cron schedule of 3AM (the CEST -> CST change over time) seems to have on errant response on get_prev

In [1]: import pytz

In [2]: from datetime import datetime

In [3]: from croniter import croniter

In [4]: tz = pytz.timezone('Europe/Paris')

In [5]: start_local = tz.localize(datetime(2018, 10, 28, 3, 0))

n [6]: iter = croniter('0 3 * * *', start_local)

In [7]: iter.get_prev(datetime)
Out[7]: datetime.datetime(2018, 10, 27, 4, 0, tzinfo=<DstTzInfo 'Europe/Paris' CEST+2:00:00 DST>)

In [8]: iter.get_prev(datetime)
Out[8]: datetime.datetime(2018, 10, 27, 3, 0, tzinfo=<DstTzInfo 'Europe/Paris' CEST+2:00:00 DST>)

In [9]: iter.get_prev(datetime)
Out[9]: datetime.datetime(2018, 10, 26, 3, 0, tzinfo=<DstTzInfo 'Europe/Paris' CEST+2:00:00 DST>)

In [10]: iter.get_next(datetime)
Out[10]: datetime.datetime(2018, 10, 27, 3, 0, tzinfo=<DstTzInfo 'Europe/Paris' CEST+2:00:00 DST>)

In [11]: iter.get_next(datetime)
Out[11]: datetime.datetime(2018, 10, 28, 2, 0, tzinfo=<DstTzInfo 'Europe/Paris' CET+1:00:00 STD>)

In [12]: iter.get_next(datetime)
Out[12]: datetime.datetime(2018, 10, 28, 3, 0, tzinfo=<DstTzInfo 'Europe/Paris' CET+1:00:00 STD>)

In [13]: iter.get_next(datetime)
Out[13]: datetime.datetime(2018, 10, 29, 3, 0, tzinfo=<DstTzInfo 'Europe/Paris' CET+1:00:00 STD>)

In [14]: iter.get_next(datetime)
Out[14]: datetime.datetime(2018, 10, 30, 3, 0, tzinfo=<DstTzInfo 'Europe/Paris' CET+1:00:00 STD>)

In [15]: iter.get_prev(datetime)
Out[15]: datetime.datetime(2018, 10, 29, 3, 0, tzinfo=<DstTzInfo 'Europe/Paris' CET+1:00:00 STD>)

In [16]: iter.get_prev(datetime)
Out[16]: datetime.datetime(2018, 10, 28, 3, 0, tzinfo=<DstTzInfo 'Europe/Paris' CET+1:00:00 STD>)

In [17]: iter.get_prev(datetime)
Out[17]: datetime.datetime(2018, 10, 27, 4, 0, tzinfo=<DstTzInfo 'Europe/Paris' CEST+2:00:00 DST>)

In [18]: iter.get_prev(datetime)
Out[18]: datetime.datetime(2018, 10, 27, 3, 0, tzinfo=<DstTzInfo 'Europe/Paris' CEST+2:00:00 DST>)

(Ipython output.)

Notice how the hour changes - sometimes to 4am, sometimes to 2am, despite always being in the Europe/Paris timezone.

On croniter 0.3.25

In [20]: import pkg_resources

In [21]: pkg_resources.require('croniter>=0.3.25')
Out[21]:
[croniter 0.3.25 (/Users/ash/.virtualenvs/airflow/lib/python3.5/site-packages),
 python-dateutil 2.6.0 (/Users/ash/.virtualenvs/airflow/lib/python3.5/site-packages/python_dateutil-2.6.0-py3.5.egg),
 six 1.11.0 (/Users/ash/.virtualenvs/airflow/lib/python3.5/site-packages)]

The 10-28T02 from get_next only shows up if you've gone backwards - if you start on 26th October at 3am and just call get_next it doesn't appear.

st31ny commented 4 years ago

I noticed another strange thing: As I reported in aiocron the behaviour changes depending on how close to dst change you start:

from croniter import croniter
from datetime import datetime

from tzlocal import get_localzone

now = datetime.now(get_localzone())
it = croniter('1 2 * * *', now)
print("next", it.get_next(datetime).isoformat())
print("prev", it.get_prev(datetime).isoformat())
print("prev", it.get_prev(datetime).isoformat())
print("next", it.get_next(datetime).isoformat())
print("next", it.get_next(datetime).isoformat())
$ faketime '2020-03-29 1:59:55' ./croniter-test.py
next 2020-03-30T01:01:00+02:00
prev 2020-03-29T01:01:00+01:00
prev 2020-03-28T02:01:00+01:00
next 2020-03-29T03:01:00+02:00
next 2020-03-30T02:01:00+02:00
$ faketime '2020-03-29 1:58:55' ./croniter-test.py
next 2020-03-29T03:01:00+02:00
prev 2020-03-29T01:01:00+01:00
prev 2020-03-28T02:01:00+01:00
next 2020-03-29T03:01:00+02:00
next 2020-03-30T01:01:00+02:00

In both cases I start well before the DST switch, still the result is much different.

lowell80 commented 4 years ago

Is this the same behavior/workaround as described here? https://medium.com/@kumar.saransh/python-cron-and-daylight-time-shifts-ef406ad45a41

The suggested solution on the blog post is:

def generate_next_schedule(base_time: datetime, tz: BaseTzInfo):
    naive_dt_time = datetime(
        base_time.year, base_time.month, base_time.day, base_time.hour, base_time.minute  # assumption is DST shift
        # happens on minute/hour
    )
    cron = croniter('0 2 * * *', naive_dt_time)
    delta = cron.get_next(datetime) - naive_dt_time
    return tz.normalize(base_time + delta)

base = central_tz.localize(datetime(2019, 3, 8, 1, 50))
for _ in range(10):
    base = generate_next_schedule(base, central_tz)
    print(f"Following schedule: {base.isoformat()}")

But my particular use case requires calling for dozens to thousands of iterations per cron schedule, and sometimes I'm calculating hundreds to thousands of schedules simultaneously. So I have some performance concerns about the above "solution." (Ironically I just tossed my own custom-built (very sad) CRON evaluator for this one just for DST support. I guess I should have read the git issues first, lol!)

Ultimately, this seems like a workaround for something that croniter should be getting right. Right? Am I missing something here? I've read a bunch of github issues on this topic, but haven't been able to get the full picture of how many actual DST issues exist and what's the next step for getting this fixed? Does anybody have any partial patches for me to try or start from? Or is this fundamentally not fixable?

kiorky commented 4 years ago

DST is a nightmare, you ll also have platform differences.

I do not have the time right now to do it (and don't know when). But, of course, you can make a failing test and the according patch right into a pull request.

If it looks good, i will be glad to merge it !

kiorky commented 4 years ago

0.3.32 is out !