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
SCB-315 Config Center module can't get config by DynamicPropertyFacto… #546
Conversation
…ry.getInstance().getStringProperty, so has to use ConcurrentCompositeConfiguration
"addressResolver." + tag + ".rotateServers", | ||
"addressResolver.rotateServers")); | ||
return addressResolverOptions; | ||
} | ||
|
||
private static List<String> getStringListProperty(List<String> defaultValue, String... keys) { | ||
private static List<String> getStringListProperty(ConcurrentCompositeConfiguration configSource, | ||
List<String> defaultValue, String... keys) { | ||
String property = null; |
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.
if configSource is null, then assign from DynamicPropertyFactory.getBackingConfigurationSource()
then return Arrays.asList(configSource.getStringArray(key))
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
@@ -95,33 +127,55 @@ public static AddressResolverOptions getAddressResover(String tag) { | |||
} | |||
} | |||
|
|||
private static int getIntProperty(int defaultValue, String... keys) { | |||
private static int getIntProperty(ConcurrentCompositeConfiguration configSource, int defaultValue, String... keys) { |
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.
we can make the logics simpler
…ry.getInstance().getStringProperty, so has to use ConcurrentCompositeConfiguration
if (property > 0) { | ||
break; | ||
Integer val = configSource.getInteger(key, null); | ||
if (val != null && val > 0) { |
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.
why a value <=0 is invalid?
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.
all integer parameters must large than zero
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.
1.if there is some property have no this limit
and developers see there is a getIntProperty method, invoke it directly......
2.users make a wrong configuration, here ignore the wrong value, and use default value
but this behavior is silence
nobody know what happened.
…ry.getInstance().getStringProperty, so has to use ConcurrentCompositeConfiguration
…ry.getInstance().getStringProperty, so has to use ConcurrentCompositeConfiguration
Follow this checklist to help us incorporate your contribution quickly and easily:
[SCB-XXX] Fixes bug in ApproximateQuantiles
, where you replaceSCB-XXX
with the appropriate JIRA issue.mvn clean install
to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.