Timezone handling fixes (#456)

* Simplified timezone handling code.

* Use integer division.

* Fix eta and expires timezone handling in Camera.

maybe_iso8601() returns an aware datetime if it can. Correctly make it naive if `USE_TZ = False`.

* Fix timestamp timezone handling in Camera.

Removed reliance on CELERY_ENABLE_UTC, because when it is not defined it is assumed False, unlike in celery.
Also some code cleanup.

* Show datetimes in local timezone in admin.

* Add Changelog entry.

Author: vytisb

Changelog

@@ -37,6 +37,10 @@
 
     Fix contributed by Rockallite Wulf.
 
+- Fix timezone handling in Camera and admin.
+
+    Fix contributed by Vytis Banaitis.
+
 .. _version-3.1.17:
 
 3.1.17

djcelery/admin.py

@@ -25,7 +25,7 @@
     PeriodicTask, IntervalSchedule, CrontabSchedule,
 )
 from .humanize import naturaldate
-from .utils import is_database_scheduler
+from .utils import is_database_scheduler, make_aware
 
 try:
     from django.utils.encoding import force_text
@@ -68,13 +68,15 @@ def node_state(node):
 def eta(task):
     if not task.eta:
         return '<span style="color: gray;">none</span>'
-    return escape(task.eta)
+    return escape(make_aware(task.eta))
 
 
 @display_field(_('when'), 'tstamp')
 def tstamp(task):
+    # convert to local timezone
+    value = make_aware(task.tstamp)
     return '<div title="{0}">{1}</div>'.format(
-        escape(str(task.tstamp)), escape(naturaldate(task.tstamp)),
+        escape(str(value)), escape(naturaldate(value)),
     )
 
 

djcelery/humanize.py

@@ -48,8 +48,8 @@ def naturaldate(date, include_seconds=False):
     delta_midnight = today - date
 
     days = delta.days
-    hours = int(round(delta.seconds / 3600, 0))
-    minutes = delta.seconds / 60
+    hours = delta.seconds // 3600
+    minutes = delta.seconds // 60
     seconds = delta.seconds
 
     if days < 0:
@@ -80,6 +80,6 @@ def naturaldate(date, include_seconds=False):
     count = 0
     for chunk, pluralizefun in OLDER_CHUNKS:
         if days >= chunk:
-            count = round((delta_midnight.days + 1) / chunk, 0)
+            count = int(round((delta_midnight.days + 1) / chunk, 0))
             fmt = pluralizefun(count)
             return fmt.format(num=count)

djcelery/loaders.py

@@ -11,7 +11,6 @@
 from celery.datastructures import DictAttribute
 from celery.loaders.base import BaseLoader
 
-import django
 from django import db
 from django.conf import settings
 from django.core import cache
@@ -20,7 +19,6 @@
 from .utils import DATABASE_ERRORS, now
 
 _RACE_PROTECTION = False
-NO_TZ = django.VERSION < (1, 4)
 
 
 def _maybe_close_fd(fh):
@@ -62,10 +60,6 @@ def read_configuration(self):
                    getattr(settings, 'CELERY_BACKEND', None))
         if not backend:
             settings.CELERY_RESULT_BACKEND = 'database'
-        if NO_TZ:
-            if getattr(settings, 'CELERY_ENABLE_UTC', None):
-                warn('CELERY_ENABLE_UTC requires Django 1.4+')
-            settings.CELERY_ENABLE_UTC = False
         return DictAttribute(settings)
 
     def _close_database(self):

djcelery/models.py

@@ -15,7 +15,7 @@
 
 from . import managers
 from .picklefield import PickledObjectField
-from .utils import fromtimestamp, now
+from .utils import now
 from .compat import python_2_unicode_compatible
 
 ALL_STATES = sorted(states.ALL_STATES)
@@ -364,17 +364,6 @@ class Meta:
         get_latest_by = 'tstamp'
         ordering = ['-tstamp']
 
-    def save(self, *args, **kwargs):
-        if self.eta is not None:
-            self.eta = fromtimestamp(float('%d.%s' % (
-                mktime(self.eta.timetuple()), self.eta.microsecond,
-            )))
-        if self.expires is not None:
-            self.expires = fromtimestamp(float('%d.%s' % (
-                mktime(self.expires.timetuple()), self.expires.microsecond,
-            )))
-        super(TaskState, self).save(*args, **kwargs)
-
     def __str__(self):
         name = self.name or 'UNKNOWN'
         s = '{0.state:<10} {0.task_id:<36} {1}'.format(self, name)

