-
Notifications
You must be signed in to change notification settings - Fork 26.4k
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 dubbo.registry.parameters.item3=value3 configuration properties #8308
Conversation
Codecov Report
@@ Coverage Diff @@
## 3.0 #8308 +/- ##
============================================
- Coverage 64.05% 63.92% -0.13%
+ Complexity 313 311 -2
============================================
Files 1077 1078 +1
Lines 45518 45741 +223
Branches 6839 6864 +25
============================================
+ Hits 29155 29241 +86
- Misses 13138 13235 +97
- Partials 3225 3265 +40
Continue to review full report at Codecov.
|
@@ -559,6 +560,13 @@ public void refresh() { | |||
postProcessRefresh(); | |||
} | |||
|
|||
private void invokeSetParameters(Map<String, String> values) { | |||
Map<String, String> map = invokeGetParameters(getClass(), this); |
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.
It is better to check argument Map<String, String> values
, If it is null or empty, just return
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.
ok, i will modify it.
invokeSetParameters(convert(StringUtils.parseParameters(value), "")); | ||
} else { | ||
// in this case, maybe parameters.item3=value3. | ||
invokeSetParameters(ConfigurationUtils.getSubProperties(subProperties, PARAMETERS)); |
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.
Should we do convert()
parameters or not?
For compatibility, convert is needed, but additional parameters will be added, which leads to more url parameters, which is not good. This compatibility does not know what the specific problem is, it is best to deal with it when the parameters are read.
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.
Some use a.b, some a-b, we will normalize parameter keys later.
What is the purpose of the change
Support the following format configuration properties. such as:
dubbo.registry.parameters.item3=value3
see #8303
Brief changelog
Verifying this change
Checklist