Skip to content

Commit

Permalink
Avoid timezone confusion
Browse files Browse the repository at this point in the history
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@redhat.com>
[dja:
 - squash 2 patches: https://patchwork.ozlabs.org/patch/876744/
                     https://patchwork.ozlabs.org/patch/877815/
 - minor changes to both patches - rejig order of migrations and
   adjust wording: "happened sooner" -> "happened earlier"]
Tested-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Daniel Axtens <dja@axtens.net>
  • Loading branch information
veruu authored and daxtens committed Mar 7, 2018
1 parent 5e0e06d commit 8465e33
Show file tree
Hide file tree
Showing 11 changed files with 79 additions and 25 deletions.
3 changes: 2 additions & 1 deletion docs/api/rest.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
46 changes: 46 additions & 0 deletions patchwork/migrations/0023_timezone_unify.py
Original file line number Diff line number Diff line change
@@ -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', '0022_add_subject_match_to_project'),
]

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),
),
]
12 changes: 6 additions & 6 deletions patchwork/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,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
Expand Down Expand Up @@ -836,7 +836,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,
Expand Down Expand Up @@ -908,7 +908,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
Expand Down Expand Up @@ -973,15 +973,15 @@ 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):
self.active = False
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
Expand All @@ -1007,5 +1007,5 @@ def __str__(self):
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)
4 changes: 2 additions & 2 deletions patchwork/notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@


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
Expand Down Expand Up @@ -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()

Expand Down
2 changes: 1 addition & 1 deletion patchwork/signals.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()


Expand Down
4 changes: 2 additions & 2 deletions patchwork/templates/patchwork/submission.html
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ <h2>Message</h2>
<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 }}
Expand All @@ -271,7 +271,7 @@ <h2>Comments</h2>
<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">
Expand Down
10 changes: 5 additions & 5 deletions patchwork/tests/test_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,12 @@ def test_checks__multiple_checks(self):
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):
Expand All @@ -107,7 +107,7 @@ def test_check_count__single_check(self):
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})

Expand All @@ -117,14 +117,14 @@ def test_check_count__multiple_users(self):
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)
Expand Down
8 changes: 4 additions & 4 deletions patchwork/tests/test_expiry.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def register(self, date):
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)

Expand All @@ -57,7 +57,7 @@ def test_old_registration_expiry(self):
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)

Expand All @@ -68,7 +68,7 @@ def test_recent_registration_expiry(self):
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
Expand All @@ -87,7 +87,7 @@ def test_patch_submitter_expiry(self):
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
Expand Down
2 changes: 1 addition & 1 deletion patchwork/tests/test_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ def setUp(self):
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()
Expand Down
6 changes: 3 additions & 3 deletions patchwork/tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,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': '',
Expand All @@ -230,7 +230,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,
}
Expand Down Expand Up @@ -276,7 +276,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):
Expand Down
7 changes: 7 additions & 0 deletions releasenotes/notes/unify-timezones-0f7022f0c2a371be.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
other:
- |
Unify timezones used -- use UTC for both email submissions and internal
events. Please note that this change doesn't modify already existing data
so in case the instance's timezone is UTC+XX, events will appear out of
order (as if they happened earlier) for XX hours in the events API feed.

0 comments on commit 8465e33

Please sign in to comment.