-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Logging or not logging or optional logging? #9
Comments
@stanislas as a user of the tool do you have a point of view? |
I think that the logging is because the numbers of logged event is quite small: at startup and when starting/stoping in between. Also, the "normal" messages are logged at INFO Level, so it is not intrusive. |
Ok it makes sense. However during the test the number of exceptions logged is quite high even though the tests pass. I think from a testing perspective it can be quite confusing to follow the logs when you see multiple exceptions and stack traces being displayed. I propose to pass an additional boolean in the NreplServer constructor to make exception logs silent when required. I could use this boolean in during the test to make the log more readable. However the default behaviour i.e. the constuctor with one parameter (port number) would keep logging exception as you suggest (to keep the current behaviour). I can do that. Any reserve? |
Done. |
it's good for me ;) |
I wonder if the log messages are a little too intrusive?
I wonder if it wouldn't make sense to make them optional or to turn them off?
I have made changes to the code so now the user has a chance to catch a RuntimeException when start,stop,registerMBean and unRegisterMBean fail.
Why do we need to force the logging of the error if we already throw an exception?
For example is it not intrusive to force the logging of a Sever error when we know we're going to catch it further up and manage it in a way which is not dramatic for the application?
The text was updated successfully, but these errors were encountered: