New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support Curator Service Discovery and Spring Cloud ZooKeeper #2749
Conversation
Motivation: Related line#2673. It will be nice if we support Curator-X-Discovery and Spring Cloud Zookeeper. Modifications: - Add `InstanceSpec` and `DiscoverySpec` to specify whether use default format or Curator compatible format. - (Breaking) - `NodeValueCodec` is gone. - You now have to use `InstanceSpec` and `DiscoverySpec` to encode and decode. - `ZooKeeperEndpointGroupBuilder.codec(...)` and `ZooKeeperUpatingListenerBuilder.codec(...)` are gone as well. Result: - You can now use Armeria client and server with Curator-X-Discovery.
Codecov Report
@@ Coverage Diff @@
## master #2749 +/- ##
==========================================
Coverage 72.69% 72.69%
- Complexity 11753 11802 +49
==========================================
Files 1032 1041 +9
Lines 45875 46004 +129
Branches 5720 5734 +14
==========================================
+ Hits 33349 33444 +95
- Misses 9594 9614 +20
- Partials 2932 2946 +14 Continue to review full report at Codecov.
|
/cc @jonefeewang (who did the initial implementation of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DiscoverySpec
->ZookeeperDiscoverySpec
InstanceSpec
->ZookeeperRegistrationSpec
Should we remove Zookeeper
for brevity? 🤔
zookeeper/src/main/java/com/linecorp/armeria/client/zookeeper/CuratorXDiscoverySpecBuilder.java
Outdated
Show resolved
Hide resolved
zookeeper/src/main/java/com/linecorp/armeria/client/zookeeper/DiscoverySpec.java
Outdated
Show resolved
Hide resolved
zookeeper/src/main/java/com/linecorp/armeria/client/zookeeper/ZooKeeperEndpointGroup.java
Outdated
Show resolved
Hide resolved
zookeeper/src/main/java/com/linecorp/armeria/server/zookeeper/CuratorXInstanceSpecBuilder.java
Outdated
Show resolved
Hide resolved
zookeeper/src/main/java/com/linecorp/armeria/server/zookeeper/ZooKeeperUpdatingListener.java
Outdated
Show resolved
Hide resolved
Thanks I will change to use
Isn't it okay not to use Zookeeper? |
...c/main/java/com/linecorp/armeria/client/zookeeper/CuratorXZookeeperDiscoverySpecBuilder.java
Outdated
Show resolved
Hide resolved
zookeeper/src/main/java/com/linecorp/armeria/client/zookeeper/ZookeeperDiscoverySpec.java
Outdated
Show resolved
Hide resolved
zookeeper/src/main/java/com/linecorp/armeria/client/zookeeper/ZookeeperDiscoverySpec.java
Outdated
Show resolved
Hide resolved
zookeeper/src/main/java/com/linecorp/armeria/client/zookeeper/ZookeeperDiscoverySpec.java
Outdated
Show resolved
Hide resolved
zookeeper/src/main/java/com/linecorp/armeria/client/zookeeper/ZookeeperDiscoverySpec.java
Outdated
Show resolved
Hide resolved
zookeeper/src/main/java/com/linecorp/armeria/server/zookeeper/ZookeeperRegistrationSpec.java
Outdated
Show resolved
Hide resolved
zookeeper/src/main/java/com/linecorp/armeria/client/zookeeper/CuratorDiscoverySpec.java
Show resolved
Hide resolved
zookeeper/src/main/java/com/linecorp/armeria/client/zookeeper/ZookeeperDiscoverySpec.java
Show resolved
Hide resolved
...per/src/main/java/com/linecorp/armeria/common/zookeeper/AbstractCuratorFrameworkBuilder.java
Outdated
Show resolved
Hide resolved
zookeeper/src/main/java/com/linecorp/armeria/internal/common/zookeeper/ZookeeperPathUtil.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work, @minwoox 👍
...per/src/main/java/com/linecorp/armeria/common/zookeeper/AbstractCuratorFrameworkBuilder.java
Outdated
Show resolved
Hide resolved
...per/src/main/java/com/linecorp/armeria/common/zookeeper/AbstractCuratorFrameworkBuilder.java
Outdated
Show resolved
Hide resolved
zookeeper/src/main/java/com/linecorp/armeria/internal/common/zookeeper/ZookeeperPathUtil.java
Show resolved
Hide resolved
zookeeper/src/test/java/com/linecorp/armeria/client/zookeeper/ZooKeeperEndpointGroupTest.java
Outdated
Show resolved
Hide resolved
zookeeper/src/test/java/com/linecorp/armeria/client/zookeeper/ZooKeeperEndpointGroupTest.java
Outdated
Show resolved
Hide resolved
zookeeper/src/test/java/com/linecorp/armeria/server/zookeeper/ZooKeeperRegistrationTest.java
Outdated
Show resolved
Hide resolved
zookeeper/src/main/java/com/linecorp/armeria/client/zookeeper/ZooKeeperEndpointGroup.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! @minwoox
ServerSets is the last missing piece of ZK service discovery. Way to go, @minwoox 🎉 |
I'm on my way. 🚗 |
Related: line#2673 Motivation: It will be nice if we support Curator Service Discovery and Spring Cloud Zookeeper. Modifications: - Add `ZookeeperRegistrationSpec` and `ZookeeperDiscoverySpec` to specify whether use legacy format or Curator compatible format. - (Breaking) You should specify `ZookeeperRegistrationSpec` When creating `ZookeeperUpdatingListener`. - (Breaking) You should specify `ZookeeperDiscoverySpec` When creating `ZookeeperEndpointGroup`. - (Breaking) - `NodeValueCodec` is gone. - You now have to use `ZookeeperRegistrationSpec` and `ZookeeperDiscoverySpec` to encode and decode. - `ZooKeeperEndpointGroupBuilder.codec(...)` and `ZooKeeperUpatingListenerBuilder.codec(...)` are gone as well. Result: - You can now use Armeria client and server with Curator Service Discovery.
Motivation:
Related #2673.
It will be nice if we support Curator Service Discovery and Spring Cloud Zookeeper.
Modifications:
ZookeeperRegistrationSpec
andZookeeperDiscoverySpec
to specify whether use legacy format or Curator compatible format.ZookeeperRegistrationSpec
When creatingZookeeperUpdatingListener
.ZookeeperDiscoverySpec
When creatingZookeeperEndpointGroup
.NodeValueCodec
is gone.ZookeeperRegistrationSpec
andZookeeperDiscoverySpec
to encode and decode.ZooKeeperEndpointGroupBuilder.codec(...)
andZooKeeperUpatingListenerBuilder.codec(...)
are gone as well.Result:
Todo: