From f45bddb709452fa0918f317968ec27c1c2b9f909 Mon Sep 17 00:00:00 2001 From: Hubert Lobit Date: Tue, 29 Oct 2024 00:02:36 +0100 Subject: [PATCH 1/8] Fix a daylight savings time issue in `CronTrigger` (1) remove the `timedelta` operations - which are not timezone aware (2) make sure the "fold" attribute remains when incrementing --- src/apscheduler/triggers/cron/__init__.py | 19 +++++--------- tests/triggers/test_cron.py | 30 +++++++++++++++++++++++ 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/src/apscheduler/triggers/cron/__init__.py b/src/apscheduler/triggers/cron/__init__.py index 6abef7b53..cfcc6bb97 100644 --- a/src/apscheduler/triggers/cron/__init__.py +++ b/src/apscheduler/triggers/cron/__init__.py @@ -1,7 +1,7 @@ from __future__ import annotations from collections.abc import Sequence -from datetime import datetime, timedelta, tzinfo +from datetime import datetime, tzinfo from typing import Any, ClassVar import attrs @@ -207,16 +207,17 @@ def _set_field_value( else: values[field.name] = new_value - return datetime(**values, tzinfo=self.timezone) + return datetime(**values, tzinfo=self.timezone).replace(fold=dateval.fold) def next(self) -> datetime | None: if self._last_fire_time: - start_time = self._last_fire_time + timedelta(microseconds=1) + next_time = datetime.fromtimestamp( + self._last_fire_time.timestamp() + 1, self.timezone + ) else: - start_time = self.start_time + next_time = self.start_time fieldnum = 0 - next_time = datetime_ceil(start_time).astimezone(self.timezone) while 0 <= fieldnum < len(self._fields): field = self._fields[fieldnum] curr_value = field.get_value(next_time) @@ -276,11 +277,3 @@ def __repr__(self) -> str: fields.append(f"timezone={timezone_repr(self.timezone)!r}") return f'CronTrigger({", ".join(fields)})' - - -def datetime_ceil(dateval: datetime) -> datetime: - """Round the given datetime object upwards.""" - if dateval.microsecond > 0: - return dateval + timedelta(seconds=1, microseconds=-dateval.microsecond) - - return dateval diff --git a/tests/triggers/test_cron.py b/tests/triggers/test_cron.py index 58b46291a..c5380032e 100644 --- a/tests/triggers/test_cron.py +++ b/tests/triggers/test_cron.py @@ -400,6 +400,36 @@ def test_dst_change( ) +@pytest.mark.parametrize( + "cron_expression, start_time, correct_next_dates", + [ + ('0 * * * *', datetime(2024, 10, 27, 2, 0, 0, 0), [ + (datetime(2024, 10, 27, 2, 0, 0, 0), 0), + (datetime(2024, 10, 27, 2, 0, 0, 0), 1), + (datetime(2024, 10, 27, 3, 0, 0, 0), 0), + ]), + ('1 * * * *', datetime(2024, 10, 27, 2, 1, 0, 0), [ + (datetime(2024, 10, 27, 2, 1, 0, 0), 0), + (datetime(2024, 10, 27, 2, 1, 0, 0), 1), + (datetime(2024, 10, 27, 3, 1, 0, 0), 0), + ]), + ], + ids=["dst_change_0", "dst_change_1"], +) +def test_dst_change2( + cron_expression, + start_time, + correct_next_dates, + timezone, +): + trigger = CronTrigger.from_crontab(cron_expression, timezone=timezone) + trigger.start_time = start_time.astimezone(timezone) + for (correct_next_date, fold) in correct_next_dates: + next_date = trigger.next() + assert next_date == correct_next_date.astimezone(timezone) + assert next_date.fold == fold + + def test_zero_value(timezone): start_time = datetime(2020, 1, 1, tzinfo=timezone) trigger = CronTrigger( From ee7e771e81c7307bc4171dfdd772afea4c51819b Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 28 Oct 2024 23:26:25 +0000 Subject: [PATCH 2/8] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/triggers/test_cron.py | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/tests/triggers/test_cron.py b/tests/triggers/test_cron.py index c5380032e..654d40e34 100644 --- a/tests/triggers/test_cron.py +++ b/tests/triggers/test_cron.py @@ -403,16 +403,24 @@ def test_dst_change( @pytest.mark.parametrize( "cron_expression, start_time, correct_next_dates", [ - ('0 * * * *', datetime(2024, 10, 27, 2, 0, 0, 0), [ - (datetime(2024, 10, 27, 2, 0, 0, 0), 0), - (datetime(2024, 10, 27, 2, 0, 0, 0), 1), - (datetime(2024, 10, 27, 3, 0, 0, 0), 0), - ]), - ('1 * * * *', datetime(2024, 10, 27, 2, 1, 0, 0), [ - (datetime(2024, 10, 27, 2, 1, 0, 0), 0), - (datetime(2024, 10, 27, 2, 1, 0, 0), 1), - (datetime(2024, 10, 27, 3, 1, 0, 0), 0), - ]), + ( + "0 * * * *", + datetime(2024, 10, 27, 2, 0, 0, 0), + [ + (datetime(2024, 10, 27, 2, 0, 0, 0), 0), + (datetime(2024, 10, 27, 2, 0, 0, 0), 1), + (datetime(2024, 10, 27, 3, 0, 0, 0), 0), + ], + ), + ( + "1 * * * *", + datetime(2024, 10, 27, 2, 1, 0, 0), + [ + (datetime(2024, 10, 27, 2, 1, 0, 0), 0), + (datetime(2024, 10, 27, 2, 1, 0, 0), 1), + (datetime(2024, 10, 27, 3, 1, 0, 0), 0), + ], + ), ], ids=["dst_change_0", "dst_change_1"], ) @@ -424,7 +432,7 @@ def test_dst_change2( ): trigger = CronTrigger.from_crontab(cron_expression, timezone=timezone) trigger.start_time = start_time.astimezone(timezone) - for (correct_next_date, fold) in correct_next_dates: + for correct_next_date, fold in correct_next_dates: next_date = trigger.next() assert next_date == correct_next_date.astimezone(timezone) assert next_date.fold == fold From 4ba001c06cc5c254d06ad8d585632ad894e0ee3b Mon Sep 17 00:00:00 2001 From: Hubert Lobit Date: Tue, 29 Oct 2024 01:01:56 +0100 Subject: [PATCH 3/8] Fix test sensitive to local timezone --- tests/triggers/test_cron.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/triggers/test_cron.py b/tests/triggers/test_cron.py index 654d40e34..08700ebff 100644 --- a/tests/triggers/test_cron.py +++ b/tests/triggers/test_cron.py @@ -431,10 +431,10 @@ def test_dst_change2( timezone, ): trigger = CronTrigger.from_crontab(cron_expression, timezone=timezone) - trigger.start_time = start_time.astimezone(timezone) + trigger.start_time = start_time.replace(tzinfo=timezone) for correct_next_date, fold in correct_next_dates: next_date = trigger.next() - assert next_date == correct_next_date.astimezone(timezone) + assert next_date == correct_next_date.replace(tzinfo=timezone) assert next_date.fold == fold From 06e2f0bb8156e034ea560db45a91333fe729e9e8 Mon Sep 17 00:00:00 2001 From: Hubert Lobit Date: Tue, 29 Oct 2024 01:11:05 +0100 Subject: [PATCH 4/8] Add an entry to version history --- docs/versionhistory.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/versionhistory.rst b/docs/versionhistory.rst index 9614762c3..d88f88d0b 100644 --- a/docs/versionhistory.rst +++ b/docs/versionhistory.rst @@ -62,6 +62,8 @@ APScheduler, see the :doc:`migration section `. acquire the same schedules at once - Changed ``SQLAlchemyDataStore`` to automatically create the explicitly specified schema if it's missing (PR by @zhu0629) +- Fixed an issue with ``CronTrigger`` infinitely looping to get next date when DST ends + (`#980 `_; PR by @hlobit) **4.0.0a5** From f903cc54b86cb99a8d3c5dd5e8b44e11cdb4a9dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Gr=C3=B6nholm?= Date: Sun, 12 Jan 2025 14:39:40 +0200 Subject: [PATCH 5/8] Apply suggestions from code review --- src/apscheduler/triggers/cron/__init__.py | 2 +- tests/triggers/test_cron.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/apscheduler/triggers/cron/__init__.py b/src/apscheduler/triggers/cron/__init__.py index 4160a40b8..e34bd6b75 100644 --- a/src/apscheduler/triggers/cron/__init__.py +++ b/src/apscheduler/triggers/cron/__init__.py @@ -207,7 +207,7 @@ def _set_field_value( else: values[field.name] = new_value - return datetime(**values, tzinfo=self.timezone).replace(fold=dateval.fold) + return datetime(**values, tzinfo=self.timezone, fold=dateval.fold) def next(self) -> datetime | None: if self._last_fire_time: diff --git a/tests/triggers/test_cron.py b/tests/triggers/test_cron.py index 08700ebff..da1798e9e 100644 --- a/tests/triggers/test_cron.py +++ b/tests/triggers/test_cron.py @@ -434,8 +434,8 @@ def test_dst_change2( trigger.start_time = start_time.replace(tzinfo=timezone) for correct_next_date, fold in correct_next_dates: next_date = trigger.next() - assert next_date == correct_next_date.replace(tzinfo=timezone) - assert next_date.fold == fold + assert next_date == correct_next_date.replace(tzinfo=timezone, fold=fold) + assert str(next_date) == str(correct_next_date) def test_zero_value(timezone): From de81df1deef5e3ee002463d0118b3d3a3e6af53f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Gr=C3=B6nholm?= Date: Sun, 19 Jan 2025 14:36:26 +0200 Subject: [PATCH 6/8] Update test_cron.py --- tests/triggers/test_cron.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/triggers/test_cron.py b/tests/triggers/test_cron.py index da1798e9e..e29472c76 100644 --- a/tests/triggers/test_cron.py +++ b/tests/triggers/test_cron.py @@ -433,8 +433,9 @@ def test_dst_change2( trigger = CronTrigger.from_crontab(cron_expression, timezone=timezone) trigger.start_time = start_time.replace(tzinfo=timezone) for correct_next_date, fold in correct_next_dates: + correct_next_date = correct_next_date.replace(tzinfo=timezone, fold=fold) next_date = trigger.next() - assert next_date == correct_next_date.replace(tzinfo=timezone, fold=fold) + assert next_date == correct_next_date assert str(next_date) == str(correct_next_date) From 25bc95669505e2226183faacb6ed3d270fa87a7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Gr=C3=B6nholm?= Date: Sun, 19 Jan 2025 14:41:43 +0200 Subject: [PATCH 7/8] Update test_cron.py --- tests/triggers/test_cron.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/triggers/test_cron.py b/tests/triggers/test_cron.py index e29472c76..8061ce3b8 100644 --- a/tests/triggers/test_cron.py +++ b/tests/triggers/test_cron.py @@ -401,10 +401,10 @@ def test_dst_change( @pytest.mark.parametrize( - "cron_expression, start_time, correct_next_dates", + "hour, start_time, correct_next_dates", [ ( - "0 * * * *", + 0, datetime(2024, 10, 27, 2, 0, 0, 0), [ (datetime(2024, 10, 27, 2, 0, 0, 0), 0), @@ -413,7 +413,7 @@ def test_dst_change( ], ), ( - "1 * * * *", + 1, datetime(2024, 10, 27, 2, 1, 0, 0), [ (datetime(2024, 10, 27, 2, 1, 0, 0), 0), @@ -425,12 +425,12 @@ def test_dst_change( ids=["dst_change_0", "dst_change_1"], ) def test_dst_change2( - cron_expression, + hour, start_time, correct_next_dates, timezone, ): - trigger = CronTrigger.from_crontab(cron_expression, timezone=timezone) + trigger = CronTrigger(hour=hour, timezone=timezone) trigger.start_time = start_time.replace(tzinfo=timezone) for correct_next_date, fold in correct_next_dates: correct_next_date = correct_next_date.replace(tzinfo=timezone, fold=fold) From 31365e648f5cce25e589235f2244380c1307f800 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Gr=C3=B6nholm?= Date: Sun, 19 Jan 2025 14:50:55 +0200 Subject: [PATCH 8/8] Update test_cron.py --- tests/triggers/test_cron.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/triggers/test_cron.py b/tests/triggers/test_cron.py index 8061ce3b8..c0a077d0b 100644 --- a/tests/triggers/test_cron.py +++ b/tests/triggers/test_cron.py @@ -401,7 +401,7 @@ def test_dst_change( @pytest.mark.parametrize( - "hour, start_time, correct_next_dates", + "minute, start_time, correct_next_dates", [ ( 0, @@ -425,12 +425,12 @@ def test_dst_change( ids=["dst_change_0", "dst_change_1"], ) def test_dst_change2( - hour, + minute, start_time, correct_next_dates, timezone, ): - trigger = CronTrigger(hour=hour, timezone=timezone) + trigger = CronTrigger(minute=minute, timezone=timezone) trigger.start_time = start_time.replace(tzinfo=timezone) for correct_next_date, fold in correct_next_dates: correct_next_date = correct_next_date.replace(tzinfo=timezone, fold=fold)