Support sofa registry #793
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #793 +/- ##
============================================
+ Coverage 38.28% 38.3% +0.02%
Complexity 1013 1013
============================================
Files 220 220
Lines 8504 8507 +3
Branches 1023 1024 +1
============================================
+ Hits 3256 3259 +3
- Misses 4861 4863 +2
+ Partials 387 385 -2
Continue to review full report at Codecov.
|
|
Fix cr |
|
Well done, we will merge it between 0.5.0 and 0.6.0. |
…nto support_sofa_registry
| validAddress(address); | ||
| String clusterName = registryProps.getProperty(PRO_CLUSTER_KEY); | ||
| PublisherRegistration publisherRegistration; | ||
| publisherRegistration = new PublisherRegistration(clusterName); |
|
|
||
| private static Properties getNamingProperties() { | ||
| Properties properties = new Properties(); | ||
| if (null != System.getProperty(PRO_SERVER_ADDR_KEY)) { |
There was a problem hiding this comment.
I think Missing prefix like -Dregistry.serverAddr but not -DserverAddr
| public void close() throws Exception { | ||
| } | ||
|
|
||
| private void validAddress(InetSocketAddress address) { |
…nto support_sofa_registry
| newAddressList.addAll(tranformData); | ||
| CLUSTER_ADDRESS_MAP.put(clusterName, newAddressList); | ||
| } | ||
| respondRegistries.countDown(); |
There was a problem hiding this comment.
I think respondRegistries should be local variable，how to support multi-cluster?
| <name>seata-discovery-sofa ${project.version}</name> | ||
|
|
||
| <properties> | ||
| <sofa.registry.version>5.2.0</sofa.registry.version> |
| region = "DEFAULT_ZONE" | ||
| datacenter = "DefaultDataCenter" | ||
| cluster = "default" | ||
| group = "SEATA" |
There was a problem hiding this comment.
yes， It is a label for sofa registry server， it will store this service in group "SEATA"，
There was a problem hiding this comment.
yes， It is a label for sofa registry server， it will store this service in group "SEATA"，
Ok， I will change it
Ⅰ. Describe what this PR did
Support SOFARegistry as a kind of seata-discover, not seata-config
Ⅱ. Does this pull request fix one issue?
fixes #792
Ⅲ. Why don't you add test cases (unit test/integration test)?
I add a test case to verify it
Ⅳ. Describe how to verify it
the test case will success
Ⅴ. Special notes for reviews
I have read the implements of the other discover extensions and study from them, but there may have some errors in my code. please review it, thanks.