go.uuid
go.uuid copied to clipboard
satori
Breaking API Change
https://github.com/satori/go.uuid/commit/0ef6afb2f6cdd6cdaeee3885a95099c63f18fc8c breaks the API that's been stable for years. Why is this important enough to change with no warning, despite the community standard to be avoiding breaking API changes without some kind of versioning system? The original issue indicates some sort of deprecation warning was considered, but seems to never have been implemented. If users are going to have to rug pulled out from under them with no warning, why should we use this library?
I just hit the same problem with github.com/markbates/pop and github.com/gobuffalo/buffalo this breaks both projects.
I agree with @packrat386 this is a big breaking API change that was sprung on everyone.
Can we please revert that change, for now, and create a path to making the breaking change?
We've already fixed it in our code, so honestly undoing it is only going to cause more problems for us, but either way this approach to API stability is a major problem. This should have been NewV1WithError() or something.
EDIT: I understand that reverting might be the best solution, just pointing out that in some cases the damage is already done.
At this point if you want to change the API, you should create a new project or version it somehow. Your change basically broke a dozen projects at our company.
Agreed with @sbowman , this broke a bunch of our projects.
If you use version tagging, at least those of us using glide and 'go dep' can protect ourselves by pinning our major versions. For example, if you tag f58768 as 1.2.0 and 0ef6af as 2.0.0, the fix is simple on our end.
I agree that the proper immediate fix is to revert, however.
Edit: For clarity, we have already protected ourselves by pinning to the latest August version (0ca0c2). I'm suggesting tagging here simply as a good pattern to follow.
I agree with you all that these kind of breaking changes should be treated in a much more soft way. BUT, consider two things:
- It's a personal project and he can do whatever he wants,
- If you're pinning to master branch, you're already doing it wrong.
This is also an issue for us because we rely on the dep chain hermes -> sprig -> go.uuid.
Could we introduce another method instead of making a breaking change, like uuid.NewV4Safe() or something along those lines ?
Original request to make the change that broke people, https://github.com/satori/go.uuid/issues/18, talked about "let's warn people for 6-12mo" which I don't think happened.
#18, talked about "let's warn people for 6-12mo" which I don't think happened.
I think that's the important bit here. If there was something like a deprecation warning or a new method, I don't think anyone would be opposed to the change. I, for one, certainly prefer a library that returns an error rather than panicking. The issue isn't with the content of the change, but with how it was rolled out.
regarding how to handle the error, is this error merely transient? ie. can we simply keep retrying to create UUID until we do not get an error?
It can sometimes change more slowly, in an ecosystem that encourages building all from head?
Which was the original, laudable plan, too :-)
-- Sent from Gmail Mobile on iPad
I personally want to support @thurt's suggestion about looping until the method succeeds for uuid.V*. As @jinroh and others pointed out functions that return multiple values should have been implemented separately. I think this project has grown popular to the point where destructive changes in the interface can no longer be excused for the project being personal. Sure it's no Python 3 but still a lot of people, myself included, love and rely on this amazing piece of work.
A version tag pre/post this breaking change would be helpful. In the meantime, I'll try and pin to a specific commit before the change.
There are many options besides committing a breaking change and causing months of collateral damage. If you want a good example of what happens when a project has a breaking change look at https://github.com/sirupsen/logrus/issues/451 - as projects slowly update their code you end up with some dependencies fixed, some broken for many months. It's most painful for the people who import the broken packages (like me, importing osrg/gobgp), because I have no control over when it is fixed.
The best course of action is to revert and create a proper version and import path for those who ARE in the position to update and wish to do so. This will also allow both versions to live side by side. Please don't leave all these projects dealing with random interruptions as various projects update over the next N months :/
EDIT: To be clear the faster it is reverted the least damage done to the ecosystem. The people who already fixed their changes are clearly agile enough to revert and it's fresh on their mind so they will know exactly where to look. The people who have not yet had to research this will never have to. Leave the change and EVERYONE who uses this library will eventually be affected.
For those using dep, I changed my Gopkg.toml to this, which pins to the last commit on Jan. 2, the day before the change:
[[constraint]]
name = "github.com/satori/go.uuid"
revision = "063359185d32c6b045fa171ad7033ea545864fa1"
Not the best solution, but unbreaks things for now.
EDIT: Use [[override]] instead of [[constraint]] if it's a transitive dependency: https://github.com/golang/dep/blob/master/docs/Gopkg.toml.md#override
Please let me apologize in advance for jumping the gun here but I was starting to get the impression that reverting the interface was pretty urgent. I tried not to destroy the modified interface as much as possible while reverting the global namespace NewV* functions (which I believe is the heart of most issues) to the original function signatures. If the NewV* methods for the Generator interface needs to be reverted there is a whole lot more to do (which might be better off just completely reverting the past few commits).
though breaking changes bring tons of damage to developers, but I want to say why don't you use release version, like v1.1.0?
Also seeing fails on github.com/Masterminds/sprig and github.com/qntfy/kazaam
To all: Please don't complain when you import from master and it breaks. The developer is doing the best he can. If you are depending on 3rd party code, you need to take responsibility for managing your dependencies yourself. There are tools for this, learn about them and use them, thanks.
To @satori: What would help me a lot is to make a v1.1.1 tag on commit 0ca0c2cf9e6f8d79757fdc089a34c854ad557138, i.e. before the recent round of development work.
Then I can change my imports to gopkg.in/satori/go.uuid.v1 and everything will go back to how it was 6 days ago. People who are using dep would also be able to pin it by name instead of by commit hash.
Then when you are satisfied with the new API and know you won't be changing it more, you can tag v2.0.0 to indicate that this is a breaking change.
Thanks for your work on this.
@jeffallen you have to remember that go get pulls from master. That’s why a lot of packages are breaking. I feel the pain of managaing open source projects, so I get where you’re coming from. With that said, if you look at the imports this is a very widely used project. So changes do affect a lot of people, businesses, and packages.
With all that said it’s clear this isn’t getting reverted any time soon. So @satori can you please tag this version? Not having a tagged version is also causing problems with tools like dep that are pulling an older version. So those packages that have updated to the new code are still breaking with those tools because dep is pulling the last tagged version, which is the old API.
At this point most people’s code are fine against master, but if using a tool like dep, they’re broken. People now need to go in and change there config a to point to master. A tag would solve that and we can all move on.
@jeffallen When backwards compatibility is broken for thousands of projects people have every right to voice their concern. There is a reason Go has the backwards compatibility guarantee, it was designed to have stable API's distinguished by import path. Some tools may have recently emerged but the build system is still not very friendly for dependency management.
The issue with this library is that it is minimal enough that other libraries import it. Just this morning I realized a library dependency I need to update has fixed their version of the library. However https://github.com/osrg/gobgp which has a set of applications as well as a public exported client API https://godoc.org/github.com/osrg/gobgp/api uses "dep" and locks in the older uuid pkg. Now I need to figure out how two libraries I depend on with incompatible API requirements with package T's they export that my library has to interact with can live in my workspace. As far as I know, this isn't possible. So I am going to have to cp -a go.satori go.satoriv2 and manually sed one of them, then create a compat pkg for my interaction points with these libs.
If this was a security issue, or serious design flaw I would have had no issues with it. I'm reasonable and if the merit was there I would be fine with it. Instead I am spending dev cycles for a breaking API change that doesn't add any real value in practice as I've mentioned here https://github.com/satori/go.uuid/pull/68#issuecomment-355674448. We have essentially broken thousands of packages for a scenario that is so unlikely to happen in any real production system the chances may as well be zero. This is why I suggested reverting it as soon as possible to minimize damage and talk about why the changes were being made to see if it was justified. Every day the change stays live is another round of dependencies updating, causing another round of pain the next day for a different group of developers and this will go on for months as we saw with logrus.
Anyways- it's not to late to fix this, but you know @satori it would be nice if you at least posted here on what your plans are. If you have no intentions on reverting so be it, but confirming those intentions will allow everyone to commit to a single resolution more quickly rather than trying to hold off on fixing because it may be reverted. Thanks.
This dependency https://github.com/satori/go.uuid/commit/676762fded691d9dd8c823ef059c31005e49ec56#diff-5d96e115a63d4a2a5264c3f3e412633dR32 was the beginning of the end...
Looking forward to use this replacement: https://github.com/google/uuid , some thoughts?
Take it easy, fellow gophers; fixing broken code is what we do everyday ;-)
@jeffallen @cstockton @satori
First of all, I want to say thank you to @satori for doing such a good job on a widely used project. I know it's not easy working open-source projects.
But more importantly, I just wanna understand the reason for the change in adding an error return to the NewVN` functions. I'm actually curious at what kind of error is returned and how often it would happen. I'm not exactly familiar with the inner workings of UUID and how randomness is achieved. But I'm interested to know the technical reasons why this change was made.
Tagging people based on activity in this thread who might know why.
@luiscvega fyi. #18 should be the original issue addressing the problem. @cstockton provides some detail on the topic in #68 which was insightful for me so it might be worth a read. I'm sorry if you've already referenced them.
@qiangli fixing broken code should not be what we do everyday, there are a bunch of other things to deal with. @satori go.uuid is a great library and it solves a real need in the golang ecosistem, it is a great work. Breaking backwards compatibility is a pain in the ass, but in terms of time economy, think about the amount of effort required to update all projects. If we suppose only 1659 projects (guessing 1 star = 1 project) and 10 minutes to adapt to the new library interface we have more than 270 hours of work dedicated to a makeup thing. Just to learn, what alternatives did you consider to avoid the backwards compat issue?
PS: sorry @all for my previous hysterical comment.
A v1.2.0 has been tagged that doesn't have the backwards incompatible changes. Please pin your dependencies to that.
@satori @daenney can we please get a 1.3.0 tag on master that has the new changes. we've all gone and changed our code to use the new changes, but because there isn't a tag for it dep breaks because it's finding the 1.2.0 release an using that instead of the new stuff.
can we please get a 1.3.0 tag on master that has the new changes.
If the project is tagged/versioned by Semantic Versioning, shouldn't this tag be more appropriately called 2.0.0 since the changes are not backwards compatible?
@TyBrown I'll take whatever I can get at this point. :) but yes, 2.0.0 would make more sense
@satori can you please cut a new release against master? When that’s done I think this ticket can be finally closed. Right now because they’re is no tag this ticket is hurting everyone who uses a tool like dep to manage package dependencies. We’ve all fixed the packages that were broken with this ticket, but things are still broken because there is no tag that corresponds to these new changes and tools like dep pull the latest tagged version which is the old changes. Please tag master. Please.
If anyone here is capable of tagging master please do so. This is causing real harm out there.
For what it's worth, I forked this project & will maintain the old API as is: https://github.com/kevinburke/go.uuid
@satori or anyone else, will you please cut a new tag for the master branch? Please! I’m sick of having to tell everyone that tries to use a package manager that they need to pin to the master branch. Cut a new tag and then close this issue. Code changes, we get it, we’ve all updated, but you’re still causing a world of hurt because no one can take 10 seconds and cut a new tag. Add me to the repo and I’ll do it, if no one else can be bothered with it. This ticket has been open for a month now. Cut a new release so the community can move on.
I was bitten by this issue today as well. Please add a 2.0.0 tag to the current master as @markbates suggests.
If anybody is coming here from Googling build errors and you're using dep then you'll need to change your Gopkg.toml to point to the master branch for go.uuid.
Like so:
[[override]]
name = "github.com/satori/go.uuid"
revision = "master"
@mattwilliamson that's not a helpful way to express frustration
For what it's worth -- when you have a project which people rely on - breaking the API in this manner warrants that level of response. It's egregious.
It's not like we are owed anything by @satori; as unpleasant as it is to break large parts of the ecosystem, there are ways around the breakage.
It's not like we are owed anything by @satori; as unpleasant as it is to break large parts of the ecosystem, there are ways around the breakage.
very true -- but you need to follow semantic versioning at the very least. When you have users relying on your library -- you should at least consider that.
but you need to follow semantic versioning at the very least
It might be polite, but is not required, nor are we (as non-paying non-customers) entitled to it :-)
you should at least consider that
It's nice and useful, but again, adhering to it is merely politeness.
I'm very for this change. Took me less than 10 minutes to fix all of my projects with simple find and replace. I'm now no longer using recover() for these functions.
This comment is for those who are just starting out with the package and are using the 'NewV4()' method. You may try this if you're following a tutorial: id := uuid.NewV4().String() which will generate "multiple-value uuid.NewV4() in single-value context" error, Instead do this: id := uuid.Must(uuid.NewV4()).String()
It could have easily been done by adding a new function instead of breaking tons and tons of projects. I, myself have about 20 that were with 8 developers on my team -- 160 changes not including laptops. Switching to github.com/google/uuid was faster and easier than implementing error checking.
By the way, why should generating a uuid ever fail? It's just a stinking random byte array. Fail to get MAC address? Fail to get timestamp? At this point I would rather have the application panic and dump a trace, because something is catastrophically wrong.
Retry or return nil? Backwards compatibility is important stuff. I don't know why you would downplay it.
I'm not downplaying it. See earlier in the thread where I described how I maintain a fork now. You asked why it could return an error and I told you.
It's been three months so I feel like this issue is pretty much, technically, closed in terms of discussion (unless we hear back from @satori ). No reversion. Deal with it, or resort to an alternative.
tl;dr
What happened?
Some old interfaces (uuid.NewV1, uuid.NewV2, and uuid.NewV4) were broken: wrap them with uuid.Must to achieve identical results (e.g. uuid.Must(uuid.NewV4())).
Alternatives?
If you want to use a pre-change compliant API go with kevinburke/go.uuid. google/uuid is another seemingly good alternative but beware as it publicly states that it may be unstable (although I doubt there being any huge API change).
That’s fine. We just want him to tag it so that we can use dep without overrides.
Some members of the Go community are now maintaining a fork of this repo. We've just cut a v2.0.0 which is for the breaking change. Our fork keeps v1.2.0 available, if you still need it:
- https://github.com/gofrs/uuid
