[PATCH v2] Avoid timezone confusion

[Veronika Kabatova]

----- Original Message -----
> From: "Daniel Axtens" <dja at axtens.net>
> To: "Veronika Kabatova" <vkabatov at redhat.com>, "Stephen Finucane" <stephen at that.guru>
> Cc: patchwork at lists.ozlabs.org
> Sent: Tuesday, February 13, 2018 12:36:37 PM
> Subject: Re: [PATCH v2] Avoid timezone confusion
> 
> Veronika Kabatova <vkabatov at redhat.com> writes:
> 
> > ----- Original Message -----
> >> From: "Stephen Finucane" <stephen at that.guru>
> >> To: vkabatov at redhat.com, patchwork at lists.ozlabs.org
> >> Sent: Wednesday, January 17, 2018 8:22:27 PM
> >> Subject: Re: [PATCH v2] Avoid timezone confusion
> >> 
> >> On Wed, 2018-01-17 at 19:18 +0000, Stephen Finucane wrote:
> >> > On Tue, 2018-01-16 at 18:58 +0100, vkabatov at redhat.com wrote:
> >> > > From: Veronika Kabatova <vkabatov at redhat.com>
> >> > > 
> >> > > Patchwork saves patches, comments etc with UTC timezone and reports
> >> > > this time when opening the patch details. However, internally
> >> > > generated
> >> > > processes such as events are reported with the instance's local
> >> > > time.
> >> > > There's nothing wrong with that and making PW timezone-aware would
> >> > > add
> >> > > useless complexity, but in a world-wide collaboration a lot of
> >> > > confusion
> >> > > may arise as the timezone is not reported at all. Instance's local
> >> > > time
> >> > > might be very different from the local time of CI integrating with
> >> > > PW,
> >> > > which is different from the local time of person dealing with it
> >> > > etc.
> >> > > 
> >> > > Use UTC everywhere by default instead of UTC for sumbissions and
> >> > > local
> >> > > timezone for internally generated events (which is not reported).
> >> > > 
> >> > > Signed-off-by: Veronika Kabatova <vkabatov at redhat.com>
> >> > 
> >> > What effect does this have on existing information in the database?
> >> > Does this mean they'll all remain UTC+12 or are they stored in UTC
> >> > format already? The Django docs [1] would lead me to suggest the
> >> > former, given that we don't have USE_TZ set to True.
> >> > 
> >> > I guess you can test that by setting up a deployment, creating
> >> > information, then switching this option over. I'd do this myself but
> >> > I'm not going to have time this week :(
> >> 
> >
> > The effect of the change for the existing data is the same as if the admin
> > decided
> > to override the default TIME_ZONE setting. In the end that's what this
> > change does,
> > move the instance time to UTC.
> >
> > If the TIME_ZONE setting is overriden (production.py) nothing changes, we
> > are only
> > changing the default.
> >
> >
> > To elaborate more, as we use timezone-naive format, the details depend on
> > database
> > backend and what underlying type from it Django uses to store datetimes.
> >
> > For mysql, they are saved as 'datetime' and this type doesn't know
> > timezones and
> > returns what you feed it. Django doesn't modify the times either, generated
> > time
> > is simply passed as a string. If you change timezone, 'datetime' is still
> > the
> > same. So submissions will have the right time, but internally triggered
> > changes
> > keep the original localized time and we can't really modify them since we
> > can't
> > retrieve the original timezone used.
> >
> > For postgres it's the exact opposite. Datetimes are stored as 'timestamp
> > with
> > time zone' in the DB, if no timezone is specified it uses the current local
> > one.
> > So it takes the times from submissions, thinking they are in local time
> > zone and
> > not UTC. OTOH, internally generated events are converted correctly exactly
> > because of the local timezone and consecutive conversion.
> >
> > I have not verified what types are used for other backends but I believe
> > they
> > are similar to the mysql example.
> >
> >
> >> Might be relevant.
> >> 
> >>   https://gist.github.com/aaugustin/2971450
> >> 
> >
> > That's something different. We need to unify the timezones used. Making PW
> > timezone-aware would not change anything, including the effect on old data
> > (exactly because the dates used are not unified, and we have no way to find
> > out the offsets the data were originally created with anyways).
> >
> Wow what a mess :( To make matters worse, Australia/Canberra osciallates
> between UTC+10 and UTC+11! So I agree we just completely give up on the
> correctness of previous data.
> 

OK, this explains the additional inconsistency I've seen and couldn't
explain myself.

> Rather than changing the TIME_ZONE setting, can we use
> datetime.datetime.utcnow as the default for date in Event in models.py,
> rather than datetime.datetime.now?
> 
> We might need to clean up some other uses - PatchChangeNotification and
> Check look like likely suspects. I'm happy to go hunting for those, I
> just want to know if there's a more fundamental reason that I've missed
> why that wouldn't work.
> 

Sure, I'll fix all the .now occurrences. I don't see a reason why it
shouldn't work either.

Veronika


> Regards,
> Daniel
> 
> 
> >
> > Hope this explains the situation, just ask if I wasn't clear
> >
> > Veronika
> >
> >
> >> Stephen
> >> 
> >> > Stephen
> >> > 
> >> > [1] https://docs.djangoproject.com/en/2.0/topics/i18n/timezones/
> > _______________________________________________
> > Patchwork mailing list
> > Patchwork at lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/patchwork
>