[v3] Avoid timezone confusion

Message ID	20180222152446.20208-1-vkabatov@redhat.com
State	Accepted
Delegated to:	Daniel Axtens
Headers	show Return-Path: <patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org> From: vkabatov@redhat.com To: patchwork@lists.ozlabs.org Subject: [PATCH v3] Avoid timezone confusion Date: Thu, 22 Feb 2018 16:24:46 +0100 Message-Id: <20180222152446.20208-1-vkabatov@redhat.com> Precedence: list MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" <patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org>
Series	[v3] Avoid timezone confusion \| expand [v3] Avoid timezone confusion

Context	Check	Description
dja/snowpatch-0_1_0	success	master/apply_patch Successfully applied
dja/snowpatch-snowpatch_job_snowpatch-patchwork	success	Test snowpatch/job/snowpatch-patchwork on branch master

Veronika Kabatova Feb. 22, 2018, 3:24 p.m. UTC

From: Veronika Kabatova <vkabatov@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@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

Daniel Axtens Feb. 23, 2018, 12:16 a.m. UTC | #1

At a quick glance this looks good. I will try to test it this weekend,
time permitting.

Thanks!

Regards,
Daniel

vkabatov@redhat.com writes:

> From: Veronika Kabatova <vkabatov@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@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@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

Daniel Axtens Feb. 23, 2018, 2:44 p.m. UTC | #2

Hi Veronika,

I have become briefly sidetracked by Docker's default behaviour to set
the timezone in the container to UTC. However, I did have 1 question:

> 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'),
> +    ]
This migration is numbered 24, and depends on 23. I only see migrations
up to 21 in master. I assume you've accidentally based this on some
internal patches. I fixed this up so you don't need to respin, but I
just wanted to check that there wasn't anything in the missing
migrations that you needed for this patch.

Regards,
Daniel

Veronika Kabatova Feb. 23, 2018, 2:57 p.m. UTC | #3

----- Original Message -----
> From: "Daniel Axtens" <dja@axtens.net>
> To: vkabatov@redhat.com, patchwork@lists.ozlabs.org
> Sent: Friday, February 23, 2018 3:44:25 PM
> Subject: Re: [PATCH v3] Avoid timezone confusion
> 
> Hi Veronika,
> 
> I have become briefly sidetracked by Docker's default behaviour to set
> the timezone in the container to UTC. However, I did have 1 question:
> 
> > 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'),
> > +    ]
> This migration is numbered 24, and depends on 23. I only see migrations
> up to 21 in master. I assume you've accidentally based this on some
> internal patches. I fixed this up so you don't need to respin, but I
> just wanted to check that there wasn't anything in the missing
> migrations that you needed for this patch.
> 

Hi, good catch! I've been working on some other features as well, namely
we have a migration for the list splitting feature that I sent the v2 a
week ago, and then I'm working on issues #57 and #113 so my testing
instance has these deployed and makemigrations script that generated the
migration took it into account.

So no, there shouldn't be any dependency, we'd just need to rename list
splitting migration no. 22 that I already sent.


Veronika

> Regards,
> Daniel
>

Daniel Axtens Feb. 24, 2018, 1:58 a.m. UTC | #4

Hi Veronika,

I am testing this now and I think I'm happy with it.

However, I set up a container in UTC+11, and while watching the events
api endpoint:
 - I added some patches without this patch
 - I applied this patch and migrated
 - I added some more patches
 - I noticed that the new events were no longer added to the front of
 the events list; they were now 11 hours back.

This is, on reflection, a straight-forward consequence of moving from
UTC+11 to UTC without attempting to migrate old data.

I still think migration is an incredibly difficult and error-prone waste
of time, so I still don't want to go down that road. What I do want is a
piece of documentation in the release notes to warn people that if you
are in UTC+n, your events feed will be out of order for n hours.

I don't know of any users of the event feed at the moment, so I'm not
worried about breaking anything: I just want a couple of sentences so
that any sysadmins/users who are especially observant don't freak out
that they're suddenly losing events.

You can do a respin of this, or just send a new patch on top of this and
I will apply both at the same time.

Regards,
Daniel

vkabatov@redhat.com writes:

> From: Veronika Kabatova <vkabatov@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@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@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

Veronika Kabatova Feb. 26, 2018, 10:54 a.m. UTC | #5

