[GitHub] [flume] avinashk07 commented on a change in pull request #326: ref #FLUME-3361 Added support for environment variable in ZooKeeper b…

[GitBox <gi...@apache.org>]

avinashk07 commented on a change in pull request #326:
URL: https://github.com/apache/flume/pull/326#discussion_r420339655



##########
File path: flume-ng-node/src/main/java/org/apache/flume/node/AbstractZooKeeperConfigurationProvider.java
##########
@@ -90,14 +97,24 @@ protected CuratorFramework createClient() {
 
   protected FlumeConfiguration configFromBytes(byte[] configData)
       throws IOException {
-    Map<String, String> configMap;
-    if (configData == null || configData.length == 0) {
-      configMap = Collections.emptyMap();
-    } else {
-      String fileContent = new String(configData, Charsets.UTF_8);
-      Properties properties = new Properties();
-      properties.load(new StringReader(fileContent));
-      configMap = toMap(properties);
+    Map<String, String> configMap = Collections.emptyMap(); 
+    if (configData != null && configData.length > 0) {

Review comment:
       Thanks for taking it up Tristan!
   I have initialized configMap to emptyMap (Map<String, String> configMap = Collections.emptyMap();) so that an emptyMap (and not null) is returned even in case of a handled exception.
   Tried to explain in detail below. Please let me know if I'm missing anything.
   
   BEFORE:
   -------
   
   Map m = null
   
   if config does not exists
   	m = empty Map
   	return m
   else
   	m = Map with config value //IO Exception is thrown
   	
   return m
   
   
   NOW:
   ----
   
   Map m = empty Map (so that I don't end up returning null in case of exceptions below)
   
   if config exists
   	m = Map with config value
   	catch new possible exceptions other than IO
   	
   return m //so returning empty map like before (retaining the behavior) 
                 //if Config does not exist or if an exception occurs that is handled




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org