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

Replace satori uuid #330

Closed
cameracker opened this issue Jul 10, 2018 · 8 comments
Closed

Replace satori uuid #330

cameracker opened this issue Jul 10, 2018 · 8 comments

Comments

@cameracker
Copy link

cameracker commented Jul 10, 2018

What version of SQLBoiler are you using (sqlboiler --version)?

v2.7.3

Further information. What did you do, what did you expect?

As I think you guys are probably aware given the conversation on #236, the maintainer of satori/uuid has been unresponsive for the last 5 months and has not been doing basic paperwork, like tagging the repo, to allow insulation from backwards incompatible changes. Because we use (and are fond of :)) this tool, we are now forced to upgrade satori/uuid, which is advice that has been passed to users recently #275

Unfortunately this advice is irresponsible, because satori/uuid has a critical defect where it doesn't generate random UUIDV4s. satori/go.uuid#73

It would be greatly appreciated if you guys either:

  1. Entertained the idea of using a different alternative to satori/uuid, perhaps a fork that fixes this issue or an alternative like https://github.com/pborman/uuid (which is used in kubernetes)
  2. Committed a known safe version of satori/uuid in the repo at least temporarily until vgo is mature (the reluctance to use dep is understood)

Thanks for your consideration

@aarondl
Copy link
Member

aarondl commented Jul 10, 2018

I'm very much in favor of moving away from that library. I'll check it out as soon as I can and it'll be done before v3 is finalized if that library is better.

@cameracker
Copy link
Author

cameracker commented Jul 10, 2018

Cool, good to hear.

I've been digging around most of the day trying to figure out a good solution for this problem. We are choosing to migrate away from satori/uuid now so that we don't have to compete with sqlboiler.

Research suggests that some good alternatives would be:

EDIT: Upon a great deal of evaluation, the google/uuid and pborman/uuid implementations seem to have some code smells (like copy pasted code) and have not been tagged in over a year (which is a big deal for dep users). I think we're going to go with the tideland/godsa option, FWIW

Hope the update noise hasn't been annoying and the post is helpful

@aarondl
Copy link
Member

aarondl commented Jul 11, 2018

No its super helpful. Though I'm curious what you mean about compete with sqlboiler, seems like a lot of work to create your own data access mechanism over a uuid library :p

Will be looking into godsa when I can.

@cameracker
Copy link
Author

Sorry, should elaborate. We had locked to 1.2.0 of satori/go.uuid via dep, so sqlboilers usage of latest/master for the unit test generation was what forced us to upgrade. For us getting away from satori\go.uuid is the best way to play nice with all of our tools :)

@aarondl
Copy link
Member

aarondl commented Jul 11, 2018

I noticed immediately that the godsa library is way broader than necessary. Quite a large dependency. I'm actually considering taking their uuid v4 and extracting it to another library so we don't have to deal with the rest of it. Thoughts?

@cameracker
Copy link
Author

cameracker commented Jul 11, 2018

That's actually a really good point that I didn't consider. The identifier subpackage is self contained and has no other dependencies to anything in the godsa library. I think the approach you suggest would be good in the long term, particularly for such an algorithmically simple feature.

EDIT: Also it'd insulate you from having to deal with this sort of thing in the future 🙃

@jakebailey
Copy link

Just as an FYI after stumbling on this issue, there's a community maintained fork of satori UUID: https://github.com/gofrs/uuid

It should be a drop-in replacement without the problems and inactivity of the original.

aarondl added a commit that referenced this issue Jul 30, 2018
@aarondl
Copy link
Member

aarondl commented Jul 30, 2018

Thanks for that package @jakebailey. Made this change easy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants