search

jurmous / etcd4j

Java / Netty client for etcd, the highly-available key value store for shared configuration and service discovery.
Apache License 2.0
267 stars 83 forks source link

Added API to get leader stats #66

Closed johneke closed 8 years ago

johneke commented 8 years ago

Please consider my PR for adding leader stats API to etcd4j.

One thing, the tests are a bit weird. They always run against a local etcd instance, so all I could do was add a test that if there is one node running, we get back information saying that its the leader.

If you point the tests to a multi node cluster it also passes, but its hard to write that up.

One thing that can be done is if we mock etcd, then we can simulate responses to different requests.

Either way, I look forward to your feedback! :)

lburgazzoli commented 8 years ago

Thank you for the PR

johneke commented 8 years ago

Great! Always glad to contribute :)

What do we need to do to get this into the maven repositories?

lburgazzoli commented 8 years ago

I need to complete a task then I can ask @jurmous to release it

johneke commented 8 years ago

:+1:

johneke commented 8 years ago

Any word on getting this uploaded to the maven repos? :)

lburgazzoli commented 8 years ago

Hi @johneke, give me a few more days :-)

johneke commented 8 years ago

Hello!! Any update on this yet? Is there any way I can help out?

lburgazzoli commented 8 years ago

I've merged my latest code, can you check if you have any regression ? @jurmous would it be possible to add have grants to publish on sonatype so I can free you for doing it ?

jurmous commented 8 years ago

@lburgazzoli I wanted to do a quick SNAPSHOT release but there is a test failure on InOrderKeys.

I will look later into giving you publishing rights, it is a complex thing. For now I can do it pretty quickly if needed. Just give me a sign.

lburgazzoli commented 8 years ago

@jurmous which version of etcd you have ? in > 2.2 the index format has changed and it is padded with zeros

jurmous commented 8 years ago

@lburgazzoli It was 2.1.1. I adapted the test so it is compatible with both 2.1 and 2.2. I have pushed 2.9.0-SNAPSHOT, if OK I will publish the final.

lburgazzoli commented 8 years ago

fine with me

johneke commented 8 years ago

Sorry to bug you again @jurmous, is 2.9.0 up yet? I'm a bit blocked by this at the moment :(

jurmous commented 8 years ago

It should be on maven central. Although maven search is sometimes a bit delayed. Enjoy the new release! :smile:

johneke commented 8 years ago

Great!!! Thank you @jurmous!