-
Notifications
You must be signed in to change notification settings - Fork 11
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
added configurable scp put timeout. added tests #121
base: main
Are you sure you want to change the base?
added configurable scp put timeout. added tests #121
Conversation
…long/trollmoves into issue120-scpclient-timeout-config
Codecov Report
@@ Coverage Diff @@
## main #121 +/- ##
==========================================
+ Coverage 76.57% 76.74% +0.17%
==========================================
Files 18 18
Lines 3893 3922 +29
==========================================
+ Hits 2981 3010 +29
Misses 912 912
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
Not sure what the stickler issue is about. ELLIPSIS_MARKER is not used. Was it used in the past and not used anymore? If you have time @pnuu |
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.
In principle LGTM, but couple of requests inline.
@@ -23,6 +23,7 @@ | |||
|
|||
"""Movers for the move_it scripts.""" | |||
|
|||
from doctest import ELLIPSIS_MARKER |
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 don't see this in main
at all, so don't know where it's coming from. Should be safe to remove.
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.
Oh, actually, it's coming from this PR since it shows up in the diff.
@@ -178,6 +179,18 @@ def test_scp_copy(self, mock_scp_client, mock_sshclient): | |||
|
|||
mocked_scp_client.put.assert_called_once_with(self.origin, urlparse(self.destination_no_port).path) | |||
|
|||
@patch('trollmoves.movers.SSHClient', autospec=True) | |||
@patch('trollmoves.movers.SCPClient', autospec=True) | |||
def test_scp_copy_custom_timeout(self, mock_scp_client, mock_sshclient): |
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.
The timeout isn't tested in this test, so either adjust the name or test the timeout.
Added configurable scpclient_timeout_seconds attribute to the scpclient put.