[SCB-662]Using cse/servicecomb duplicate key will get list of values #767
Conversation
| source.addProperty(cseKey, source.getProperty(key)); | ||
| } else { | ||
| LOGGER | ||
| .warn( |
There was a problem hiding this comment.
Not sure if the warning message is enough for this time, if the configuration is wrong, java-chassis may need to stop working instead of sending out the warning message to let the admin know about it.
BTW, we may need a default override policy here.
There was a problem hiding this comment.
As shown in test case, this can work. Just when merge, it will not take the cse-config-order(which is implemented by file configuration source). We can not cover all orders for Configuration sources like enviroment, properties, dynamic config and so on, so give a warning message.
There was a problem hiding this comment.
"stop working" can not be applied because we may configure some default values using cse or servicecomb, if doing this users code have to change to work properly. We can't do this now.
| @@ -171,7 +171,14 @@ private static void duplicateServiceCombConfigToCse(AbstractConfiguration source | |||
| } | |||
|
|
|||
| String cseKey = CONFIG_CSE_PREFIX + key.substring(key.indexOf(".") + 1); | |||
| source.addProperty(cseKey, source.getProperty(key)); | |||
| if (!source.containsKey(cseKey)) { | |||
| source.addProperty(cseKey, source.getProperty(key)); | |||
There was a problem hiding this comment.
source.setProperty is better?
There was a problem hiding this comment.
source.setProperty will clear and add. Here we checked existence and logged a warning message.
| @@ -36,6 +36,8 @@ | |||
| import org.springframework.http.ResponseEntity; | |||
| import org.springframework.web.client.RestTemplate; | |||
|
|
|||
| import com.netflix.config.DynamicPropertyFactory; | |||
There was a problem hiding this comment.
This import can be removed.
Follow this checklist to help us incorporate your contribution quickly and easily:
[SCB-XXX] Fixes bug in ApproximateQuantiles, where you replaceSCB-XXXwith the appropriate JIRA issue.mvn clean installto make sure basic checks pass. A more thorough check will be performed on your pull request automatically.