timeutil: embed Go time zone data into CockroachDB#56634
timeutil: embed Go time zone data into CockroachDB#56634craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
|
This change is |
knz
left a comment
There was a problem hiding this comment.
LGTM, but can you investigate what the acceptance test is trying to tell you?
Also because of this PR we will now need to be more proactive about upgrading the go version that we use (or manually pull in new versions of tzdata into patch releases). What do you and @jlinder think about this? What's a good way to do this moving forward?
Reviewed 11 of 11 files at r1.
Reviewable status:complete! 0 of 0 LGTMs obtained
|
pretty sure the acceptance test failure is because i forgot to update the builder image. sending that for a re-run.
feels like every major release from now on needs an explicit review of the embedded tzdata? i'm not totally sure what to answer here. it is a fallback in the end, so if users care about up-to-date tz data they would need to ensure their local tzdata packages are up to date. |
0a85978 to
c4c8938
Compare
|
looks like the gce workers are go1.14. how do they get installed? |
* Use go's time/tzdata to embed timezone data. * Modify timeutil package to always use this package. * Remove cli checks for tzdata. * Add lint rule to use timeutil.LoadLocation The binary size changes by ~1MB here. Release note (general change): The timezone data is now built in to the CockroachDB binary, which is the fallback source of time if tzdata is not located by the default Go standard library.
|
I thought this was from scripts/gceworker.sh but apparently there's already 1.15 in there. Also I see your CI is fine now. So I'd say you can probably merge this? |
|
It was the need to run packer and update the TC image...
…On Mon, 16 Nov 2020, 9:58 pm kena, ***@***.***> wrote:
I thought this was from scripts/gceworker.sh
which in turn runs build/bootstrap/bootstrap-debian.sh
but apparently there's already 1.15 in there.
Also I see your CI is fine now.
So I'd say you can probably merge this?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#56634 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AA32FQ5GC6WUD5E6MAHSZYTSQEAVFANCNFSM4TT6IUOQ>
.
|
|
bors r=knz |
|
Build succeeded: |
Refs #36864
The binary size changes by ~1MB here.
Release note (general change): The timezone data is now built in to the
CockroachDB binary, which is the fallback source of time if tzdata is
not located by the default Go standard library.