-
Notifications
You must be signed in to change notification settings - Fork 32
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
SQL-1835: The driver disables all logger when connecting #244
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good, great idea with using the connection hashcode.
I had one comment.
// properties. This way, our handler configuration is not affected by other application | ||
// using JUL | ||
FileHandler fileHandler = | ||
new FileHandler(logPath, 10000000, 1, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the logging.properties
that was are moving away from the Filehandler.count is 5
, is the change to 1
here intentional?
Another question here is, do we want to eventually make some of these values configurable such as the limit
and count
values? If so, we can have a JIRA to address it if we later decide to implement it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is to keep the same behavior as before.
If you look at the previous code, you'll see that we were using the FileHandler(String pattern) constructor and it ignores the properties and sets limit to 0 and count to 1.
public FileHandler(String pattern) throws IOException, SecurityException {
if (pattern.length() < 1 ) {
throw new IllegalArgumentException();
}
checkPermission();
configure();
this.pattern = pattern;
this.limit = 0;
this.count = 1;
openFiles();
}
Using 5 for count actually creates a new files for each "batch" of 5 connections.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes look good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks for the explanation!
MongoDriver used to reset the log manager configuration with the configuration of the logging.properties from inside our jar. This is problematic for other applications using JUL as their configuration is deleted.
Instead, we now do the configuration for our Filehandler programmatically.