From f45bddb709452fa0918f317968ec27c1c2b9f909 Mon Sep 17 00:00:00 2001 From: Hubert Lobit Date: Tue, 29 Oct 2024 00:02:36 +0100 Subject: [PATCH 1/5] 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 6abef7b5..cfcc6bb9 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 58b46291..c5380032 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/5] [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 c5380032..654d40e3 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/5] 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 654d40e3..08700ebf 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/5] 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 9614762c..d88f88d0 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/5] 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 4160a40b..e34bd6b7 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 08700ebf..da1798e9 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):