-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Pass SessionLog obj to session_log arg #3308
Conversation
netmiko/base_connection.py
Outdated
# SessionLog object | ||
self.session_log = session_log | ||
# Add filter after self.session_log is constructed | ||
log.addFilter(SecretsFilter(no_log=self.session_log.no_log)) |
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.
I think we should decouple this, not have Netmiko does this automatically (i.e. the user should do this in their code if they want to set the Python logging the same way as the session_log).
End-user should be able to do this:
>>> import netmiko
>>> netmiko.log
<Logger netmiko (WARNING)>
>>> from netmiko.base_connection import SecretsFilter
>>> netmiko.log.addFilter(SecretsFilter(no_log=log_dict))
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.
Agreed. I pushed a change to remove the filter.
netmiko/base_connection.py
Outdated
log.addFilter(SecretsFilter(no_log=self.session_log.no_log)) | ||
# Check if filename was provided in session_log | ||
if self.session_log.file_name is not None: | ||
self.session_log.open() |
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.
I probably wouldn't call open()
, I would pass in an already open() session_log. I think this would give end-users more flexibility (for example, if they were trying to do something more complex like multiple-thread session_log.
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.
Okay. Would you mind suggesting how that should be accomplished? I'm unsure how to implement that... maybe I'm overthinking 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.
Just remove lines 397
and 398
.
The SessionLog object would already have been opened
when passed into Netmiko (i.e. so you pass in an already opened SessionLog
object that you can just write()
)
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.
Ah yeah, I figured I was overthinking it 😅. Changes pushed.
Superseded by: |
Allows SessionLog object to be passed to session_log.