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 snowflake dependency and add more tests #1738

Merged
merged 2 commits into from
Apr 27, 2022

Conversation

dnephin
Copy link
Contributor

@dnephin dnephin commented Apr 26, 2022

Summary

While working on a change to our API, I ran into some unexpected behaviour. I traced it to this bug fix, which we don't have yet because there has not been a release of this project for 3 years.

This PR adds some more tests, and updates the dependency.

@dnephin dnephin requested a review from a team as a code owner April 26, 2022 21:14
@dnephin dnephin force-pushed the dnephin/update-and-fix-snowflake-ids branch 2 times, most recently from 08c9ec6 to 25b17d8 Compare April 26, 2022 22:08
@dnephin dnephin mentioned this pull request Apr 26, 2022
7 tasks
To pickup some bug fixes from dnephin/snowid@7511dd2

Which fix problems with uid.Parse
id: "JPwcyDCgEuqJPwcyDCgEuq",
expectedErr: `invalid base58 id "JPwcyDCgEuqJPwcyDCgEuq"`,
},
// does not result in an error, but probably should
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we keep this commented out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ya, we can't fix this one yet until bwmarrin/snowflake#45 is merged, or we use a fork. I can add a link.

@dnephin dnephin changed the title Update snowflow dependency and add more tests Update snowflake dependency and add more tests Apr 27, 2022
@dnephin dnephin force-pushed the dnephin/update-and-fix-snowflake-ids branch from 25b17d8 to 1a99f24 Compare April 27, 2022 16:22
@dnephin dnephin merged commit 77fd505 into main Apr 27, 2022
@dnephin dnephin deleted the dnephin/update-and-fix-snowflake-ids branch April 27, 2022 16:28
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.

3 participants