djcelery/snapshot.py

@@ -1,7 +1,7 @@
 from __future__ import absolute_import, unicode_literals
 
 from collections import defaultdict
-from datetime import datetime, timedelta
+from datetime import timedelta
 
 from django.conf import settings
 
@@ -13,7 +13,7 @@
 from celery.utils.timeutils import maybe_iso8601
 
 from .models import WorkerState, TaskState
-from .utils import fromtimestamp, maybe_make_aware
+from .utils import fromtimestamp, correct_awareness
 
 WORKER_UPDATE_FREQ = 60  # limit worker timestamp write freq.
 SUCCESS_STATES = frozenset([states.SUCCESS])
@@ -31,11 +31,6 @@
 debug = logger.debug
 
 
-def aware_tstamp(secs):
-    """Event timestamps uses the local timezone."""
-    return maybe_make_aware(fromtimestamp(secs))
-
-
 class Camera(Polaroid):
     TaskState = TaskState
     WorkerState = WorkerState
@@ -57,10 +52,7 @@ def get_heartbeat(self, worker):
             heartbeat = worker.heartbeats[-1]
         except IndexError:
             return
-        # Check for timezone settings
-        if getattr(settings, "USE_TZ", False):
-            return aware_tstamp(heartbeat)
-        return datetime.fromtimestamp(heartbeat)
+        return fromtimestamp(heartbeat)
 
     def handle_worker(self, hostname_worker):
         (hostname, worker) = hostname_worker
@@ -86,10 +78,10 @@ def handle_task(self, uuid_task, worker=None):
             'name': task.name,
             'args': task.args,
             'kwargs': task.kwargs,
-            'eta': maybe_make_aware(maybe_iso8601(task.eta)),
-            'expires': maybe_make_aware(maybe_iso8601(task.expires)),
+            'eta': correct_awareness(maybe_iso8601(task.eta)),
+            'expires': correct_awareness(maybe_iso8601(task.expires)),
             'state': task.state,
-            'tstamp': aware_tstamp(task.timestamp),
+            'tstamp': fromtimestamp(task.timestamp),
             'result': task.result or task.exception,
             'traceback': task.traceback,
             'runtime': task.runtime,
@@ -121,9 +113,6 @@ def update_task(self, state, **kwargs):
 
         for k, v in defaults.items():
             setattr(obj, k, v)
-        for datefield in ('eta', 'expires', 'tstamp'):
-            # Brute force trying to fix #183
-            setattr(obj, datefield, maybe_make_aware(getattr(obj, datefield)))
         obj.save()
 
         return obj

djcelery/tests/test_snapshot.py

@@ -9,6 +9,9 @@
 from celery.events.state import State, Worker, Task
 from celery.utils import gen_unique_id
 
+from django.test.utils import override_settings
+from django.utils import timezone
+
 from djcelery import celery
 from djcelery import snapshot
 from djcelery import models
@@ -74,7 +77,7 @@ def test_handle_worker(self):
 
     def test_handle_task_received(self):
         worker = Worker(hostname='fuzzie')
-        worker.event('oneline', time(), time(), {})
+        worker.event('online', time(), time(), {})
         self.cam.handle_worker((worker.hostname, worker))
 
         task = create_task(worker)
@@ -115,7 +118,60 @@ def test_handle_task(self):
         mt = self.cam.handle_task((task3.uuid, task3))
         self.assertIsNone(mt)
 
