Skip to content
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

Closed
wants to merge 10 commits into from

Conversation

dannywade
Copy link
Contributor

Allows SessionLog object to be passed to session_log.

@dannywade dannywade changed the title Fixes #3307 - pass SessionLog obj to session_log arg Pass SessionLog obj to session_log arg Oct 13, 2023
# 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))
Copy link
Owner

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))

Copy link
Contributor Author

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.

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()
Copy link
Owner

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.

Copy link
Contributor Author

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.

Copy link
Owner

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() )

Copy link
Contributor Author

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.

@ktbyers
Copy link
Owner

ktbyers commented Oct 18, 2023

Superseded by:

#3313

@ktbyers ktbyers closed this Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants