lhotari commented on a change in pull request #17:
URL: https://github.com/apache/pulsar-adapters/pull/17#discussion_r652376017
##########
File path:
pulsar-log4j2-appender/src/main/java/org/apache/pulsar/log4j2/appender/PulsarAppender.java
##########
@@ -186,7 +185,7 @@ public void start() {
try {
manager.startup();
} catch (Exception e) {
- // fail to start the manager
+ LOGGER.error("Failed to start pulsar manager", e);
Review comment:
I didn't change the external behavior of the previous solution. I simply
removed the catch in the location which you linked to and moved the logging to
the upper level (here) since it reduces the confusion about swallowing the
exception and not logging it.
It would be a breaking change to throw the exception as a RuntimeException.
For example, if Pulsar is unavailable the application wouldn't be able to
start. This might not be the desired behavior. I think changing the behavior
would require a new configuration option and that would be a separate PR.
--
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
