Fix case where last_run_at=None and CELERY_TIMEZONE!=TIME_ZONE (#294)

* Fix case where last_run_at=None, USE_TZ=False, DJANGO_CELERY_BEAT_TZ_AWARE=False, and CELERY_TIMEZONE != settings.TIME_ZONE.

* Make _default_now always return a naive datetime in utc when returning a naive datetime. maybe_make_aware expects naive objects to be in utc time, while make_aware expects localtime. Here, we swap to matching maybe_make_aware's expectations and call it instead. It would be better if the two functions expected the same thing, but I'm not sure how disruptive that change would be. Also, I updated a few gaps in the relevant tests and made them more real. (resetting TZ like django would for one)

* Fix case where last_run_at=None, USE_TZ=False, DJANGO_CELERY_BEAT_TZ_AWARE=False, and CELERY_TIMEZONE != settings.TIME_ZONE.

* Make _default_now always return a naive datetime in utc when returning a naive datetime. maybe_make_aware expects naive objects to be in utc time, while make_aware expects localtime. Here, we swap to matching maybe_make_aware's expectations and call it instead. It would be better if the two functions expected the same thing, but I'm not sure how disruptive that change would be. Also, I updated a few gaps in the relevant tests and made them more real. (resetting TZ like django would for one)

* Fix flake8 errors.

* Fix flake issues in this file too. I only changed these two so hopefully that's it.

* Fix readme.md

I think we are hitting a bug in the markdown processing, where triple backticks are not working properly... I got it to work another way here though, so saving this to make the readme show up properly.

* Fix case where last_run_at=None, USE_TZ=False, DJANGO_CELERY_BEAT_TZ_AWARE=False, and CELERY_TIMEZONE != settings.TIME_ZONE.

* Make _default_now always return a naive datetime in utc when returning a naive datetime. maybe_make_aware expects naive objects to be in utc time, while make_aware expects localtime. Here, we swap to matching maybe_make_aware's expectations and call it instead. It would be better if the two functions expected the same thing, but I'm not sure how disruptive that change would be. Also, I updated a few gaps in the relevant tests and made them more real. (resetting TZ like django would for one)

* Fix flake8 errors.

* Fix flake issues in this file too. I only changed these two so hopefully that's it.

Author: marky1991

django_celery_beat/schedulers.py

@@ -26,7 +26,6 @@
     CrontabSchedule, IntervalSchedule,
     SolarSchedule, ClockedSchedule
 )
-from .utils import make_aware
 from .clockedschedule import clocked
 
 try:
@@ -134,7 +133,11 @@ def is_due(self):
             self.model.save()
             return schedules.schedstate(False, None)  # Don't recheck
 
-        return self.schedule.is_due(make_aware(self.last_run_at))
+        # CAUTION: make_aware assumes settings.TIME_ZONE for naive datetimes,
+        # while maybe_make_aware assumes utc for naive datetimes
+        tz = self.app.timezone
+        last_run_at_in_tz = maybe_make_aware(self.last_run_at).astimezone(tz)
+        return self.schedule.is_due(last_run_at_in_tz)
 
     def _default_now(self):
         # The PyTZ datetime must be localised for the Django-Celery-Beat
@@ -144,7 +147,9 @@ def _default_now(self):
             now = self.app.now()
             now = now.tzinfo.localize(now.replace(tzinfo=None))
         else:
-            now = datetime.datetime.now()
+            # this ends up getting passed to maybe_make_aware, which expects
+            # all naive datetime objects to be in utc time.
+            now = datetime.datetime.utcnow()
         return now
 
     def __next__(self):

t/unit/test_schedulers.py

@@ -1,6 +1,7 @@
 from __future__ import absolute_import, unicode_literals
 
 import math
+import os
 import time
 import pytest
 
@@ -151,13 +152,18 @@ def test_entry(self):
         USE_TZ=False,
         DJANGO_CELERY_BEAT_TZ_AWARE=False
     )
+    @pytest.mark.usefixtures('depends_on_current_app')
     @timezone.override('Europe/Berlin')
     @pytest.mark.celery(timezone='Europe/Berlin')
     def test_entry_is_due__no_use_tz(self):
+        old_tz = os.environ.get("TZ")
+        os.environ["TZ"] = "Europe/Berlin"
+        if hasattr(time, "tzset"):
+            time.tzset()
         assert self.app.timezone.zone == 'Europe/Berlin'
 
         # simulate last_run_at from DB - not TZ aware but localtime
-        right_now = timezone.now()
+        right_now = datetime.utcnow()
 
         m = self.create_model_crontab(
             crontab(minute='*/10'),
@@ -167,6 +173,47 @@ def test_entry_is_due__no_use_tz(self):
 
         assert e.is_due().is_due is False
         assert e.is_due().next <= 600  # 10 minutes; see above
+        if old_tz is not None:
+            os.environ["TZ"] = old_tz
+        else:
+            del os.environ["TZ"]
+        if hasattr(time, "tzset"):
+            time.tzset()
+
+    @override_settings(
+        USE_TZ=False,
+        DJANGO_CELERY_BEAT_TZ_AWARE=False,
+        TIME_ZONE="Europe/Berlin",
+        CELERY_TIMEZONE="America/New_York"
+    )
+    @pytest.mark.usefixtures('depends_on_current_app')
+    @timezone.override('Europe/Berlin')
+    @pytest.mark.celery(timezone='America/New_York')
+    def test_entry_is_due__celery_timezone_doesnt_match_time_zone(self):
+        old_tz = os.environ.get("TZ")
+        os.environ["TZ"] = "Europe/Berlin"
+        if hasattr(time, "tzset"):
+            time.tzset()
+        assert self.app.timezone.zone == 'America/New_York'
+
+        # simulate last_run_at all none, doing the same thing that
+        # _default_now() would do
+        right_now = datetime.utcnow()
+
+        m = self.create_model_crontab(
+            crontab(minute='*/10'),
+            last_run_at=right_now,
+        )
+        e = self.Entry(m, app=self.app)
+
+        assert e.is_due().is_due is False
+        assert e.is_due().next <= 600  # 10 minutes; see above
+        if old_tz is not None:
+            os.environ["TZ"] = old_tz
+        else:
+            del os.environ["TZ"]
+        if hasattr(time, "tzset"):
+            time.tzset()
 
     def test_task_with_start_time(self):
         interval = 10