From 9017bef472be49d75ababc7eac943881dbee8009 Mon Sep 17 00:00:00 2001 From: Trygve Aspenes Date: Fri, 17 Jun 2022 13:35:10 +0200 Subject: [PATCH 1/3] added configurable scp put timeout. added tests --- trollmoves/movers.py | 13 +++++++++-- trollmoves/tests/test_ssh_server.py | 35 ++++++++++++++++++++++++++++- 2 files changed, 45 insertions(+), 3 deletions(-) diff --git a/trollmoves/movers.py b/trollmoves/movers.py index dbd5e354..a7134beb 100644 --- a/trollmoves/movers.py +++ b/trollmoves/movers.py @@ -23,6 +23,7 @@ """Movers for the move_it scripts.""" +from doctest import ELLIPSIS_MARKER import logging import os import shutil @@ -35,7 +36,7 @@ from ftplib import FTP, all_errors, error_perm from paramiko import SSHClient, SSHException, AutoAddPolicy -from scp import SCPClient +from scp import SCPClient, SCPException from trollmoves.utils import clean_url @@ -357,7 +358,8 @@ def copy(self): self._dest_username) try: - scp = SCPClient(ssh_connection.get_transport()) + scp = SCPClient(ssh_connection.get_transport(), + socket_timeout=self.attrs.get('scpclient_timeout_seconds', 10.0)) except Exception as err: LOGGER.error("Failed to initiate SCPClient: %s", str(err)) ssh_connection.close() @@ -373,6 +375,13 @@ def copy(self): else: LOGGER.error("OSError in scp.put: %s", str(osex)) raise + except SCPException as scpe: + if str(err) in "Timeout waiting for scp response": + LOGGER.error("SCPClient put got a socket timeout. You could add scpclient_timeout_seconds to your config " + "to increase the timeout interval. Default timeout is 10 seconds.") + else: + LOGGER.error("SCPException: %s", str(err)) + raise except Exception as err: LOGGER.error("Something went wrong with scp: %s", str(err)) LOGGER.error("Exception name %s", type(err).__name__) diff --git a/trollmoves/tests/test_ssh_server.py b/trollmoves/tests/test_ssh_server.py index 51411348..bb0be4db 100644 --- a/trollmoves/tests/test_ssh_server.py +++ b/trollmoves/tests/test_ssh_server.py @@ -1,7 +1,7 @@ #!/usr/bin/env python # -*- coding: utf-8 -*- # -# Copyright (c) 2019 +# Copyright (c) 2019, 2022 # # Author(s): # @@ -54,6 +54,7 @@ def setUp(self): self._attrs_empty = {} self._attrs_connection_uptime = {'connection_uptime': 0} + self._attrs_timeout = {'scpclient_timeout_seconds': 1} def tearDown(self): try: @@ -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): + """Check scp copy.""" + mocked_scp_client = MagicMock() + mock_scp_client.return_value = mocked_scp_client + + scp_mover = trollmoves.movers.ScpMover(self.origin, self.destination_no_port, attrs=self._attrs_timeout) + scp_mover.copy() + + 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_generic_exception(self, mock_scp_client, mock_sshclient): @@ -216,6 +229,26 @@ def test_scp_copy_put_exception(self, mock_scp_client): with pytest.raises(Exception): scp_mover.copy() + @patch('trollmoves.movers.SCPClient', autospec=True) + def test_scp_copy_put_SCPException(self, mock_scp_client): + """Check scp client.put() raising SCPException.""" + from scp import SCPException + scp_mover = trollmoves.movers.ScpMover(self.origin, self.destination_no_port, attrs=self._attrs_empty) + mock_scp_client.return_value.put.side_effect = SCPException('Timeout waiting for scp response') + + with pytest.raises(SCPException, match='Timeout waiting for scp response'): + scp_mover.copy() + + @patch('trollmoves.movers.SCPClient', autospec=True) + def test_scp_copy_put_SCPException2(self, mock_scp_client): + """Check scp client.put() raising SCPException.""" + from scp import SCPException + scp_mover = trollmoves.movers.ScpMover(self.origin, self.destination_no_port, attrs=self._attrs_empty) + mock_scp_client.return_value.put.side_effect = SCPException('Other exception') + + with pytest.raises(SCPException, match='Other exception'): + scp_mover.copy() + @patch('trollmoves.movers.SSHClient', autospec=True) @patch('trollmoves.movers.SCPClient', autospec=True) def test_scp_move(self, mock_scp_client, mock_sshclient): From ef0b68187ecc816ac600a4176ab11327c6b7719b Mon Sep 17 00:00:00 2001 From: Trygve Aspenes Date: Fri, 17 Jun 2022 13:45:59 +0200 Subject: [PATCH 2/3] fix stikler --- trollmoves/movers.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/trollmoves/movers.py b/trollmoves/movers.py index a7134beb..118c3f49 100644 --- a/trollmoves/movers.py +++ b/trollmoves/movers.py @@ -376,11 +376,11 @@ def copy(self): LOGGER.error("OSError in scp.put: %s", str(osex)) raise except SCPException as scpe: - if str(err) in "Timeout waiting for scp response": - LOGGER.error("SCPClient put got a socket timeout. You could add scpclient_timeout_seconds to your config " - "to increase the timeout interval. Default timeout is 10 seconds.") + if str(scpe) in "Timeout waiting for scp response": + LOGGER.error("SCPClient put got a socket timeout. You could add scpclient_timeout_seconds " + "to your config to increase the timeout interval. Default timeout is 10 seconds.") else: - LOGGER.error("SCPException: %s", str(err)) + LOGGER.error("SCPException: %s", str(scpe)) raise except Exception as err: LOGGER.error("Something went wrong with scp: %s", str(err)) From 570883d1179b0f860a2525e9cea96defe1ba2a77 Mon Sep 17 00:00:00 2001 From: Trygve Aspenes Date: Fri, 17 Jun 2022 15:28:45 +0200 Subject: [PATCH 3/3] need to be a value --- trollmoves/movers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trollmoves/movers.py b/trollmoves/movers.py index 0e99c84a..ab5b1349 100644 --- a/trollmoves/movers.py +++ b/trollmoves/movers.py @@ -360,7 +360,7 @@ def copy(self): try: scp = SCPClient(ssh_connection.get_transport(), - socket_timeout=self.attrs.get('scpclient_timeout_seconds', 10.0)) + socket_timeout=int(self.attrs.get('scpclient_timeout_seconds', 10))) except Exception as err: LOGGER.error("Failed to initiate SCPClient: %s", str(err)) ssh_connection.close()