----- Original Message -----
> From: "Daniel Axtens" <dja@axtens.net>
> To: vkabatov@redhat.com, patchwork@lists.ozlabs.org
> Sent: Saturday, February 24, 2018 2:58:56 AM
> Subject: Re: [PATCH v3] Avoid timezone confusion
> 
> Hi Veronika,
> 
> I am testing this now and I think I'm happy with it.
> 
> However, I set up a container in UTC+11, and while watching the events
> api endpoint:
>  - I added some patches without this patch
>  - I applied this patch and migrated
>  - I added some more patches
>  - I noticed that the new events were no longer added to the front of
>  the events list; they were now 11 hours back.
> 
> This is, on reflection, a straight-forward consequence of moving from
> UTC+11 to UTC without attempting to migrate old data.
> 
> I still think migration is an incredibly difficult and error-prone waste
> of time, so I still don't want to go down that road. What I do want is a
> piece of documentation in the release notes to warn people that if you
> are in UTC+n, your events feed will be out of order for n hours.
> 

Yes, the "delayed" events are expected. Additional documentation about
it seems fair.

> I don't know of any users of the event feed at the moment, so I'm not
> worried about breaking anything: I just want a couple of sentences so
> that any sysadmins/users who are especially observant don't freak out
> that they're suddenly losing events.
> 

We tried it but the events API was very slow and unreliable so we
switched back to polling series. I believe Stephen worked on fixing
up the API so hopefully there will be a working solution merged soon :)

> You can do a respin of this, or just send a new patch on top of this and
> I will apply both at the same time.
> 

I'll send an additional patch with releasnote changes and cc you so
you can find it easily.

> Regards,
> Daniel
> 
> vkabatov@redhat.com writes:
> 
> > From: Veronika Kabatova <vkabatov@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@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@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/patchwork
>

Stephen Finucane Feb. 27, 2018, 11:31 a.m. UTC | #6

On Thu, 2018-02-22 at 16:24 +0100, vkabatov@redhat.com wrote:
> From: Veronika Kabatova <vkabatov@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@redhat.com>

This all looks good to me and you've resolved Daniel's once outstanding
comment via the additional reno. As such,

Reviewed-by: Stephen Finucane <stephen@that.guru>

I'll give it a few days to give Daniel the chance to sign off before
applying this.

Stephen

Daniel Axtens Feb. 28, 2018, 12:02 a.m. UTC | #7

Stephen Finucane <stephen@that.guru> writes:

> On Thu, 2018-02-22 at 16:24 +0100, vkabatov@redhat.com wrote:
>> From: Veronika Kabatova <vkabatov@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@redhat.com>
>
> This all looks good to me and you've resolved Daniel's once outstanding
> comment via the additional reno. As such,
>
> Reviewed-by: Stephen Finucane <stephen@that.guru>
>
> I'll give it a few days to give Daniel the chance to sign off before
> applying this.

Sounds good; I just wanted to check that what is stored in the database
makes sense before and after.

[I'm not intuitively sure what TZ the DB will use and I'd like to have a
quick look and compare that with the docs and just check we're not doing
anything stupid. My concern would be if we're creating a time as utc and
the db/django is interpreting it as in the instance timezone and then
converting it to UTC again to store, so we end up storing a time in a
totally weird and meaningless timezone. That or MySQL/Postgres
differences that I at least want to be aware of.]

I should get this applied in the next couple of days.

Regards,
Daniel

>
> Stephen
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

Daniel Axtens March 7, 2018, 2:39 p.m. UTC | #8

Hi Veronika,

Thank you for your patience.

> 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).

I found that Docker set up containers in the UTC timezone, which made
things difficult to test. I patched the dockerfile to put the container
in UTC+11, which should match the TZ value that Django uses.

Without your patch, events were created with local time, stored in - as
far as I can tell - timezone-unaware format in the database.

I then applied your patch.

With the patch, the events were created with UTC time, again stored
AFAICT TZ-unaware.

That all checks out - I was a bit worried Django was going to do
something 'clever' and try to store in UTC and convert back and forth,
and our conversion would lead to some sort of awful double-convert. But
everything is in order so we can proceed :)

So I:
 - made the fixup to the migration number and dependency, as discussed.
 - made a minor edit to the doc fixup (s/sooner/earlier)
 - squashed your two patches
 - added
