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

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

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


##########
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:
   You are correct, given the way the code is written it is not possible to have both `expiringCredential == null` and `principalName != null`.  I can't remember specifically why the code was written this way, but it was probably just being defensive in case the above constraint is ever violated.  But the constraint is an intuitive one and the code will almost certainly not evolve in such a way as to violate it, so I think it may just cause the question to be raised and add to confusion.



-- 
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-unsubscribe@kafka.apache.org

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