MINOR: Move Log4jControllerRegistration to server module#22779
MINOR: Move Log4jControllerRegistration to server module#22779majialoong wants to merge 3 commits into
Conversation
github-actions
Bot
added
triage
mimaison
left a comment
mimaison
left a comment
There was a problem hiding this comment.
Thanks for the PR. I left a question
| } else { | ||
| LOGGER.warn("Couldn't register `{}` MBean", MBEAN_NAME); |
There was a problem hiding this comment.
I wonder if we actually need this branch. When registerMBean() returns false it already prints an error message with a similar message. It seems the previous catch clause in the Scala code was unreachable as registerMBean() can't throw.
There was a problem hiding this comment.
Thanks for the review.
Good point. Utils.registerMBean already catches registration failures and logs them internally, so the previous Scala catch block was effectively unreachable. I agree that the extra warning is redundant, so I'll remove this branch.
| protected var logIdent: String = _ | ||
|
|
||
| Log4jControllerRegistration | ||
| Log4jControllerRegistration.register() |
There was a problem hiding this comment.
I'm wondering whether this is the best class to call Log4jControllerRegistration.register()?
3 participants
Move
Log4jControllerRegistrationfrom core to the server module.Reviewers: Mickael Maison mickael.maison@gmail.com, Chia-Ping Tsai chia7712@gmail.com