Tested-by: Daniel Axtens <dja@axtens.net>
 - pushed to master at 8465e33c23310e4873d464fe2581842df2e9c6f8

Thanks again for your contributions to patchwork!

Regards,
Daniel

>
>
> Signed-off-by: Veronika Kabatova <vkabatov@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@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork

Veronika Kabatova March 7, 2018, 3:20 p.m. UTC | #9

----- Original Message -----
> From: "Daniel Axtens" <dja@axtens.net>
> To: vkabatov@redhat.com, patchwork@lists.ozlabs.org
> Sent: Wednesday, March 7, 2018 3:39:01 PM
> Subject: Re: [PATCH v3] Avoid timezone confusion
> 
> Hi Veronika,
> 
> Thank you for your patience.
> 
> > 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).
> 
> I found that Docker set up containers in the UTC timezone, which made
> things difficult to test. I patched the dockerfile to put the container
> in UTC+11, which should match the TZ value that Django uses.
> 
> Without your patch, events were created with local time, stored in - as
> far as I can tell - timezone-unaware format in the database.
> 
> I then applied your patch.
> 
> With the patch, the events were created with UTC time, again stored
> AFAICT TZ-unaware.
> 
> That all checks out - I was a bit worried Django was going to do
> something 'clever' and try to store in UTC and convert back and forth,
> and our conversion would lead to some sort of awful double-convert. But
> everything is in order so we can proceed :)
> 

Glad it works for you! Having unified timezones saved us a lot of time
when debugging things internally so we wanted to help out.

> So I:
>  - made the fixup to the migration number and dependency, as discussed.
>  - made a minor edit to the doc fixup (s/sooner/earlier)
>  - squashed your two patches
>  - added
> Tested-by: Daniel Axtens <dja@axtens.net>
>  - pushed to master at 8465e33c23310e4873d464fe2581842df2e9c6f8
> 

Awesome, thanks! I think you missed Stephen's Reviewed-by [1] in the
commit. It doesn't matter too much to me, just wanted to bring it up
in case he'd like it there :)


[1] https://patchwork.ozlabs.org/patch/876744/#1864659

Thanks again,

Veronika


> Thanks again for your contributions to patchwork!
> 
> Regards,
> Daniel
> 
> >
> >
> > Signed-off-by: Veronika Kabatova <vkabatov@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@lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/patchwork
>

Stephen Finucane March 7, 2018, 4:18 p.m. UTC | #10

On Wed, 2018-03-07 at 10:20 -0500, Veronika Kabatova wrote:

[snip]

> > So I:
> >  - made the fixup to the migration number and dependency, as discussed.
> >  - made a minor edit to the doc fixup (s/sooner/earlier)
> >  - squashed your two patches
> >  - added
> > Tested-by: Daniel Axtens <dja@axtens.net>
> >  - pushed to master at 8465e33c23310e4873d464fe2581842df2e9c6f8
> > 
> 
> Awesome, thanks! I think you missed Stephen's Reviewed-by [1] in the
> commit. It doesn't matter too much to me, just wanted to bring it up
> in case he'd like it there :)
> 
> [1] https://patchwork.ozlabs.org/patch/876744/#1864659

Nope! Glad to have this closed out :)

Stephe

Daniel Axtens March 8, 2018, 12:44 a.m. UTC | #11

Stephen Finucane <stephen@that.guru> writes:

> On Wed, 2018-03-07 at 10:20 -0500, Veronika Kabatova wrote:
>
> [snip]
>
>> > So I:
>> >  - made the fixup to the migration number and dependency, as discussed.
>> >  - made a minor edit to the doc fixup (s/sooner/earlier)
>> >  - squashed your two patches
>> >  - added
>> > Tested-by: Daniel Axtens <dja@axtens.net>
>> >  - pushed to master at 8465e33c23310e4873d464fe2581842df2e9c6f8
>> > 
>> 
>> Awesome, thanks! I think you missed Stephen's Reviewed-by [1] in the
>> commit. It doesn't matter too much to me, just wanted to bring it up
>> in case he'd like it there :)
>> 
>> [1] https://patchwork.ozlabs.org/patch/876744/#1864659
>
> Nope! Glad to have this closed out :)

Yes, I realised shortly after I pushed :/ Sorry Stephen.

Regards,
Daniel.
>
> Stephe

[v3] Avoid timezone confusion

Checks

Commit Message

Comments

Patch