belugabehr commented on a change in pull request #1716: HADOOP-16691: Unify
Logging in UserGroupInformation
URL: https://github.com/apache/hadoop/pull/1716#discussion_r345971093
##########
File path:
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/security/UserGroupInformation.java
##########
@@ -237,15 +227,13 @@ public boolean commit() throws LoginException {
} catch (Exception e) {
throw (LoginException)(new
LoginException(e.toString()).initCause(e));
}
- if (LOG.isDebugEnabled()) {
- LOG.debug("User entry: \"" + userEntry.toString() + "\"" );
- }
+ LOG.debug("User entry: \"{}\"", userEntry);
subject.getPrincipals().add(userEntry);
return true;
}
- LOG.error("Can't find user in " + subject);
- throw new LoginException("Can't find user name");
+ LOG.error("Failed to find user name in " + subject);
Review comment:
Using a parameterized message is typically reserved for DEBUG and TRACE
messages. It is however, also helpful for readability in my opinion. However,
it's not that important here because ERROR level messages will always be
printed, so any kind of optimization is futile.
However, you do bring to my attention that this is an example of the
throw-and-log situation. I changed to remove the logging altogether.
----------------------------------------------------------------
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]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
