From 4d454e4001d7600ca6e8a69282dd91da1f148ed9 Mon Sep 17 00:00:00 2001 From: Stan Triepels <1939656+GDay@users.noreply.github.com> Date: Wed, 21 Dec 2022 01:44:34 +0100 Subject: [PATCH] Fix: Daylight saving time issue with scheduler (#47) --- django_q/cluster.py | 38 ++------------------ django_q/conf.py | 5 +++ django_q/models.py | 59 ++++++++++++++++++++++++++++++- django_q/tests/settings.py | 2 +- django_q/tests/test_scheduler.py | 60 +++++++++++++++++++++++++++++++- django_q/utils.py | 16 +++++++++ docs/configure.rst | 7 ++++ 7 files changed, 149 insertions(+), 38 deletions(-) diff --git a/django_q/cluster.py b/django_q/cluster.py index f34c81d..ba3408a 100644 --- a/django_q/cluster.py +++ b/django_q/cluster.py @@ -6,6 +6,7 @@ import socket import traceback import uuid from datetime import datetime, timedelta +from pytz import timezone as pytz_timezone from multiprocessing import Event, Process, Value, current_process from time import sleep @@ -43,7 +44,7 @@ from django_q.signals import post_execute, pre_execute from django_q.signing import BadSignature, SignedPackage from django_q.status import Stat, Status -from .utils import add_months, add_years, get_func_repr +from .utils import get_func_repr, localtime class Cluster: @@ -700,33 +701,7 @@ def scheduler(broker: Broker = None): if s.schedule_type != s.ONCE: next_run = s.next_run while True: - if s.schedule_type == s.MINUTES: - next_run = next_run + timedelta(minutes=(s.minutes or 1)) - elif s.schedule_type == s.HOURLY: - next_run = next_run + timedelta(hours=1) - elif s.schedule_type == s.DAILY: - next_run = next_run + timedelta(days=1) - elif s.schedule_type == s.WEEKLY: - next_run = next_run + timedelta(weeks=1) - elif s.schedule_type == s.BIWEEKLY: - next_run = next_run + timedelta(weeks=2) - elif s.schedule_type == s.MONTHLY: - next_run = add_months(next_run, 1) - elif s.schedule_type == s.BIMONTHLY: - next_run = add_months(next_run, 2) - elif s.schedule_type == s.QUARTERLY: - next_run = add_months(next_run, 3) - elif s.schedule_type == s.YEARLY: - next_run = add_years(next_run, 1) - elif s.schedule_type == s.CRON: - if not croniter: - raise ImportError( - _( - "Please install croniter to enable cron " - "expressions" - ) - ) - next_run = croniter(s.cron, localtime()).get_next(datetime) + next_run = s.calculate_next_run(next_run) if Conf.CATCH_UP or next_run > localtime(): break @@ -842,10 +817,3 @@ def rss_check(): elif psutil: return psutil.Process().memory_info().rss >= Conf.MAX_RSS * 1024 return False - - -def localtime() -> datetime: - """Override for timezone.localtime to deal with naive times and local times""" - if settings.USE_TZ: - return timezone.localtime() - return datetime.now() diff --git a/django_q/conf.py b/django_q/conf.py index 77ef602..3d32f0b 100644 --- a/django_q/conf.py +++ b/django_q/conf.py @@ -211,6 +211,11 @@ class Conf: # to manage workarounds during testing TESTING = conf.get("testing", False) + # Timezone for next_run, overrules Django timezone + TIME_ZONE = None + if settings.USE_TZ: + TIME_ZONE = conf.get("time_zone", settings.TIME_ZONE) + # logger logger = logging.getLogger("django-q") diff --git a/django_q/models.py b/django_q/models.py index eed8901..1e5048a 100644 --- a/django_q/models.py +++ b/django_q/models.py @@ -1,3 +1,5 @@ +from datetime import datetime, timedelta + # Django from django import get_version from django.core.exceptions import ValidationError @@ -5,6 +7,7 @@ from django.db import models from django.template.defaultfilters import truncatechars from django.urls import reverse from django.utils import timezone +from django.utils.timezone import is_aware from django.utils.html import format_html from django.utils.translation import gettext_lazy as _ @@ -13,8 +16,9 @@ from picklefield import PickledObjectField from picklefield.fields import dbsafe_decode # Local -from django_q.conf import croniter +from django_q.conf import croniter, Conf from django_q.signing import SignedPackage +from django_q.utils import localtime, add_months, add_years from .utils import get_func_repr @@ -208,6 +212,59 @@ class Schedule(models.Model): task = models.CharField(max_length=100, null=True, editable=False) cluster = models.CharField(max_length=100, default=None, null=True, blank=True) + def calculate_next_run(self, next_run=None): + # next run is always in UTC + next_run = next_run or self.next_run + + if self.schedule_type == self.CRON: + if not croniter: + raise ImportError( + _("Please install croniter to enable cron expressions") + ) + return croniter(self.cron, localtime()).get_next(datetime) + + if self.schedule_type == self.MINUTES: + add = timedelta(minutes=(self.minutes or 1)) + elif self.schedule_type == self.HOURLY: + add = timedelta(hours=1) + elif self.schedule_type == self.DAILY: + add = timedelta(days=1) + elif self.schedule_type == self.WEEKLY: + add = timedelta(weeks=1) + elif self.schedule_type == self.BIWEEKLY: + add = timedelta(weeks=2) + elif self.schedule_type == self.MONTHLY: + add = timedelta(days=(add_months(next_run, 1) - next_run).days) + elif self.schedule_type == self.BIMONTHLY: + add = timedelta(days=(add_months(next_run, 2) - next_run).days) + elif self.schedule_type == self.QUARTERLY: + add = timedelta(days=(add_months(next_run, 3) - next_run).days) + elif self.schedule_type == self.YEARLY: + add = timedelta(days=(add_years(next_run, 1) - next_run).days) + + # add normal timedelta, we will correct this later based on timezone + next_run += add + + # DST differencers don't matter with minutes, hourly or yearly, so skip those + if self.schedule_type not in [self.MINUTES, self.HOURLY, self.YEARLY]: + # Get localtimes and then remove the tzinfo, so we can get the actual difference + current_next_run = localtime(next_run - add).replace(tzinfo=None) + new_next_run = localtime(next_run).replace(tzinfo=None) + + # get the difference between them, this should be (-)1 or (-)0.5 hour + # based on DST active or not + extra_diff = (new_next_run - current_next_run) - add + + # if we have one positive hour difference, then subtract it, so we are even + # and vice versa. In most cases, this will be 0, as there won't be a + # timezone diff + if extra_diff > timedelta(hours=0): + next_run -= extra_diff + else: + next_run += extra_diff + + return next_run + def success(self): if self.task and Task.objects.filter(id=self.task): return Task.objects.get(id=self.task).success diff --git a/django_q/tests/settings.py b/django_q/tests/settings.py index b651adb..b624644 100644 --- a/django_q/tests/settings.py +++ b/django_q/tests/settings.py @@ -75,7 +75,7 @@ DATABASES = { LANGUAGE_CODE = "en-us" -TIME_ZONE = "UTC" +TIME_ZONE = "Europe/Amsterdam" USE_I18N = True diff --git a/django_q/tests/test_scheduler.py b/django_q/tests/test_scheduler.py index 82ecb22..f53e7b4 100644 --- a/django_q/tests/test_scheduler.py +++ b/django_q/tests/test_scheduler.py @@ -1,5 +1,6 @@ import os -from datetime import timedelta +import pytz +from datetime import datetime, timedelta from multiprocessing import Event, Value from unittest import mock @@ -83,6 +84,63 @@ MULTIPLE_APPS_DATABASES = { } +@pytest.mark.django_db +def test_scheduler_daylight_saving_time_daily(broker, monkeypatch): + # Set up a startdate in the Amsterdam timezone (without dst 1 hour ahead). The + # 28th of March 2021 is the day when sunlight saving starts (at 2 am) + + monkeypatch.setattr(Conf, "TIME_ZONE", "Europe/Amsterdam") + tz = pytz.timezone('Europe/Amsterdam') + broker.list_key = "scheduler_test:q" + # Let's start a schedule at 1 am on the 27th of March. This is in AMS timezone. + # So, 2021-03-27 00:00:00 when saved (due to TZ being Amsterdam and saved in UTC) + start_date = datetime(2021, 3, 27, 1, 0, 0) + + # Create schedule with the next run date on the start date. It will move one day + # forward when we run the scheduler + schedule = create_schedule( + "math.copysign", + 1, + -1, + name="test math", + schedule_type=Schedule.DAILY, + next_run=start_date, + ) + + # Run scheduler so we get the next run date + scheduler(broker=broker) + schedule.refresh_from_db() + + # It's now the day after exactly at midnight UTC + next_run = schedule.next_run + assert str(next_run) == "2021-03-28 00:00:00+00:00" + + # In the Amsterdam timezone, it's 1 hour over midnight (+01) + next_run = next_run.astimezone(tz) + assert str(next_run) == "2021-03-28 01:00:00+01:00" + + # Run scheduler so we get the next run date + scheduler(broker=broker) + schedule.refresh_from_db() + + next_run = schedule.next_run + + assert str(next_run) == "2021-03-28 23:00:00+00:00" + next_run = next_run.astimezone(tz) + # In the Amsterdam timezone, it's 1 hour over midnight (+02) + assert str(next_run) == "2021-03-29 01:00:00+02:00" + + # Run scheduler so we get the next run date + scheduler(broker=broker) + schedule.refresh_from_db() + + next_run = schedule.next_run + + assert str(next_run) == "2021-03-29 23:00:00+00:00" + next_run = next_run.astimezone(tz) + assert str(next_run) == "2021-03-30 01:00:00+02:00" + + @pytest.mark.django_db def test_scheduler(broker, monkeypatch): broker.list_key = "scheduler_test:q" diff --git a/django_q/utils.py b/django_q/utils.py index c0c6f7f..8c2d293 100644 --- a/django_q/utils.py +++ b/django_q/utils.py @@ -1,7 +1,13 @@ +from datetime import datetime +import pytz import calendar import inspect from datetime import date +from django.utils import timezone +from django.conf import settings + +from django_q.conf import Conf # credits: https://stackoverflow.com/a/4131114 # Made them aware of timezone @@ -39,3 +45,13 @@ def get_func_repr(func): ) else: return str(func) + + +def localtime(value=None) -> datetime: + """Override for timezone.localtime to deal with naive times and local times""" + if settings.USE_TZ: + return timezone.localtime(value=value, timezone=pytz.timezone(Conf.TIME_ZONE)) + if value is None: + return datetime.now() + else: + return value diff --git a/docs/configure.rst b/docs/configure.rst index 32d6f11..1b83fe6 100644 --- a/docs/configure.rst +++ b/docs/configure.rst @@ -70,6 +70,13 @@ Set this to something that makes sense for your project. Can be overridden for i See :ref:`retry` for details how to set values for timeout and retry. +.. _time_zone: + +time_zone +~~~~~~~ + +The timezone that is used for task scheduling. Use this if you are having issue with DST. The scheduler uses UTC to calculate the next date and will therefore ignore any DST changes. This will cause 1 hour or 0.5 hour changes in the schedule when time is moved one hour ahead or back. Defaults to `settings.TIME_ZONE` if `USE_TZ` is enabled. + .. _ack_failures: ack_failures