[GitHub] [helix] pkuwm commented on a change in pull request #1237: Fix empty host list for instance stoppable response

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

pkuwm commented on a change in pull request #1237:
URL: https://github.com/apache/helix/pull/1237#discussion_r476144914



##########
File path: helix-rest/src/main/java/org/apache/helix/rest/server/service/InstanceServiceImpl.java
##########
@@ -213,7 +219,12 @@ public StoppableCheck getInstanceStoppableCheck(String clusterId, String instanc
           instancesForNextCheck.add(instance);
         }
       } catch (InterruptedException | ExecutionException e) {
-        LOG.error("Failed to get StoppableChecks in parallel. Instance: {}", instance, e);
+        LOG.warn("Failed to get StoppableChecks in parallel. Instance: {}. Caused by {}", instance,

Review comment:
       The code change is not necessary and kept as it is.
   
   Regarding warn or error, I look at it from server perspective: if the exception is not a server exception that affects running of the REST server, I would say it is an error.
   But obviously this is not an exception that stops REST server running. It is just a normal running of the API check. It should not be logged as an error. A warning should already be enough to help debug.
     




----------------------------------------------------------------
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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org