From 2a0659b187e3eca5dcf3333f8b767e692bd4d9fa Mon Sep 17 00:00:00 2001 From: Trygve Aspenes Date: Thu, 15 Dec 2022 09:42:54 +0100 Subject: [PATCH 1/5] first attempt to add copy to remote tmp filename and rename --- trollmoves/movers.py | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/trollmoves/movers.py b/trollmoves/movers.py index a1c8da39..c033f356 100644 --- a/trollmoves/movers.py +++ b/trollmoves/movers.py @@ -354,6 +354,7 @@ def move(self): def copy(self): """Upload the file.""" from scp import SCPClient + from paramiko import SSHException ssh_connection = self.get_connection(self.destination.hostname, self.destination.port or 22, @@ -367,7 +368,24 @@ def copy(self): raise try: - scp.put(self.origin, self.destination.path) + destination = self.destination.path + remote_tmp = self.attrs.get("remote_tmp", "True") + if remote_tmp: + destination = os.path.join(destination, '.' + os.path.basename(self.origin)) + scp.put(self.origin, destination) + + if remote_tmp: + timeout = self.attrs.get("ssh_connection_timeout", None) + _remote_orig = os.path.join(self.destination.path, os.path.basename(self.origin)) + _cmd = f"mv {destination} {_remote_orig}" + (_, out_ret, err_ret) = ssh_connection.exec_command(_cmd, timeout=timeout) + out_lines = out_ret.readlines() + for l in out_lines: + LOGGER.debug("Remote rename stdout: %s ", str(l)) + err_lines = err_ret.readlines() + for l in err_lines: + LOGGER.error("Remote rename stderr: %s ", str(l)) + print("Remote rename return with %s and %s", str(out_lines), str(err_lines)) except OSError as osex: if osex.errno == 2: LOGGER.error("No such file or directory. File not transfered: " @@ -376,6 +394,8 @@ def copy(self): else: LOGGER.error("OSError in scp.put: %s", str(osex)) raise + except SSHException as sshe: + LOGGER.exception("Failed to rename from tmp name: %s", str(sshe)) except Exception as err: LOGGER.error("Something went wrong with scp: %s", str(err)) LOGGER.error("Exception name %s", type(err).__name__) From 9c9538143248026f8e57e98b8fc60bd91fc5a9b2 Mon Sep 17 00:00:00 2001 From: Trygve Aspenes Date: Thu, 15 Dec 2022 10:12:20 +0100 Subject: [PATCH 2/5] flake8 --- trollmoves/movers.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/trollmoves/movers.py b/trollmoves/movers.py index c033f356..50e4b762 100644 --- a/trollmoves/movers.py +++ b/trollmoves/movers.py @@ -380,12 +380,11 @@ def copy(self): _cmd = f"mv {destination} {_remote_orig}" (_, out_ret, err_ret) = ssh_connection.exec_command(_cmd, timeout=timeout) out_lines = out_ret.readlines() - for l in out_lines: - LOGGER.debug("Remote rename stdout: %s ", str(l)) + for line in out_lines: + LOGGER.debug("Remote rename stdout: %s ", str(line)) err_lines = err_ret.readlines() - for l in err_lines: - LOGGER.error("Remote rename stderr: %s ", str(l)) - print("Remote rename return with %s and %s", str(out_lines), str(err_lines)) + for line in err_lines: + LOGGER.error("Remote rename stderr: %s ", str(line)) except OSError as osex: if osex.errno == 2: LOGGER.error("No such file or directory. File not transfered: " From 5bd05d2c39339bd7a7ff146837d1692880128473 Mon Sep 17 00:00:00 2001 From: Trygve Aspenes Date: Thu, 15 Dec 2022 10:58:15 +0100 Subject: [PATCH 3/5] remote tmp defaults to None --- trollmoves/movers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/trollmoves/movers.py b/trollmoves/movers.py index 50e4b762..aa079598 100644 --- a/trollmoves/movers.py +++ b/trollmoves/movers.py @@ -369,7 +369,7 @@ def copy(self): try: destination = self.destination.path - remote_tmp = self.attrs.get("remote_tmp", "True") + remote_tmp = self.attrs.get("remote_tmp", None) if remote_tmp: destination = os.path.join(destination, '.' + os.path.basename(self.origin)) scp.put(self.origin, destination) From eded42446eb9b634f4a217b19c7ed0dc2ac41460 Mon Sep 17 00:00:00 2001 From: Trygve Aspenes Date: Thu, 15 Dec 2022 18:00:34 +0100 Subject: [PATCH 4/5] add tests --- trollmoves/tests/test_ssh_server.py | 84 +++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/trollmoves/tests/test_ssh_server.py b/trollmoves/tests/test_ssh_server.py index fb68210f..9d6341c0 100644 --- a/trollmoves/tests/test_ssh_server.py +++ b/trollmoves/tests/test_ssh_server.py @@ -21,6 +21,9 @@ # along with this program. If not, see . """Test the ssh server.""" +import os +import sys +import logging import shutil from unittest.mock import Mock, MagicMock, patch import unittest @@ -33,6 +36,16 @@ import trollmoves +logger = logging.getLogger() + +class MockChannel: + def __init__(self, content=None): + self.content = [] if not content else [content] + def __str__(self) -> str: + return str(self.content) + def readlines(self): + return self.content + class TestSSHMovers(unittest.TestCase): """Tests for SSH Mover.""" @@ -254,6 +267,77 @@ def test_scp_move(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.ScpMover.get_connection') + @patch('paramiko.SSHClient.connect') + @patch('scp.SCPClient', autospec=True) + def test_scp_copy_via_remote_tmp2(self, mock_scp_client, mock_sshconnect, mock_sshexec): + """Check scp copy using remote temporary file.""" + from trollmoves.movers import ScpMover + + mocked_scp_client = MagicMock() + mock_scp_client.return_value = mocked_scp_client + mock_sshexec.return_value.exec_command.return_value = [(None), (MockChannel()), (MockChannel())] + scp_mover = ScpMover(self.origin, self.destination_no_port, attrs={'remote_tmp': True}) + scp_mover.copy() + + tmp_bn = os.path.join(urlparse(self.destination_no_port).path, + "." + os.path.basename(self.origin)) + mocked_scp_client.put.assert_called_once_with(self.origin, tmp_bn) + final_remote = os.path.join(urlparse(self.destination_no_port).path, + os.path.basename(self.origin)) + _cmd = f"mv {tmp_bn} {final_remote}" + mock_sshexec.return_value.exec_command.assert_called_once_with(_cmd, timeout=None) + + @patch('trollmoves.movers.ScpMover.get_connection') + @patch('paramiko.SSHClient.connect') + @patch('scp.SCPClient', autospec=True) + def test_scp_copy_via_remote_tmp_return_values(self, mock_scp_client, mock_sshconnect, mock_sshexec): + """Check scp copy using remote temporary file.""" + from trollmoves.movers import ScpMover + stream_handler = logging.StreamHandler(sys.stdout) + logger.addHandler(stream_handler) + logger.setLevel(logging.INFO) + + mocked_scp_client = MagicMock() + mock_scp_client.return_value = mocked_scp_client + mock_sshexec.return_value.exec_command.return_value = [(None), (MockChannel("stdout")), (MockChannel("stderr"))] + try: + with self.assertLogs(logger, level=logging.DEBUG) as lc: + scp_mover = ScpMover(self.origin, self.destination_no_port, attrs={'remote_tmp': True}) + scp_mover.copy() + self.assertIn("Remote rename stdout: stdout", "".join(lc.output)) + self.assertIn("Remote rename stderr: stderr", "".join(lc.output)) + finally: + logger.removeHandler(stream_handler) + + tmp_bn = os.path.join(urlparse(self.destination_no_port).path, + "." + os.path.basename(self.origin)) + mocked_scp_client.put.assert_called_once_with(self.origin, tmp_bn) + final_remote = os.path.join(urlparse(self.destination_no_port).path, + os.path.basename(self.origin)) + _cmd = f"mv {tmp_bn} {final_remote}" + mock_sshexec.return_value.exec_command.assert_called_once_with(_cmd, timeout=None) + + @patch('trollmoves.movers.ScpMover.get_connection') + @patch('paramiko.SSHClient.connect') + @patch('scp.SCPClient', autospec=True) + def test_scp_copy_via_remote_tmp_exception(self, mock_scp_client, mock_sshconnect, mock_sshexec): + """Check scp copy using remote temporary file.""" + from trollmoves.movers import ScpMover + stream_handler = logging.StreamHandler(sys.stdout) + logger.addHandler(stream_handler) + logger.setLevel(logging.INFO) + + mocked_scp_client = MagicMock() + mock_scp_client.return_value = mocked_scp_client + mock_sshexec.return_value.exec_command.side_effect = MagicMock(side_effect=SSHException) + try: + with self.assertLogs(logger, level=logging.DEBUG) as lc: + scp_mover = ScpMover(self.origin, self.destination_no_port, attrs={'remote_tmp': True}) + scp_mover.copy() + self.assertIn("Failed to rename from tmp name:", "".join(lc.output)) + finally: + logger.removeHandler(stream_handler) if __name__ == '__main__': unittest.main() From c1a7aae868d857731f349a6a5be4617ddfcf77b4 Mon Sep 17 00:00:00 2001 From: stickler-ci Date: Thu, 15 Dec 2022 17:03:25 +0000 Subject: [PATCH 5/5] Fixing style errors. --- trollmoves/tests/test_ssh_server.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/trollmoves/tests/test_ssh_server.py b/trollmoves/tests/test_ssh_server.py index 9d6341c0..83dd5bb0 100644 --- a/trollmoves/tests/test_ssh_server.py +++ b/trollmoves/tests/test_ssh_server.py @@ -38,11 +38,14 @@ logger = logging.getLogger() + class MockChannel: def __init__(self, content=None): self.content = [] if not content else [content] + def __str__(self) -> str: return str(self.content) + def readlines(self): return self.content @@ -280,10 +283,10 @@ def test_scp_copy_via_remote_tmp2(self, mock_scp_client, mock_sshconnect, mock_s scp_mover = ScpMover(self.origin, self.destination_no_port, attrs={'remote_tmp': True}) scp_mover.copy() - tmp_bn = os.path.join(urlparse(self.destination_no_port).path, + tmp_bn = os.path.join(urlparse(self.destination_no_port).path, "." + os.path.basename(self.origin)) mocked_scp_client.put.assert_called_once_with(self.origin, tmp_bn) - final_remote = os.path.join(urlparse(self.destination_no_port).path, + final_remote = os.path.join(urlparse(self.destination_no_port).path, os.path.basename(self.origin)) _cmd = f"mv {tmp_bn} {final_remote}" mock_sshexec.return_value.exec_command.assert_called_once_with(_cmd, timeout=None) @@ -310,10 +313,10 @@ def test_scp_copy_via_remote_tmp_return_values(self, mock_scp_client, mock_sshco finally: logger.removeHandler(stream_handler) - tmp_bn = os.path.join(urlparse(self.destination_no_port).path, + tmp_bn = os.path.join(urlparse(self.destination_no_port).path, "." + os.path.basename(self.origin)) mocked_scp_client.put.assert_called_once_with(self.origin, tmp_bn) - final_remote = os.path.join(urlparse(self.destination_no_port).path, + final_remote = os.path.join(urlparse(self.destination_no_port).path, os.path.basename(self.origin)) _cmd = f"mv {tmp_bn} {final_remote}" mock_sshexec.return_value.exec_command.assert_called_once_with(_cmd, timeout=None)