+    def test_handle_task_timezone(self):
+        worker = Worker(hostname='fuzzie')
+        worker.event('online', time(), time(), {})
+        self.cam.handle_worker((worker.hostname, worker))
+
+        tstamp = 1464793200.0  # 2016-06-01T15:00:00Z
+
+        with override_settings(USE_TZ=True, TIME_ZONE='Europe/Helsinki'):
+            task = create_task(worker,
+                               eta='2016-06-01T15:16:17.654321+00:00',
+                               expires='2016-07-01T15:16:17.765432+03:00')
+            task.event('received', tstamp, tstamp, {})
+            mt = self.cam.handle_task((task.uuid, task))
+            self.assertEqual(
+                mt.tstamp,
+                datetime(2016, 6, 1, 15, 0, 0, tzinfo=timezone.utc),
+            )
+            self.assertEqual(
+                mt.eta,
+                datetime(2016, 6, 1, 15, 16, 17, 654321, tzinfo=timezone.utc),
+            )
+            self.assertEqual(
+                mt.expires,
+                datetime(2016, 7, 1, 12, 16, 17, 765432, tzinfo=timezone.utc),
+            )
+
+            task = create_task(worker, eta='2016-06-04T15:16:17.654321')
+            task.event('received', tstamp, tstamp, {})
+            mt = self.cam.handle_task((task.uuid, task))
+            self.assertEqual(
+                mt.eta,
+                datetime(2016, 6, 4, 15, 16, 17, 654321, tzinfo=timezone.utc),
+            )
+
+        with override_settings(USE_TZ=False, TIME_ZONE='Europe/Helsinki'):
+            task = create_task(worker,
+                               eta='2016-06-01T15:16:17.654321+00:00',
+                               expires='2016-07-01T15:16:17.765432+03:00')
+            task.event('received', tstamp, tstamp, {})
+            mt = self.cam.handle_task((task.uuid, task))
+            self.assertEqual(mt.tstamp, datetime(2016, 6, 1, 18, 0, 0))
+            self.assertEqual(mt.eta, datetime(2016, 6, 1, 18, 16, 17, 654321))
+            self.assertEqual(mt.expires,
+                             datetime(2016, 7, 1, 15, 16, 17, 765432))
+
+            task = create_task(worker, eta='2016-06-04T15:16:17.654321')
+            task.event('received', tstamp, tstamp, {})
+            mt = self.cam.handle_task((task.uuid, task))
+            self.assertEqual(mt.eta, datetime(2016, 6, 4, 15, 16, 17, 654321))
+
     def assertExpires(self, dec, expired, tasks=10):
+        # Cleanup leftovers from previous tests
+        self.cam.on_cleanup()
+
         worker = Worker(hostname='fuzzie')
         worker.event('online', time(), time(), {})
         for total in range(tasks):

djcelery/utils.py

@@ -5,6 +5,7 @@
 from datetime import datetime
 
 from django.conf import settings
+from django.utils import timezone
 
 # Database-related exceptions.
 from django.db import DatabaseError
@@ -44,51 +45,35 @@
                    _oracle_database_errors)
 
 
-try:
-    from django.utils import timezone
-    is_aware = timezone.is_aware
-
-    # see Issue #222
-    now_localtime = getattr(timezone, 'template_localtime', timezone.localtime)
-
-    def make_aware(value):
-        if getattr(settings, 'USE_TZ', False):
-            # naive datetimes are assumed to be in UTC.
-            if timezone.is_naive(value):
-                value = timezone.make_aware(value, timezone.utc)
-            # then convert to the Django configured timezone.
-            default_tz = timezone.get_default_timezone()
-            value = timezone.localtime(value, default_tz)
-        return value
-
-    def make_naive(value):
-        if getattr(settings, 'USE_TZ', False):
-            default_tz = timezone.get_default_timezone()
-            value = timezone.make_naive(value, default_tz)
-        return value
+def make_aware(value):
+    if settings.USE_TZ:
+        # naive datetimes are assumed to be in UTC.
+        if timezone.is_naive(value):
+            value = timezone.make_aware(value, timezone.utc)
+        # then convert to the Django configured timezone.
+        default_tz = timezone.get_default_timezone()
+        value = timezone.localtime(value, default_tz)
+    return value
 
-    def now():
-        if getattr(settings, 'USE_TZ', False):
-            return now_localtime(timezone.now())
-        else:
-            return timezone.now()
 
-except ImportError:
-    now = datetime.now
+def make_naive(value):
+    if settings.USE_TZ:
+        default_tz = timezone.get_default_timezone()
+        value = timezone.make_naive(value, default_tz)
+    return value
 
-    def _pass(x):
-        return x
-    make_aware = make_naive = _pass
 
-    def is_aware(x):
-        return False
+def now():
+    return make_aware(timezone.now())
 
 
-def maybe_make_aware(value):
-    if isinstance(value, datetime) and is_aware(value):
-        return value
-    if value:
-        return make_aware(value)
+def correct_awareness(value):
+    if isinstance(value, datetime):
+        if settings.USE_TZ:
+            return make_aware(value)
+        elif timezone.is_aware(value):
+            default_tz = timezone.get_default_timezone()
+            return timezone.make_naive(value, default_tz)
     return value
 
 
@@ -101,7 +86,7 @@ def is_database_scheduler(scheduler):
 
 
 def fromtimestamp(value):
-    if getattr(settings, 'CELERY_ENABLE_UTC', False):
-        return datetime.utcfromtimestamp(value)
+    if settings.USE_TZ:
+        return make_aware(datetime.utcfromtimestamp(value))
     else:
         return datetime.fromtimestamp(value)