volatiletech / sqlboiler

Generate a Go ORM tailored to your database schema.
BSD 3-Clause "New" or "Revised" License
6.73k stars 544 forks source link

Replace satori uuid #330

Closed cameracker closed 6 years ago

cameracker commented 6 years ago

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 https://github.com/volatiletech/sqlboiler/pull/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 https://github.com/volatiletech/sqlboiler/pull/275

Unfortunately this advice is irresponsible, because satori/uuid has a critical defect where it doesn't generate random UUIDV4s. https://github.com/satori/go.uuid/issues/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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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