Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update go dependencies #7050

Merged
merged 3 commits into from
Jul 25, 2016
Merged

update go dependencies #7050

merged 3 commits into from
Jul 25, 2016

Conversation

corylanou
Copy link
Contributor

@corylanou corylanou commented Jul 22, 2016

Our dependencies were getting fairly out of date. Getting them updated prior to 1.0 GA release.

This is experimental to see how the tests do. We should take a close look at the changes to make sure it won't affect any compatiblity with file encoding,etc.

Here is a list comparisons for each dependency updated in this PR.

BurntSushi/toml@f0aeabc...master
boltdb/bolt@6d4e6a3...master
davecgh/go-spew@fc32781...master
dgrijalva/jwt-go@40bd0f3...9b486c8
dgryski/go-bits@86c69b3...master
dgryski/go-bitstream@27cd597...master
dvsekhvalnov/jose2go@6387d3c...master
gogo/protobuf@74b6e9d...master
golang/snappy@723cc1e...master
jwilder/encoding@b421ab4...master
peterh/liner@82a939e...master

It is important to note that https://github.com/dgrijalva/jwt-go has went through a major upgrade and will require a refactor of our code to re-implement to get to the current version of the package.

Required for all non-trivial PRs
  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated

Related to https://github.com/influxdata/plutonium/pull/387

@corylanou
Copy link
Contributor Author

I tested this locally by starting a binary up that didn't have the updates. Loaded a bunch of data for strings, ints, floats, etc. Waited for compaction to run. Then started a new binary with the new dependencies and everything looked good.

/cc @mark-rushakoff @jwilder

@@ -63,7 +63,7 @@ func Test_StringEncoder_Multi_Compressed(t *testing.T) {
t.Fatalf("unexpected encoding: got %v, exp %v", b[0], stringCompressedSnappy)
}

if exp := 47; len(b) != exp {
if exp := 51; len(b) != exp {
Copy link
Contributor

Choose a reason for hiding this comment

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

I looked through the diff of what changed in snappy (golang/snappy@5979233...d9eb7a3) a bit, but it wasn't obvious to me why the encoded length went up by 4 bytes - presumably an implementation detail, maybe it will change again in the future?

Would it be better to just assert that the encoded length is less than some fraction of the input length?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would make the test less brittle (still brittle, just less). I'm fine with it. @jwilder thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd rather keep the length check and have the test fail so someone investigates why as opposed to possibly letting a data corruption bug sneak in quietly.

@mark-rushakoff
Copy link
Contributor

I like the renamed test functions. 👍 from me after you make a decision on the length assertion.

@jsternberg
Copy link
Contributor

👍

@corylanou corylanou merged commit 2c1a65f into master Jul 25, 2016
@corylanou corylanou deleted the cjl-dep-updates branch July 25, 2016 13:52
@timhallinflux timhallinflux added this to the 1.0.0 milestone Dec 19, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants