[GitHub] [accumulo] EdColeman commented on a diff in pull request #3177: Validate Property Values

["EdColeman (via GitHub)" <gi...@apache.org>]

EdColeman commented on code in PR #3177:
URL: https://github.com/apache/accumulo/pull/3177#discussion_r1094623202


##########
server/base/src/main/java/org/apache/accumulo/server/util/SystemPropUtil.java:
##########
@@ -74,7 +76,13 @@ private static String validateSystemProperty(String property, final String value
     if (!Property.isValidZooPropertyKey(property)) {
       IllegalArgumentException iae =
           new IllegalArgumentException("Zookeeper property is not mutable: " + property);
-      log.debug("Attempted to set zookeeper property.  It is not mutable", iae);
+      log.error("Encountered error setting zookeeper property", iae);

Review Comment:
   Did you happen to look at the exceptions as printed in the logs?  It seems likely that the method that catches the exception will also printout the stack trace.  If there was additional info to help development, maybe a trace statement?  Otherwise the logs may end up with two identical stack traces without adding additional information.
   
   Also, error seems rather aggressive, services are going to keep running, 
   
   If this code get reworked, it may be worth considering.
   
   (I realize this has been merged, it was changed, committed before I got back to it. Just wanted to mention it if it helps going forward)



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

To unsubscribe, e-mail: notifications-unsubscribe@accumulo.apache.org

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