narendly commented on a change in pull request #1033:
URL: https://github.com/apache/helix/pull/1033#discussion_r436040023



##########
File path: 
helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java
##########
@@ -639,14 +639,18 @@ private void subscribeForChanges(NotificationContext.Type 
callbackType, String p
             }
           }
         } catch (ZkNoNodeException | HelixMetaDataAccessException e) {
-          logger.warn(
-              "fail to subscribe child/data change. path: " + path + ", 
listener: " + _listener, e);
+          logger.warn("Failed to subscribe child/data change. path: {}, 
listener: {}", path,
+              _listener);
+          // TODO: avoid calling getChildren for path that does not exist
+          if (_changeType != CUSTOMIZED_STATE_ROOT) {
+            logger.warn("", e);
+          }

Review comment:
       ```
   if (_changeType != CUSTOMIZED_STATE_ROOT) {
     LOG.warn("Failed to subscribe .... Instance doesn't support Customized 
State!, path, _listerner);
   } else {
     LOG.warn("Failed to.... ");
   }
   ```
   How about this? this way you don't print all the exceptions stack trace, and 
it doesn't look like it's duplicate code, and the message is clear.




----------------------------------------------------------------
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to