Skip to content

Ensure Keys are watchable with etcd#582

Closed
abronan wants to merge 1 commit intomoby:masterfrom
abronan:ensure_key_is_watchable
Closed

Ensure Keys are watchable with etcd#582
abronan wants to merge 1 commit intomoby:masterfrom
abronan:ensure_key_is_watchable

Conversation

@abronan
Copy link

@abronan abronan commented Oct 4, 2015

etcd v2.0+ with API v2 cannot watch on a regular key, we must enforce
the 'IsDir' parameter while creating the key for it to be considered
as a directory.

This step is necessary unless 'libkv' includes and supports the new
etcd API v3 which removes the directory/key distinction to allow for
a better abstraction.

Fixes: #392
See: docker/libkv#20

Signed-off-by: Alexandre Beslic [email protected]

etcd v2.0+ with API v2 cannot watch on a regular key, we must enforce
the 'IsDir' parameter while creating the key for it to be considered
as a directory.

This step is necessary unless 'libkv' includes and supports the new
etcd API v3 which removes the directory/key distinction to allow for
a better abstraction.

Fixes: moby#392
See: docker/libkv#20

Signed-off-by: Alexandre Beslic <[email protected]>
@abronan
Copy link
Author

abronan commented Oct 4, 2015

depends on docker/libkv#75

@tomdee
Copy link
Contributor

tomdee commented Oct 7, 2015

@mavenugo What needs to happen for this to get merged? The libkv changes have been merged so do you just need to tell circleCI to run the tests again?

@mavenugo
Copy link
Contributor

mavenugo commented Oct 7, 2015

@tomdee this PR must update libkv Godeps vendor in. thats it :)
@abronan can you please take care of that ?

@mrjana
Copy link
Contributor

mrjana commented Oct 7, 2015

@abronan We don't do any WatchTree in libnetwork any more. Do we still need this?

@mavenugo
Copy link
Contributor

mavenugo commented Oct 7, 2015

@mrjana good point... forgot about it :)

@abronan
Copy link
Author

abronan commented Oct 7, 2015

@tomdee @mavenugo @mrjana Yes this PR is useless now with the latest changes ;)

Will test again though to make sure it works with etcd.

Closing.

@abronan abronan closed this Oct 7, 2015
@mavenugo
Copy link
Contributor

mavenugo commented Oct 7, 2015

@abronan btw, do we need to upgrade the libkv version in libnetwork in order to use the latest etcd client ? Will that get us anything that is not already available in the current version ?

@abronan
Copy link
Author

abronan commented Oct 7, 2015

@mavenugo It will get you nothing if you don't use WatchTree ;) maybe faster on the first Put because it involves less round-trip with the etcd server. Aside from general performance with the client (and I can't backup this claim yet), IDK. We should still update libkv here as we don't know if go-etcd is going to work well with the new etcd versions in the future. Having this one ensures that the client is actually supported.

@abronan abronan deleted the ensure_key_is_watchable branch October 7, 2015 07: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.

4 participants