-
Notifications
You must be signed in to change notification settings - Fork 642
CASSGO-24 Remove global logger #1765
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
Conversation
|
Please format your commit message following the rules stated in CONTRIBUTING.md and add this JIRA to the We need another committer +1 to merge this. |
ccd50b0 to
9aa5d34
Compare
Done. |
2a6781c to
c8cfbeb
Compare
|
Just a nit pick, the commit title is in past tense now ( |
Good catch, I'll change it. |
6f71a2b to
493c135
Compare
493c135 to
3bd1594
Compare
|
@joao-r-reis Should it be merged? |
|
Please update the commit message to include Jackson as a reviewer and then I'll merge it 👍 |
By default, if the logger in the cluster config is not set, the NewSession() method sets a default logger instance which is a deprecated global variable. patch by Oleksandr Luzhniy; reviewed by João Reis, Stanislav Bychkov, Jackson Fleming, for CASSGO-24
3bd1594 to
5f99d18
Compare
Done |
|
|
||
| // Logger for this ClusterConfig. | ||
| // If not specified, defaults to the global gocql.Logger. | ||
| // If not specified, defaults to the gocql.defaultLogger. |
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.
defaultLogger is private so you'd have to look at the code to know what this does. Instead this should be:
// If not specified, defaults to using the log package.
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.
Understood, it makes sense. I will lay a hand on that on the next PRs.
This PR removes the global instance of the logger. For now, if the logger in the cluster config is not set, the new approach will set a new instance of the default logger for each session.
Issue: #1766