gemmellr commented on PR #4177: URL: https://github.com/apache/activemq-artemis/pull/4177#issuecomment-1214907597
This seems quite complicated for just specifying an alternative logging config. It appears to support updating only a subset of the base config, e.g adding new things, or overriding an existing property value, without necessarily specifying the full configuration, also jumping hoops to make sure other areas are updated appropriately (e.g adding a new logger to the list of loggers). Its not clear it really supports removing any existing stuff (except perhaps by re-definining them an empty value for an existing key, and hoping the impl doesnt barf on such prop values). It doesnt seem very nice/maintainable that this programatically duplicates the regular logging.properties file contents within JBossLoggingConfiguration as its starting point for creating a modified config. The two definitions could easily get out of sync, the basic props and the programmatic equivalent. In particular, the fact we are also working to remove JBL usage just now also makes it seem like a less than ideal time to introduce something new as complicated as the JBossLoggingConfiguration class which is heavily dependent on that config structure, and would need adapted (if it even could be) to a replacement. Why not not simply have the CLI option allow providing a _complete_ logging configuration file the user can provide? Then if the option is specified, that file is copied into place as the instance is created, and if it isnt specified then the default one is copied into place the same as it is currently. Much simpler, likely a handful of lines rather than the several hundred used here, with no need for the mechanism to be handling the structure of the file at all. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
