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

assume "unknown" platform if _exec_command throws exception #35

Merged
merged 3 commits into from
Nov 16, 2019

Conversation

mrk-its
Copy link
Contributor

@mrk-its mrk-its commented Nov 16, 2019

It seems that it solves problem of forbidden execution of external commands on some servers - after setting platform to "unknown" self.exec_command is never called again.

Fixes #34

@codecov
Copy link

codecov bot commented Nov 16, 2019

Codecov Report

Merging #35 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #35      +/-   ##
==========================================
+ Coverage   96.94%   96.96%   +0.02%     
==========================================
  Files           5        5              
  Lines         327      330       +3     
==========================================
+ Hits          317      320       +3     
  Misses         10       10
Impacted Files Coverage Δ
fs/sshfs/sshfs.py 95.63% <100%> (+0.05%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6495a35...4197876. Read the comment docs.

@mrk-its
Copy link
Contributor Author

mrk-its commented Nov 16, 2019

cc: @althonos please let me know what do you think about that. Thanks!

@althonos
Copy link
Owner

althonos commented Nov 16, 2019

@mrk-its : thanks for the patch! there's an issue with Travis-CI (which is not shown in the PR checks since recently the Github/Travis-CI integration started to fuck up): Mock.assert_called was only introduced in Python 3.6, so the test fails on 3.5.

Simply wrap it that way (there's no need to refactor to handle the test in 3.5, let's just skip it) and we should be clear to merge:

    def test_exec_command_exception(self):
        with utils.mock.patch.object(
            self.fs.delegate_fs(),
            '_exec_command',
            side_effect=paramiko.ssh_exception.SSHException()
        ) as _exec_command:
            self.assertEquals(self.fs.delegate_fs().platform, "unknown")
            if sys.version_info[:2] != (3,5):
                _exec_command.assert_called()

@mrk-its
Copy link
Contributor Author

mrk-its commented Nov 16, 2019

@althonos done!

@althonos
Copy link
Owner

Thanks again ! I'll merge and push a patch release shortly.

@althonos althonos merged commit 4d4423a into althonos:master Nov 16, 2019
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.

"SSHException: Channel" closed on server with no shell access
2 participants