[WFCORE-6843] Logging a WARN if a deployment's runtime name doesn't have an extension #6340
+28
−0
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
|
Thank you for your very quick response and sorry for the late response, I overlooked that. I will look at it now. Thanks for the information. :) |
khermano
force-pushed
the
WFCORE-6843_from_main
branch
from
2f2cb16 to
ac61cc4
Compare
yersan
requested changes
Feb 17, 2025
Comment on lines
118
to
123
| /** | ||
| * A logger with the category of {@code org.jboss.as.server.deployment.warning}. | ||
| * <strong>Usage:</strong> Use this in code related to deployment's warnings | ||
| */ | ||
| ServerLogger DEPLOYMENT_WARN_LOGGER = Logger.getMessageLogger(MethodHandles.lookup(), ServerLogger.class, "org.jboss.as.server.deployment.warning"); | ||
|
|
There was a problem hiding this comment.
We should not create specific root loggers for specific levels. We create root loggers and then we configure the levels from outside, for example in a logger configuration file.
In this file there is already a logger for deployment, use it instead.
Comment on lines
59
to
64
| /** | ||
| * A logger with the category of {@code org.jboss.as.domain.controller.operations.deployment.warning}. | ||
| * <strong>Usage:</strong> Use this in code related to deployment's warnings | ||
| */ | ||
| DomainControllerLogger DEPLOYMENT_WARN_LOGGER = Logger.getMessageLogger(MethodHandles.lookup(), DomainControllerLogger.class, "org.jboss.as.domain.controller.operations.deployment.warning"); | ||
|
|
There was a problem hiding this comment.
We should not create specific root loggers for specific levels. We create root loggers and then we configure the levels from outside, for example in a logger configuration file.
There was a problem hiding this comment.
I understand, I just wasn't sure how to implement this correctly since the description of the issue was talking about using distinct logger for this. So, can I just use the ROOT_LOGGER for the domain and DEPLOYMENT_LOGGER for the standalone @yersan ?
There was a problem hiding this comment.
I just wasn't sure how to implement this correctly since the description of the issue was talking about using distinct logger for this
... and I did not properly read the Jira description :(
Got it, it was requested to use a different logger so users can turn off the logger under demand.
In such a case, I would suggest using the following (for sure open to any other idea for the name):
"org.jboss.as.server.deployment.runtime.namecheck"
"org.jboss.as.domain.controller.operations.deployment.runtime.namecheck"
There was a problem hiding this comment.
Sounds great to me, thank you.
First I thought, that this logger could have name so it could be reused for other deployment warnings in the future, but now I realized that it could be tricky to decide when to turn off the warning I am implementing now if the logger would be associated with another warnings with different meaning.
So, I will change also the name DEPLOYMENT_WARN_LOGGER to maybe DEPLOYMENT_NAMECHECK_LOGGER? I am also open to ideas.
There was a problem hiding this comment.
DEPLOYMENT_NAMECHECK_LOGGER is ok, variable name is not an issue, if you read it and identify its meaning, that's ok
| @@ -94,6 +95,9 @@ static void validateRuntimeNames(String deploymentName, OperationContext context | |||
| Resource root = context.readResourceFromRoot(PathAddress.EMPTY_ADDRESS); | |||
| ModelNode domainDeployment = root.getChild(PathElement.pathElement(DEPLOYMENT, deploymentName)).getModel(); | |||
| String runtimeName = getRuntimeName(deploymentName, deployment, domainDeployment); | |||
| if (!runtimeName.contains(".")) { | |||
| DEPLOYMENT_WARN_LOGGER.deploymentsRuntimeNameWithoutExtension(deploymentName, runtimeName); | |||
There was a problem hiding this comment.
Use DomainControllerLogger.ROOT_LOGGER here instead.
|
Core -> WildFly Preview Integration Build 14381 outcome was FAILURE using a merge of ac61cc4 Failed tests |
khermano
force-pushed
the
WFCORE-6843_from_main
branch
from
ac61cc4 to
dac24f1
Compare
|
Core -> Full Integration Build 14591 outcome was FAILURE using a merge of dac24f1 Failed tests |
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: https://issues.redhat.com/browse/WFCORE-6843