use the correct spi ZookeeperTransporter when initialize the default … #4513
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4513 +/- ##
============================================
+ Coverage 62.9% 62.93% +0.02%
Complexity 449 449
============================================
Files 770 770
Lines 33006 33008 +2
Branches 5216 5216
============================================
+ Hits 20763 20774 +11
+ Misses 9846 9840 -6
+ Partials 2397 2394 -3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
If the client is just useful for ZookeeperDynamicConfiguration but useless for other DynamicConfiguration implementation, I think we should add a field "client" to ConfigCenterConfig, we can add the client to the map parameters. Look at the comments in ConfigCenterConfig class.
/* If the Config Center product you use have some special parameters that is not covered by this class, you can add it to here.
For example, with XML:
<dubbo:config-center>
<dubbo:parameter key="config.{your key}" value="{your value}" />
</dubbo:config-center>
*/
private Map<String, String> parameters;Can you modify your implementation ?
|
@tswstarplanet yes, i know your purpose. |
…ConfigCenter's zkClient(apache#4511)
|
LGTM |
| @@ -619,6 +619,8 @@ private void useRegistryForConfigIfNecessary() { | |||
| Environment.getInstance().getDynamicConfiguration().orElseGet(() -> { | |||
| ConfigManager configManager = ConfigManager.getInstance(); | |||
| ConfigCenterConfig cc = configManager.getConfigCenter().orElse(new ConfigCenterConfig()); | |||
| cc.setParameters(new HashMap<>()); | |||
| cc.getParameters().put(org.apache.dubbo.remoting.Constants.CLIENT_KEY,rc.getClient()); | |||
There was a problem hiding this comment.
1.check if there exists client in RegistryConfig
2.get the map first, then if it is null we create a new map; put the client into the map if it is absent
Would you please check and modify it again ?
There was a problem hiding this comment.
你好，首先为你们带来的麻烦带来抱歉。
但是我检查过，好像这个地方只有在未声明过config-center的时候才会进入这个代码块，那就意味着这里并不会存在覆盖已有的配置？
Environment.getInstance().getDynamicConfiguration().orElseGet()不存在DynamicConfiguration才会调用orElseGet中的方法，此时应该也是不存在ConfigCenterConfig的。
不知道我的说法对不对？
There was a problem hiding this comment.
@CodingSinger
客气。确实是你说的这样。不过一般这种都是先获取已存在的，不存在才建一个新的。这样即使其他地方改了代码，也不会影响这里。这里就这样吧
|
@CodingSinger My fault, pls check the above comment mentioned by @tswstarplanet |
…ConfigCenter's zkClient(#4511)
What is the purpose of the change
use the correct spi ZookeeperTransporter when initialize the default configcenter's zkClient
fix the issue #4511