[PATCH v3] Avoid timezone confusion
Daniel Axtens
dja at axtens.net
Fri Feb 23 11:16:55 AEDT 2018
At a quick glance this looks good. I will try to test it this weekend,
time permitting.
Thanks!
Regards,
Daniel
vkabatov at redhat.com writes:
> 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
>
> _______________________________________________
> Patchwork mailing list
> Patchwork at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
More information about the Patchwork
mailing list