Fix LowestAddressJoinDecider cluster formation warning log to not depend on INFO logging level being set #647
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ptrdom
force-pushed
the
lowest-address-join-decider-logging-fix
branch
from
94c5a48
to
b86f78c
Compare
octonato
previously approved these changes
Mar 30, 2020
octonato
reviewed
Mar 30, 2020
octonato
dismissed
their stale review
I'm removing my review. I have missed something, see comments.
octonato
reviewed
Mar 30, 2020
...er-bootstrap/src/main/scala/akka/management/cluster/bootstrap/LowestAddressJoinDecider.scala
Outdated
Show resolved
Hide resolved
…end on INFO logging level being set
ptrdom
force-pushed
the
lowest-address-join-decider-logging-fix
branch
from
eb2c326
to
c48cdb8
Compare
octonato
reviewed
Mar 30, 2020
octonato
reviewed
Mar 30, 2020
| @@ -74,7 +74,8 @@ class LowestAddressJoinDecider(system: ActorSystem, settings: ClusterBootstrapSe | |||
| lowestAddress.map(contactPointString).getOrElse(""), | |||
| info.contactPoints.map(contactPointString).mkString(", ") | |||
| ) | |||
| else | |||
| } else { | |||
| if (log.isWarningEnabled) | |||
There was a problem hiding this comment.
Strictly speaking, checking if warning logging is enabled is not that useful. Warn logging is often enable so the chances of optimisation are minimal here. That's much different that info and debug. Those are the levels that are in practice tweaked by users.
There was a problem hiding this comment.
Anyway, not a big issue and I think it's ok to leave like that. That was just a side comment.
octonato
approved these changes
Mar 30, 2020
|
The build failure is because we test against the latest version of k8s with minikube and the install for that no longer works. I'll fix that under a separate issue. |
|
The failure is unrelated and fixed on master |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue is quite self explanatory - I can't see a good reason to have an important warning log to be disabled by not setting logging level to INFO.
References #645