[GitHub] [kafka] ijuma commented on a diff in pull request #12587: MINOR: Use named class for ExpiringCredential to improve `principalLogText()` output

[GitBox]

ijuma commented on code in PR #12587:
URL: https://github.com/apache/kafka/pull/12587#discussion_r962039832


##########
clients/src/main/java/org/apache/kafka/common/security/oauthbearer/internals/expiring/ExpiringCredentialRefreshingLogin.java:
##########
@@ -421,17 +419,17 @@ private void reLogin() throws LoginException, 
ExitRefresherThreadDueToIllegalSta
                     throw new 
ExitRefresherThreadDueToIllegalStateException(String.format(
                             "Subject's private credentials still contains the 
previous, soon-to-expire instance of %s even though login() followed by 
logout() was invoked; exiting refresh thread",
                             expiringCredential.getClass().getName()));
-                principalName = expiringCredential.principalName();
                 if (log.isDebugEnabled())
                     log.debug("[Principal={}]: It is an expiring credential 
after re-login as expected",
                             principalLogText());
             }
         }
     }
 
-    private String principalLogText() {
-        return expiringCredential == null ? principalName

Review Comment:
   @rondagostino Do you recall why we had this code? I don't understand how 
we'd have no expiring credential and a non-null principal name. It seems like 
the latter would always be set from the expiring credential? Or am I missing 
something?



-- 
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: jira-unsubscr...@kafka.apache.org

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