Fix #25288 asadmin delete-log-levels command fails if the --target option is specified #25289
Conversation
Signed-off-by: Takahiro Nagao <[email protected]>
| @@ -385,7 +385,7 @@ public synchronized Map<String, String> deleteLoggingProperties(final Collection | |||
| LOG.log(Level.INFO, "deleteLoggingProperties(properties={0})", properties); | |||
| loadLoggingProperties(); | |||
| remove(properties); | |||
| rewritePropertiesFileAndNotifyMonitoring(); | |||
| rewritePropertiesFileAndNotifyMonitoring(""); | |||
There was a problem hiding this comment.
Thank you for your suggestion.
But existing code uses "" instead of null as below, and using "" and null mixedly would not be desirable...
If we want to refactor this code, method overloading (i.e. defining rewritePropertiesFileAndNotifyMonitoring() and closePropFile() without arguments) would be better rather than using "" and null.
Since doing such refactoring in the current pull request obscures the essense of the bug fix, I think we can do it in another pull request if needed.
There was a problem hiding this comment.
I would just replace all those empty strings with null (and in the future this class should never get an empty string target, it should be resolved when parsing the user input). Adding overloaded methods doesn't bring a value in private methods. Refactoring is ok if you put it to separate commit from the fix.
Additional PR means more work to each of us and waiting on CI ;-)
There was a problem hiding this comment.
Okay, I've modified the code to use null instead of "".
nucleus/common/common-util/src/main/java/com/sun/common/util/logging/LoggingConfigImpl.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Takahiro Nagao <[email protected]>
Signed-off-by: Takahiro Nagao <[email protected]>
Fixes #25288.
Fixes the
LoggingConfigImpl#deleteLoggingPropertiesmethod to handle thetargetargument.