Skip to content
This repository has been archived by the owner on Jan 17, 2024. It is now read-only.

[965] fix daylight savings time detection on Docker containers #757

Merged
merged 2 commits into from Nov 4, 2020

Conversation

aldavidson
Copy link
Contributor

@aldavidson aldavidson commented Nov 4, 2020

Context

Trello card 965 There's an issue that means since the Daylight Savings Time switch back on Oct 24th 2020, Alpine linux by default is not reporting the DST status correctly. This filters through to Ruby and Rails via the tzinfo gem, and means that datetimes are being stored with an incorrect UTC offset.

For more details, see this tzinfo issue tzinfo/tzinfo#120:

Hopefully this will be fixed in a future release of tzinfo, at which point we'll be able to remove this workaround from our Dockerfile. As the maintainer of the project says:

Had I known this change was going to be made in 2020b, I'd have planned to add support for the POSIX-style TZ string beforehand. I'm working on a change to add this support now for both v2.x and v1.x.

But also :

Please note that TZInfo is and always has been an unpaid side project for me. I am only able to work on it in my spare time and my spare time is finite.

Changes proposed in this pull request

Recompile the tzdata as suggested in this comment on the tzinfo issue

Guidance to review

Before (on production):

> Time.zone.now
=> Wed, 04 Nov 2020 13:47:40 BST +01:00
> Time.zone.now.dst?
=> true

After (tested on dev):

> Time.zone.now
=> Wed, 04 Nov 2020 13:14:09 GMT +00:00
> Time.zone.now.dst?
=> false

@muqi1 muqi1 temporarily deployed to ghwt-review-pr-757 November 4, 2020 13:36 Inactive
@@ -7,6 +7,10 @@ USER root
# dependencies relied upon to build native-extension gems etc
RUN apk update
RUN apk add libxml2-dev libxslt-dev build-base postgresql-dev tzdata
# Fix incompatibility with slim tzdata from 2020b onwards
RUN wget https://data.iana.org/time-zones/tzdb/tzdata.zi -O /usr/share/zoneinfo/tzdata.zi && \
Copy link
Contributor

@benilovj benilovj Nov 4, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few questions:

  • Is there a link you can add which explains this in more detail, in case someone's wondering about it?
  • Is this a temporary issue that will go away soon (i.e. that this step can be taken back out)
  • how much does this slow the build by?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Yep, the three links in the "This filters through to Ruby and Rails via the tzinfo gem" line of the description are links to explanations of details from the different perspectives. I could maybe be more explicit, true - I'll update the description with a long-form URL

  2. Hopefully a future version of tzdata will fix it, at which point we can remove this line

  3. A few seconds, not long

Copy link
Contributor

@asmega asmega left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice one!

@muqi1 muqi1 temporarily deployed to ghwt-review-pr-757 November 4, 2020 13:57 Inactive
@aldavidson aldavidson merged commit 38b833e into main Nov 4, 2020
@aldavidson aldavidson deleted the 965-fix-timezones branch November 4, 2020 14:02
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants