You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "chickenlj (GitHub)" <gi...@apache.org> on 2019/01/18 09:07:58 UTC
[GitHub] [incubator-dubbo] chickenlj opened pull request #3271: Add
new getConfigFile api to DynamicConfiguration to
## What is the purpose of the change
1. Separate the retrieving of start up configurations from that of governance rules.
2. To better adapt to Apollo's namespace model.
## Brief changelog
* Add getConfigFile() api for DynamicConfiguration and the corresponding implementation in ZookeeperDynamicConfiguraiton and ApolloDynamicConfiguration
* Small optimization on ConfigCenterConfig
## Verifying this change
Follow this checklist to help us incorporate your contribution quickly and easily:
- [x] Make sure there is a [GITHUB_issue](https://github.com/apache/incubator-dubbo/issues) field for the change (usually before you start working on it). Trivial changes like typos do not require a GITHUB issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
- [x] Format the pull request title like `[Dubbo-XXX] Fix UnknownException when host config not exist #XXX`. Each commit in the pull request should have a meaningful subject line and body.
- [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
- [x] Write necessary unit-test to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in [test module](https://github.com/apache/incubator-dubbo/tree/master/dubbo-test).
- [x] Run `mvn clean install -DskipTests=false` & `mvn clean test-compile failsafe:integration-test` to make sure unit-test and integration-test pass.
- [x] If this contribution is large, please follow the [Software Donation Guide](https://github.com/apache/incubator-dubbo/wiki/Software-donation-guide).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3271: [Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
As this new overloaded method has added with timeout, looking at related changes I not seeing any one is using **timeout**. Are we going to use it in recent future ?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
suggest change to:
```java
default String getProperties(String group) {
return getProperties(group, 0L);
}
String getProperties(String group, long timeout);
```
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3271: [Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Thanks for addressing this. Would it be possible to write a UT for the same? If UT was there, it could have caught already. What do you say?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=h1) Report
> Merging [#3271](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-dubbo/commit/e07038b00e057bb3cd6989252534b581039916f9?src=pr&el=desc) will **increase** coverage by `0.07%`.
> The diff coverage is `21.73%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #3271 +/- ##
============================================
+ Coverage 63.86% 63.94% +0.07%
Complexity 75 75
============================================
Files 661 661
Lines 28624 28947 +323
Branches 4826 4970 +144
============================================
+ Hits 18282 18510 +228
- Misses 8132 8203 +71
- Partials 2210 2234 +24
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...figcenter/support/nop/NopDynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L25vcC9Ob3BEeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `71.42% <0%> (-11.91%)` | `0 <0> (ø)` | |
| [...g/apache/dubbo/config/spring/ConfigCenterBean.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvQ29uZmlnQ2VudGVyQmVhbi5qYXZh) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...g/apache/dubbo/config/AbstractInterfaceConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9BYnN0cmFjdEludGVyZmFjZUNvbmZpZy5qYXZh) | `72.78% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...ter/support/apollo/ApolloDynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci1hcG9sbG8vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L2Fwb2xsby9BcG9sbG9EeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...va/org/apache/dubbo/config/ConfigCenterConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9Db25maWdDZW50ZXJDb25maWcuamF2YQ==) | `14.06% <100%> (+0.62%)` | `0 <0> (ø)` | :arrow_down: |
| [...pache/dubbo/configcenter/DynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9EeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `90.9% <100%> (-9.1%)` | `0 <0> (ø)` | |
| [...pport/zookeeper/ZookeeperDynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci16b29rZWVwZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L3pvb2tlZXBlci9ab29rZWVwZXJEeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `68.88% <100%> (-3.84%)` | `0 <0> (ø)` | |
| [...ava/org/apache/dubbo/rpc/filter/TimeoutFilter.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9maWx0ZXIvVGltZW91dEZpbHRlci5qYXZh) | `42.85% <0%> (-40.48%)` | `0% <0%> (ø)` | |
| [...che/dubbo/remoting/transport/mina/MinaChannel.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbWluYS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvZHViYm8vcmVtb3RpbmcvdHJhbnNwb3J0L21pbmEvTWluYUNoYW5uZWwuamF2YQ==) | `43.42% <0%> (-10.53%)` | `0% <0%> (ø)` | |
| ... and [24 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=footer). Last update [e07038b...615c198](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3271: [Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
should we use Constants.DEFAULT_DUBBO_PROPERTIES here?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3271: [Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Just an thought, below way would be more clear and easy to understand
```
public String getConfigFile(String key, String group, long timeout) throws IllegalStateException {
if(StringUtils.isEmpty(group)) {
return dubboConfigFile.getContent();
}
if (group.equals(url.getParameter(Constants.APPLICATION_KEY))) {
return ConfigService.getConfigFile(APOLLO_APPLICATION_KEY, ConfigFileFormat.Properties).getContent();
}
ConfigFile configFile = ConfigService.getConfigFile(group, ConfigFileFormat.Properties);
if (configFile == null) {
throw new IllegalStateException("There is no namespace named " + group + " in Apollo.");
}
return configFile.getContent();
}
```
What do you say?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3271: [Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
is this was public method, will there be any issue with backward compatibility?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3271: [Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
should check whether given **application** is valid or not like we are doing in getApplication method?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=h1) Report
> Merging [#3271](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=desc) into [2.7.0-release](https://codecov.io/gh/apache/incubator-dubbo/commit/1c78148ccfdbc5e24773876a69ff29a45b825153?src=pr&el=desc) will **decrease** coverage by `0.09%`.
> The diff coverage is `17.24%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## 2.7.0-release #3271 +/- ##
=================================================
- Coverage 63.7% 63.6% -0.1%
Complexity 75 75
=================================================
Files 658 658
Lines 28545 28560 +15
Branches 4825 4829 +4
=================================================
- Hits 18185 18166 -19
- Misses 8090 8118 +28
- Partials 2270 2276 +6
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...g/apache/dubbo/config/AbstractInterfaceConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9BYnN0cmFjdEludGVyZmFjZUNvbmZpZy5qYXZh) | `72.18% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...figcenter/support/nop/NopDynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L25vcC9Ob3BEeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `71.42% <0%> (-11.91%)` | `0 <0> (ø)` | |
| [...ter/support/apollo/ApolloDynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci1hcG9sbG8vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L2Fwb2xsby9BcG9sbG9EeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...g/apache/dubbo/config/spring/ConfigCenterBean.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvQ29uZmlnQ2VudGVyQmVhbi5qYXZh) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...pache/dubbo/configcenter/DynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9EeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `90.9% <100%> (-9.1%)` | `0 <0> (ø)` | |
| [...pport/zookeeper/ZookeeperDynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci16b29rZWVwZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L3pvb2tlZXBlci9ab29rZWVwZXJEeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `68.88% <100%> (-3.84%)` | `0 <0> (ø)` | |
| [...va/org/apache/dubbo/config/ConfigCenterConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9Db25maWdDZW50ZXJDb25maWcuamF2YQ==) | `12.5% <30%> (-0.94%)` | `0 <0> (ø)` | |
| [.../remoting/transport/netty4/NettyClientHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHk0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvbmV0dHk0L05ldHR5Q2xpZW50SGFuZGxlci5qYXZh) | `75% <0%> (-11.12%)` | `0% <0%> (ø)` | |
| [...in/java/org/apache/dubbo/common/utils/JVMUtil.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9jb21tb24vdXRpbHMvSlZNVXRpbC5qYXZh) | `73.58% <0%> (-7.55%)` | `0% <0%> (ø)` | |
| ... and [6 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=footer). Last update [1c78148...c72371b](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on issue #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
> @chickenlj do let me know if you planning to address the given few feedback in this PR. Otherthan given few minor comment (if looks valid to you). This PR looks good to me.
@khanimteyaz Sure, I am working on it considering @beiwei30's and your feedbacks.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
I will see how to avoid that.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
> suggest change to:
>
> default String getProperties(String group) {
> return getProperties(group, 0L);
> }
>
> String getProperties(String group, long timeout);
1. I have changed to `String getConfigs(String key, String group, long timeout);`, in case of supporting `yaml` format in the future, `getProperties` would be confusing.
2. I think it would be more flexible to have the `key` parameter kept.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
done, please review.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
Do you mean to use a fixed key `dubbo.properties`? But we have given users the to provide a customized key by `ConfigCenterConfig.configFile`: https://github.com/apache/incubator-dubbo/blob/9f3ac611eff6f3918fd9e367a90f76d0ee9cc609/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ConfigCenterConfig.java#L47
We should remove `ConfigCenterConfig.configFile` at the same time if decide to remove key from this interface?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
Yes, I think so.
Except for `Zookeeper` and `Apollo` we already supported. We may integrate with more implementations, more likely some of them will have a timeout parameter.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=h1) Report
> Merging [#3271](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=desc) into [2.7.0-release](https://codecov.io/gh/apache/incubator-dubbo/commit/2134b891725c0545dc3bceec56ceaf7b25e28526?src=pr&el=desc) will **decrease** coverage by `0.01%`.
> The diff coverage is `17.24%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## 2.7.0-release #3271 +/- ##
===================================================
- Coverage 63.65% 63.63% -0.02%
Complexity 75 75
===================================================
Files 658 658
Lines 28545 28560 +15
Branches 4825 4829 +4
===================================================
+ Hits 18171 18175 +4
- Misses 8099 8109 +10
- Partials 2275 2276 +1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...figcenter/support/nop/NopDynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L25vcC9Ob3BEeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `71.42% <0%> (-11.91%)` | `0 <0> (ø)` | |
| [...g/apache/dubbo/config/spring/ConfigCenterBean.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvQ29uZmlnQ2VudGVyQmVhbi5qYXZh) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...g/apache/dubbo/config/AbstractInterfaceConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9BYnN0cmFjdEludGVyZmFjZUNvbmZpZy5qYXZh) | `72.18% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...ter/support/apollo/ApolloDynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci1hcG9sbG8vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L2Fwb2xsby9BcG9sbG9EeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...pache/dubbo/configcenter/DynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9EeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `90.9% <100%> (-9.1%)` | `0 <0> (ø)` | |
| [...pport/zookeeper/ZookeeperDynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci16b29rZWVwZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L3pvb2tlZXBlci9ab29rZWVwZXJEeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `68.88% <100%> (-3.84%)` | `0 <0> (ø)` | |
| [...va/org/apache/dubbo/config/ConfigCenterConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9Db25maWdDZW50ZXJDb25maWcuamF2YQ==) | `12.5% <30%> (-0.94%)` | `0 <0> (ø)` | |
| [...bbo/registry/support/ProviderConsumerRegTable.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tcmVnaXN0cnkvZHViYm8tcmVnaXN0cnktYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZWdpc3RyeS9zdXBwb3J0L1Byb3ZpZGVyQ29uc3VtZXJSZWdUYWJsZS5qYXZh) | `80.48% <0%> (-4.88%)` | `0% <0%> (ø)` | |
| [...onfig/spring/extension/SpringExtensionFactory.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvZXh0ZW5zaW9uL1NwcmluZ0V4dGVuc2lvbkZhY3RvcnkuamF2YQ==) | `80.48% <0%> (-4.88%)` | `0% <0%> (ø)` | |
| ... and [8 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=footer). Last update [2134b89...5db4947](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
This is a crucial bug.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] beiwei30 commented on pull request #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "beiwei30 (GitHub)" <gi...@apache.org>.
We should not introduce dependency from ConfigCenterConfig to ApplicationConfig.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on issue #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
@chickenlj do let me know if you planning to address the given few feedback in this PR. Otherthan given few minor comment (if looks valid to you). This PR looks good to me.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=h1) Report
> Merging [#3271](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=desc) into [2.7.0-release](https://codecov.io/gh/apache/incubator-dubbo/commit/de4f91b68940874f29df5c6df73c1adc456f0961?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `17.24%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## 2.7.0-release #3271 +/- ##
==================================================
- Coverage 63.63% 63.6% -0.03%
Complexity 75 75
==================================================
Files 658 658
Lines 28545 28560 +15
Branches 4825 4829 +4
==================================================
+ Hits 18164 18166 +2
- Misses 8104 8118 +14
+ Partials 2277 2276 -1
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...figcenter/support/nop/NopDynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L25vcC9Ob3BEeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `71.42% <0%> (-11.91%)` | `0 <0> (ø)` | |
| [...g/apache/dubbo/config/spring/ConfigCenterBean.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvQ29uZmlnQ2VudGVyQmVhbi5qYXZh) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...g/apache/dubbo/config/AbstractInterfaceConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9BYnN0cmFjdEludGVyZmFjZUNvbmZpZy5qYXZh) | `72.18% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...ter/support/apollo/ApolloDynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci1hcG9sbG8vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L2Fwb2xsby9BcG9sbG9EeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...pache/dubbo/configcenter/DynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9EeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `90.9% <100%> (-9.1%)` | `0 <0> (ø)` | |
| [...pport/zookeeper/ZookeeperDynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci16b29rZWVwZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L3pvb2tlZXBlci9ab29rZWVwZXJEeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `68.88% <100%> (-3.84%)` | `0 <0> (ø)` | |
| [...va/org/apache/dubbo/config/ConfigCenterConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9Db25maWdDZW50ZXJDb25maWcuamF2YQ==) | `12.5% <30%> (-0.94%)` | `0 <0> (ø)` | |
| [...ache/dubbo/remoting/p2p/support/AbstractGroup.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctcDJwL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9wMnAvc3VwcG9ydC9BYnN0cmFjdEdyb3VwLmphdmE=) | `45.45% <0%> (-11.37%)` | `0% <0%> (ø)` | |
| [.../remoting/transport/netty4/NettyClientHandler.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctbmV0dHk0L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy90cmFuc3BvcnQvbmV0dHk0L05ldHR5Q2xpZW50SGFuZGxlci5qYXZh) | `75% <0%> (-11.12%)` | `0% <0%> (ø)` | |
| ... and [9 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=footer). Last update [de4f91b...c72371b](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
I am afraid it will break the backward compatibility in theory.
But I tend to fix the inappropriate design directly as there're still few users depends on it.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] codecov-io commented on issue #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "codecov-io (GitHub)" <gi...@apache.org>.
# [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=h1) Report
> Merging [#3271](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=desc) into [2.7.0-release](https://codecov.io/gh/apache/incubator-dubbo/commit/de4f91b68940874f29df5c6df73c1adc456f0961?src=pr&el=desc) will **decrease** coverage by `0.03%`.
> The diff coverage is `7.69%`.
[](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## 2.7.0-release #3271 +/- ##
===================================================
- Coverage 63.63% 63.59% -0.04%
Complexity 75 75
===================================================
Files 658 658
Lines 28545 28560 +15
Branches 4825 4829 +4
===================================================
- Hits 18164 18163 -1
- Misses 8104 8123 +19
+ Partials 2277 2274 -3
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=tree) | Coverage Δ | Complexity Δ | |
|---|---|---|---|
| [...va/org/apache/dubbo/config/ConfigCenterConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9Db25maWdDZW50ZXJDb25maWcuamF2YQ==) | `12.5% <0%> (-0.94%)` | `0 <0> (ø)` | |
| [...figcenter/support/nop/NopDynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L25vcC9Ob3BEeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `71.42% <0%> (-11.91%)` | `0 <0> (ø)` | |
| [...g/apache/dubbo/config/spring/ConfigCenterBean.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1zcHJpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9zcHJpbmcvQ29uZmlnQ2VudGVyQmVhbi5qYXZh) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...g/apache/dubbo/config/AbstractInterfaceConfig.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnL2R1YmJvLWNvbmZpZy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZy9BYnN0cmFjdEludGVyZmFjZUNvbmZpZy5qYXZh) | `72.18% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...ter/support/apollo/ApolloDynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci1hcG9sbG8vc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L2Fwb2xsby9BcG9sbG9EeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `0% <0%> (ø)` | `0 <0> (ø)` | :arrow_down: |
| [...pache/dubbo/configcenter/DynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9EeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `90.9% <100%> (-9.1%)` | `0 <0> (ø)` | |
| [...pport/zookeeper/ZookeeperDynamicConfiguration.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tY29uZmlnY2VudGVyL2R1YmJvLWNvbmZpZ2NlbnRlci16b29rZWVwZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL2NvbmZpZ2NlbnRlci9zdXBwb3J0L3pvb2tlZXBlci9ab29rZWVwZXJEeW5hbWljQ29uZmlndXJhdGlvbi5qYXZh) | `68.88% <100%> (-3.84%)` | `0 <0> (ø)` | |
| [...ng/exchange/support/header/HeartbeatTimerTask.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tcmVtb3RpbmcvZHViYm8tcmVtb3RpbmctYXBpL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9kdWJiby9yZW1vdGluZy9leGNoYW5nZS9zdXBwb3J0L2hlYWRlci9IZWFydGJlYXRUaW1lclRhc2suamF2YQ==) | `73.68% <0%> (-5.27%)` | `0% <0%> (ø)` | |
| [...org/apache/dubbo/rpc/protocol/AbstractInvoker.java](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree#diff-ZHViYm8tcnBjL2R1YmJvLXJwYy1hcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2R1YmJvL3JwYy9wcm90b2NvbC9BYnN0cmFjdEludm9rZXIuamF2YQ==) | `62.9% <0%> (-3.23%)` | `0% <0%> (ø)` | |
| ... and [9 more](https://codecov.io/gh/apache/incubator-dubbo/pull/3271/diff?src=pr&el=tree-more) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=footer). Last update [de4f91b...2562b29](https://codecov.io/gh/apache/incubator-dubbo/pull/3271?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
Do you mean to use a fixed key `dubbo.properties`? But we have given users the chance to provide a customized key by `ConfigCenterConfig.configFile`: https://github.com/apache/incubator-dubbo/blob/9f3ac611eff6f3918fd9e367a90f76d0ee9cc609/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ConfigCenterConfig.java#L47
We should remove `ConfigCenterConfig.configFile` at the same time if decide to remove key from this interface?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3271: [Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
we need to add return statement here?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
I am not sure about this, will confirm later.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on issue #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
@beiwei30 @khanimteyaz Please help review the latest commits
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
done.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
done
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] khanimteyaz commented on pull request
#3271: [Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "khanimteyaz (GitHub)" <gi...@apache.org>.
Is Constants.DUBBO can be used here ?
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org
[GitHub] [incubator-dubbo] chickenlj commented on pull request #3271:
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to
Apollo's namespace storage model.
Posted by "chickenlj (GitHub)" <gi...@apache.org>.
done.
[ Full content available at: https://github.com/apache/incubator-dubbo/pull/3271 ]
This message was relayed via gitbox.apache.org for notifications@dubbo.apache.org