[PATCH v2] Avoid timezone confusion

[Daniel Axtens]

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.

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.

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