[GitHub] liubao68 commented on a change in pull request #569: SCB-210 remove extra useless config item
liubao68 commented on a change in pull request #569: SCB-210 remove extra useless config item URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/569#discussion_r172183864 ## File path: foundations/foundation-config/src/main/java/org/apache/servicecomb/config/ConfigUtil.java ## @@ -150,7 +151,7 @@ private static void duplicateServiceCombConfigToCse(ConcurrentCompositeConfigura } public static DynamicWatchedConfiguration createConfigFromConfigCenter(Configuration localConfiguration) { -if (localConfiguration.getProperty(configCenterUrlKey) == null) { +if ((localConfiguration.getProperty(configCenterUrlKey) == null) && (localConfiguration.getProperties((configApolloUrlKey)) == null )) { Review comment: This code should be implementation independent. Can you find a better solution if possible? This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services
[GitHub] liubao68 commented on a change in pull request #569: SCB-210 remove extra useless config item
liubao68 commented on a change in pull request #569: SCB-210 remove extra useless config item URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/569#discussion_r172719593 ## File path: foundations/foundation-config/src/main/java/org/apache/servicecomb/config/spi/ConfigCenterConfigurationSource.java ## @@ -22,5 +22,6 @@ import com.netflix.config.WatchedConfigurationSource; public interface ConfigCenterConfigurationSource extends WatchedConfigurationSource { + boolean isValidSource(Configuration localConfiguration); Review comment: For compatibility, this method should give default implementation. But it's fine now , this is not known implementations. This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services