docker / libkv

Distributed key/value store abstraction library
Apache License 2.0
853 stars 205 forks source link

Use fork of go-zookeeper #130

Open talbright opened 8 years ago

talbright commented 8 years ago

Changes zk to use talbright fork (https://github.com/talbright/go-zookeeper).

This fork of zk has a number of fixes and backwards compatible enhancements. One of those enhancements allows for watches to be canceled (https://github.com/talbright/go-zookeeper/pull/14), something that can be added to Watch and WatchTree to make things cleaner.

See zookeeper.go#L185 zookeeper.go#L229

Signed-off-by: Trent Albright trent.albright@gmail.com

thaJeztah commented 8 years ago

Instead of forking, would it be possible to get those changes into go-zookeeper upstream?

talbright commented 8 years ago

Long story short is upstream isn't being maintained consistently or often.

At SFDC we use go-zookeeper internally for some of our apps. I've found over the past 6 months that the author just isn't available. Even critical fixes take far too long to be pulled in, features just as long. The build in the upstream is flakey and broken, making it even more difficult to make changes with confidence. Internally we need a quality, stable zk library we can depend on. So overall my fork addresses more than needing the ability to cancel watches 🎉

Separate question: why doesn't libkv use godeps, glide, etc to manage dependencies?

thaJeztah commented 8 years ago

Long story short is upstream isn't being maintained consistently or often.

alright, thanks!

Separate question: why doesn't libkv use godeps, glide, etc to manage dependencies?

Good question, I know that the jury's still out on which solution fits best in some of the other docker open-source projects. Not sure about this one (I'm not a maintainer in this repository), @abronan possibly knows

abronan commented 8 years ago

Hi @talbright, thank you for the contribution.

I can definitely see how the fork can improve the overall reliability of programs using libkv. We've seen some odd behavior too with the client, especially recently while we switched the Go version.

Genuine question though: is there any way to switch ownership of the main repo or add more maintainers to deal with the situation? While we might be strongly leaning towards switching to the fork because it includes some much needed features and reliability improvement (from the very few I can see), it would be nice to bring everyone together and make sure we modify the same codebase rather than seeing the two repos evolve on their own. This would really benefit the quality of the library imho. Maybe it's not possible but just wondering.

On the question of dependency management, libkv has been used historically as a library by Swarm or Docker. So the dependency management part was left to those projects (to include etcd, consul, zookeeper client dependencies, etc.). I'm not sure this makes sense to use dependency management here as libkv is not supposed to be used standalone. This would make sense (and make things very interesting) if we add a common control CLI to all the distributed stores.

talbright commented 8 years ago

You bet!

Genuine question though: is there any way to switch ownership of the main repo or add more maintainers to deal with the situation? While we might be strongly leaning towards switching to the fork because it includes some much needed features and reliability improvement (from the very few I can see), it would be nice to bring everyone together and make sure we modify the same codebase rather than seeing the two repos evolve on their own. This would really benefit the quality of the library imho. Maybe it's not possible but just wondering.

I agree, it would be highly preferable to have one code base, and bring everyone together. I'm more than willing to add contributors to my fork, but I'm not sure if that's gonna work upstream when the author is kinda mia for stretches. For every fix or enhancement I merged in, even if it needed major reworking, I referenced the upstream PR to encourage discussion. See similar conversation in hashicorp/vault#1367

jefferai commented 8 years ago

@talbright Another option would be to see if the upstream author will be willing to add collaborators onto the repo.