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

Fix potential _channel UnboundLocalError #15

Conversation

RobertCochran
Copy link

Sometimes when assigning to _channel, an exception can occur in SFTPClient.from_transport(). Because _channel doesn't exist if that happens, it causes a second exception when trying to call close() in the finally block. Assign to the variable outside the try block initially so that this exception can't happen in the future. Also make sure that _channel is not None before we try to close it to account for it being sometimes None.

erans and others added 2 commits September 12, 2022 16:08
Add ability to disable certain algorithms
If there's an exception thrown as we're calling `SFTPClient.from_transport()`,
then `_channel` won't ever get created in the lexical environment and this
causes a `UnboundLocalError` on top of the exception that's getting passed
along, creating unnecessary noise. Fix this by always defining the variable, and
then making sure it is non-`None` before trying to close it.
@byteskeptical byteskeptical self-assigned this Sep 24, 2022
@byteskeptical byteskeptical added the bug Something isn't working label Sep 24, 2022
@byteskeptical
Copy link
Owner

I'm merging these changes with PR #13 for a few different reasons. Hoping to keep all commits into root signed at least for now. I also wanted to update changes in the docs directory but couldn't push bach to your branch. Kudos for moving to GitLab and for providing a fix for this regression.

byteskeptical added a commit that referenced this pull request Sep 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

calling basicConfig at this level for a library that should be embedded changes all dowstream logs
3 participants