----- Original Message ----- > From: "Daniel Axtens" <d...@axtens.net> > To: "Veronika Kabatova" <vkaba...@redhat.com>, "Stephen Finucane" > <step...@that.guru> > Cc: patchwork@lists.ozlabs.org > Sent: Tuesday, February 13, 2018 12:36:37 PM > Subject: Re: [PATCH v2] Avoid timezone confusion > > Veronika Kabatova <vkaba...@redhat.com> writes: > > > ----- Original Message ----- > >> From: "Stephen Finucane" <step...@that.guru> > >> To: vkaba...@redhat.com, patchwork@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, vkaba...@redhat.com wrote: > >> > > From: Veronika Kabatova <vkaba...@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 <vkaba...@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@lists.ozlabs.org > > https://lists.ozlabs.org/listinfo/patchwork > _______________________________________________ Patchwork mailing list Patchwork@lists.ozlabs.org https://lists.ozlabs.org/listinfo/patchwork
