Skip to content

Conversation

@vladak
Copy link
Member

@vladak vladak commented Apr 9, 2020

This change adds more info to the toString() method of LdapServer. This aids debugging when authorization framework is reloaded or when a server is reconnecting.

Copy link
Contributor

@idodeclare idodeclare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

*/
private synchronized LdapContext connect() {
LOGGER.log(Level.INFO, "Server {0} connecting", this.url);
LOGGER.log(Level.INFO, "Connecting to LDAP server {0} ", this.toString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: is toString() superfluous here and below (and if not, should these logging computations be short-circuited by convention with e.g. if (LOGGER.isLoggable(Level.INFO)))?

@tarzanek
Copy link
Contributor

lgtm, I agree with the nitpick, but this is I guess the same debate of explicit versus implicit
that said I like Scala a lot (although the way how the language is implicit can drive you crazy :-D
but then it's easy to read without all the boilerplate code)

@tarzanek tarzanek merged commit 945dc58 into oracle:master Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants