[PATCH v3] Avoid timezone confusion
vkabatov at redhat.com
vkabatov at redhat.com
Fri Feb 23 02:24:46 AEDT 2018
From: Veronika Kabatova <vkabatov at redhat.com>
Patchwork saves patches, comments etc with UTC timezone and reports
this time when opening the patch details. However, internally generated
processes such as events are reported with the instance's local time.
There's nothing wrong with that and making PW timezone-aware would add
useless complexity, but in a world-wide collaboration a lot of confusion
may arise as the timezone is not reported at all. Instance's local time
might be very different from the local time of CI integrating with PW,
which is different from the local time of person dealing with it etc.
Use UTC everywhere by default instead of UTC for sumbissions and local
timezone for internally generated events (which is not reported).
Signed-off-by: Veronika Kabatova <vkabatov at redhat.com>
---
Changes: Implement Daniel's idea about using datetime.datetime.utcnow
---
docs/api/rest.rst | 3 +-
patchwork/migrations/0024_timezone_unify.py | 46 ++++++++++++++++++++++
patchwork/models.py | 12 +++---
patchwork/notifications.py | 4 +-
patchwork/signals.py | 2 +-
patchwork/templates/patchwork/submission.html | 4 +-
patchwork/tests/test_checks.py | 10 ++---
patchwork/tests/test_expiry.py | 8 ++--
patchwork/tests/test_notifications.py | 2 +-
patchwork/tests/utils.py | 6 +--
.../notes/unify-timezones-0f7022f0c2a371be.yaml | 5 +++
11 files changed, 77 insertions(+), 25 deletions(-)
create mode 100644 patchwork/migrations/0024_timezone_unify.py
create mode 100644 releasenotes/notes/unify-timezones-0f7022f0c2a371be.yaml
diff --git a/docs/api/rest.rst b/docs/api/rest.rst
index 3d7292e..d526b27 100644
--- a/docs/api/rest.rst
+++ b/docs/api/rest.rst
@@ -107,7 +107,8 @@ Schema
------
Responses are returned as JSON. Blank fields are returned as ``null``, rather
-than being omitted. Timestamps use the ISO 8601 format::
+than being omitted. Timestamps use the ISO 8601 format, times are by default
+in UTC::
YYYY-MM-DDTHH:MM:SSZ
diff --git a/patchwork/migrations/0024_timezone_unify.py b/patchwork/migrations/0024_timezone_unify.py
new file mode 100644
index 0000000..99f0642
--- /dev/null
+++ b/patchwork/migrations/0024_timezone_unify.py
@@ -0,0 +1,46 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.10.8 on 2018-02-22 23:11
+from __future__ import unicode_literals
+
+import datetime
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ ('patchwork', '0023_submissiontag'),
+ ]
+
+ operations = [
+ migrations.AlterField(
+ model_name='check',
+ name='date',
+ field=models.DateTimeField(default=datetime.datetime.utcnow),
+ ),
+ migrations.AlterField(
+ model_name='comment',
+ name='date',
+ field=models.DateTimeField(default=datetime.datetime.utcnow),
+ ),
+ migrations.AlterField(
+ model_name='emailconfirmation',
+ name='date',
+ field=models.DateTimeField(default=datetime.datetime.utcnow),
+ ),
+ migrations.AlterField(
+ model_name='event',
+ name='date',
+ field=models.DateTimeField(default=datetime.datetime.utcnow, help_text=b'The time this event was created.'),
+ ),
+ migrations.AlterField(
+ model_name='patchchangenotification',
+ name='last_modified',
+ field=models.DateTimeField(default=datetime.datetime.utcnow),
+ ),
+ migrations.AlterField(
+ model_name='submission',
+ name='date',
+ field=models.DateTimeField(default=datetime.datetime.utcnow),
+ ),
+ ]
diff --git a/patchwork/models.py b/patchwork/models.py
index a8bb015..c5a2059 100644
--- a/patchwork/models.py
+++ b/patchwork/models.py
@@ -308,7 +308,7 @@ class EmailMixin(models.Model):
# email metadata
msgid = models.CharField(max_length=255)
- date = models.DateTimeField(default=datetime.datetime.now)
+ date = models.DateTimeField(default=datetime.datetime.utcnow)
headers = models.TextField(blank=True)
# content
@@ -828,7 +828,7 @@ class Check(models.Model):
patch = models.ForeignKey(Patch, on_delete=models.CASCADE)
user = models.ForeignKey(User, on_delete=models.CASCADE)
- date = models.DateTimeField(default=datetime.datetime.now)
+ date = models.DateTimeField(default=datetime.datetime.utcnow)
state = models.SmallIntegerField(
choices=STATE_CHOICES, default=STATE_PENDING,
@@ -900,7 +900,7 @@ class Event(models.Model):
db_index=True,
help_text='The category of the event.')
date = models.DateTimeField(
- default=datetime.datetime.now,
+ default=datetime.datetime.utcnow,
help_text='The time this event was created.')
# event object
@@ -965,7 +965,7 @@ class EmailConfirmation(models.Model):
email = models.CharField(max_length=200)
user = models.ForeignKey(User, null=True, on_delete=models.CASCADE)
key = HashField()
- date = models.DateTimeField(default=datetime.datetime.now)
+ date = models.DateTimeField(default=datetime.datetime.utcnow)
active = models.BooleanField(default=True)
def deactivate(self):
@@ -973,7 +973,7 @@ class EmailConfirmation(models.Model):
self.save()
def is_valid(self):
- return self.date + self.validity > datetime.datetime.now()
+ return self.date + self.validity > datetime.datetime.utcnow()
def save(self, *args, **kwargs):
limit = 1 << 32
@@ -999,5 +999,5 @@ class EmailOptout(models.Model):
class PatchChangeNotification(models.Model):
patch = models.OneToOneField(Patch, primary_key=True,
on_delete=models.CASCADE)
- last_modified = models.DateTimeField(default=datetime.datetime.now)
+ last_modified = models.DateTimeField(default=datetime.datetime.utcnow)
orig_state = models.ForeignKey(State, on_delete=models.CASCADE)
diff --git a/patchwork/notifications.py b/patchwork/notifications.py
index 88e9662..a5f6423 100644
--- a/patchwork/notifications.py
+++ b/patchwork/notifications.py
@@ -35,7 +35,7 @@ from patchwork.models import PatchChangeNotification
def send_notifications():
- date_limit = datetime.datetime.now() - datetime.timedelta(
+ date_limit = datetime.datetime.utcnow() - datetime.timedelta(
minutes=settings.NOTIFICATION_DELAY_MINUTES)
# We delay sending notifications to a user if they have other
@@ -104,7 +104,7 @@ def expire_notifications():
Users whose registration confirmation has expired are removed.
"""
# expire any invalid confirmations
- q = (Q(date__lt=datetime.datetime.now() - EmailConfirmation.validity) |
+ q = (Q(date__lt=datetime.datetime.utcnow() - EmailConfirmation.validity) |
Q(active=False))
EmailConfirmation.objects.filter(q).delete()
diff --git a/patchwork/signals.py b/patchwork/signals.py
index e5e7370..f7b4f54 100644
--- a/patchwork/signals.py
+++ b/patchwork/signals.py
@@ -65,7 +65,7 @@ def patch_change_callback(sender, instance, raw, **kwargs):
notification.delete()
return
- notification.last_modified = dt.now()
+ notification.last_modified = dt.utcnow()
notification.save()
diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
index 6ed20c3..e817713 100644
--- a/patchwork/templates/patchwork/submission.html
+++ b/patchwork/templates/patchwork/submission.html
@@ -255,7 +255,7 @@ function toggle_div(link_id, headers_id)
<div class="comment">
<div class="meta">
<span>{{ submission.submitter|personify:project }}</span>
- <span class="pull-right">{{ submission.date }}</span>
+ <span class="pull-right">{{ submission.date }} UTC</span>
</div>
<pre class="content">
{{ submission|commentsyntax }}
@@ -271,7 +271,7 @@ function toggle_div(link_id, headers_id)
<div class="comment">
<div class="meta">
<span>{{ item.submitter|personify:project }}</span>
- <span class="pull-right">{{ item.date }} | <a href="{% url 'comment-redirect' comment_id=item.id %}"
+ <span class="pull-right">{{ item.date }} UTC | <a href="{% url 'comment-redirect' comment_id=item.id %}"
>#{{ forloop.counter }}</a></span>
</div>
<pre class="content">
diff --git a/patchwork/tests/test_checks.py b/patchwork/tests/test_checks.py
index c0487f3..797390c 100644
--- a/patchwork/tests/test_checks.py
+++ b/patchwork/tests/test_checks.py
@@ -86,12 +86,12 @@ class PatchChecksTest(TransactionTestCase):
self.assertChecksEqual(self.patch, [check_a, check_b])
def test_checks__duplicate_checks(self):
- self._create_check(date=(dt.now() - timedelta(days=1)))
+ self._create_check(date=(dt.utcnow() - timedelta(days=1)))
check = self._create_check()
# this isn't a realistic scenario (dates shouldn't be set by user so
# they will always increment), but it's useful to verify the removal
# of older duplicates by the function
- self._create_check(date=(dt.now() - timedelta(days=2)))
+ self._create_check(date=(dt.utcnow() - timedelta(days=2)))
self.assertChecksEqual(self.patch, [check])
def test_checks__nultiple_users(self):
@@ -107,7 +107,7 @@ class PatchChecksTest(TransactionTestCase):
self.assertCheckCountEqual(self.patch, 1, {Check.STATE_SUCCESS: 1})
def test_check_count__multiple_checks(self):
- self._create_check(date=(dt.now() - timedelta(days=1)))
+ self._create_check(date=(dt.utcnow() - timedelta(days=1)))
self._create_check(context='new/test1')
self.assertCheckCountEqual(self.patch, 2, {Check.STATE_SUCCESS: 2})
@@ -117,14 +117,14 @@ class PatchChecksTest(TransactionTestCase):
self.assertCheckCountEqual(self.patch, 2, {Check.STATE_SUCCESS: 2})
def test_check_count__duplicate_check_same_state(self):
- self._create_check(date=(dt.now() - timedelta(days=1)))
+ self._create_check(date=(dt.utcnow() - timedelta(days=1)))
self.assertCheckCountEqual(self.patch, 1, {Check.STATE_SUCCESS: 1})
self._create_check()
self.assertCheckCountEqual(self.patch, 2, {Check.STATE_SUCCESS: 1})
def test_check_count__duplicate_check_new_state(self):
- self._create_check(date=(dt.now() - timedelta(days=1)))
+ self._create_check(date=(dt.utcnow() - timedelta(days=1)))
self.assertCheckCountEqual(self.patch, 1, {Check.STATE_SUCCESS: 1})
self._create_check(state=Check.STATE_FAIL)
diff --git a/patchwork/tests/test_expiry.py b/patchwork/tests/test_expiry.py
index 054d156..ce308bc 100644
--- a/patchwork/tests/test_expiry.py
+++ b/patchwork/tests/test_expiry.py
@@ -46,7 +46,7 @@ class TestRegistrationExpiry(TestCase):
return (user, conf)
def test_old_registration_expiry(self):
- date = ((datetime.datetime.now() - EmailConfirmation.validity) -
+ date = ((datetime.datetime.utcnow() - EmailConfirmation.validity) -
datetime.timedelta(hours=1))
user, conf = self.register(date)
@@ -57,7 +57,7 @@ class TestRegistrationExpiry(TestCase):
EmailConfirmation.objects.filter(pk=conf.pk).exists())
def test_recent_registration_expiry(self):
- date = ((datetime.datetime.now() - EmailConfirmation.validity) +
+ date = ((datetime.datetime.utcnow() - EmailConfirmation.validity) +
datetime.timedelta(hours=1))
user, conf = self.register(date)
@@ -68,7 +68,7 @@ class TestRegistrationExpiry(TestCase):
EmailConfirmation.objects.filter(pk=conf.pk).exists())
def test_inactive_registration_expiry(self):
- user, conf = self.register(datetime.datetime.now())
+ user, conf = self.register(datetime.datetime.utcnow())
# confirm registration
conf.user.is_active = True
@@ -87,7 +87,7 @@ class TestRegistrationExpiry(TestCase):
submitter = patch.submitter
# ... then starts registration...
- date = ((datetime.datetime.now() - EmailConfirmation.validity) -
+ date = ((datetime.datetime.utcnow() - EmailConfirmation.validity) -
datetime.timedelta(hours=1))
user = create_user(link_person=False, email=submitter.email)
user.is_active = False
diff --git a/patchwork/tests/test_notifications.py b/patchwork/tests/test_notifications.py
index 6cd3200..6d902f8 100644
--- a/patchwork/tests/test_notifications.py
+++ b/patchwork/tests/test_notifications.py
@@ -120,7 +120,7 @@ class PatchNotificationEmailTest(TestCase):
self.project = create_project(send_notifications=True)
def _expire_notifications(self, **kwargs):
- timestamp = datetime.datetime.now() - \
+ timestamp = datetime.datetime.utcnow() - \
datetime.timedelta(minutes=settings.NOTIFICATION_DELAY_MINUTES + 1)
qs = PatchChangeNotification.objects.all()
diff --git a/patchwork/tests/utils.py b/patchwork/tests/utils.py
index 004c2ca..12b6d9e 100644
--- a/patchwork/tests/utils.py
+++ b/patchwork/tests/utils.py
@@ -214,7 +214,7 @@ def create_check(**kwargs):
values = {
'patch': create_patch() if 'patch' not in kwargs else None,
'user': create_user() if 'user' not in kwargs else None,
- 'date': dt.now(),
+ 'date': dt.utcnow(),
'state': Check.STATE_SUCCESS,
'target_url': 'http://example.com/',
'description': '',
@@ -229,7 +229,7 @@ def create_series(**kwargs):
"""Create 'Series' object."""
values = {
'project': create_project() if 'project' not in kwargs else None,
- 'date': dt.now(),
+ 'date': dt.utcnow(),
'submitter': create_person() if 'submitter' not in kwargs else None,
'total': 1,
}
@@ -275,7 +275,7 @@ def _create_submissions(create_func, count=1, **kwargs):
'submitter': create_person() if 'submitter' not in kwargs else None,
}
values.update(kwargs)
- date = dt.now()
+ date = dt.utcnow()
objects = []
for i in range(0, count):
diff --git a/releasenotes/notes/unify-timezones-0f7022f0c2a371be.yaml b/releasenotes/notes/unify-timezones-0f7022f0c2a371be.yaml
new file mode 100644
index 0000000..2513650
--- /dev/null
+++ b/releasenotes/notes/unify-timezones-0f7022f0c2a371be.yaml
@@ -0,0 +1,5 @@
+---
+other:
+ - |
+ Unify timezones used -- use UTC for both email submissions and internal
+ events.
--
2.13.6
More information about the Patchwork
mailing list