-
Notifications
You must be signed in to change notification settings - Fork 26.5k
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
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to Apollo's namespace storage model. #3271
[Dubbo-3266] Change DynamicConfiguration definition to better adapt to Apollo's namespace storage model. #3271
Conversation
1. separate the retrieving of start up configurations from that of governance rules. 2. better adapt to Apollo's namespace model.
Codecov Report
@@ Coverage Diff @@
## master #3271 +/- ##
============================================
+ Coverage 62.69% 62.73% +0.03%
- Complexity 544 545 +1
============================================
Files 762 762
Lines 32758 32752 -6
Branches 5165 5162 -3
============================================
+ Hits 20538 20546 +8
+ Misses 9861 9848 -13
+ Partials 2359 2358 -1
Continue to review full report at Codecov.
|
dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ConfigCenterConfig.java
Outdated
Show resolved
Hide resolved
dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ConfigCenterConfig.java
Outdated
Show resolved
Hide resolved
dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ConfigCenterConfig.java
Outdated
Show resolved
Hide resolved
...o/src/main/java/org/apache/dubbo/configcenter/support/apollo/ApolloDynamicConfiguration.java
Outdated
Show resolved
Hide resolved
dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ConfigCenterConfig.java
Show resolved
Hide resolved
...dubbo-configcenter-api/src/main/java/org/apache/dubbo/configcenter/DynamicConfiguration.java
Outdated
Show resolved
Hide resolved
de4f91b
to
1c78148
Compare
...o/src/main/java/org/apache/dubbo/configcenter/support/apollo/ApolloDynamicConfiguration.java
Outdated
Show resolved
Hide resolved
@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. |
2134b89
to
d2b62d3
Compare
...dubbo-configcenter-api/src/main/java/org/apache/dubbo/configcenter/DynamicConfiguration.java
Outdated
Show resolved
Hide resolved
dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ConfigCenterConfig.java
Outdated
Show resolved
Hide resolved
@khanimteyaz Sure, I am working on it considering @beiwei30's and your feedbacks. |
# Conflicts: # dubbo-configcenter/dubbo-configcenter-zookeeper/src/test/java/org/apache/dubbo/configcenter/support/zookeeper/ZookeeperDynamicConfigurationTest.java
@beiwei30 @khanimteyaz Please help review the latest commits |
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.
LGTM.
# Conflicts: # dubbo-configcenter/dubbo-configcenter-zookeeper/src/main/java/org/apache/dubbo/configcenter/support/zookeeper/ZookeeperDynamicConfiguration.java
...onfig/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/ConfigCenterBean.java
Show resolved
Hide resolved
...dubbo-configcenter-api/src/main/java/org/apache/dubbo/configcenter/DynamicConfiguration.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.
LGTM
# Conflicts: # dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ConfigCenterConfig.java # dubbo-configcenter/dubbo-configcenter-apollo/src/main/java/org/apache/dubbo/configcenter/support/apollo/ApolloDynamicConfiguration.java # dubbo-configcenter/dubbo-configcenter-nacos/src/main/java/org/apache/dubbo/configcenter/support/nacos/NacosDynamicConfiguration.java
What is the purpose of the change
#3266
Brief changelog
Verifying this change
Follow this checklist to help us incorporate your contribution quickly and easily:
[Dubbo-XXX] Fix UnknownException when host config not exist #XXX
. Each commit in the pull request should have a meaningful subject line and body.mvn clean install -DskipTests=false
&mvn clean test-compile failsafe:integration-test
to make sure unit-test and integration-test pass.