From ed387d824de46049c637c1571760daddeefc1ae1 Mon Sep 17 00:00:00 2001 From: Alexander Schepanovski Date: Wed, 17 Jul 2019 22:12:15 +0700 Subject: [PATCH] remote: do not swallow exceptions by ssh pool --- dvc/remote/ssh/pool.py | 1 + tests/conftest.py | 13 +++++++------ tests/func/test_data_cloud.py | 5 ++--- tests/unit/remote/ssh/test_pool.py | 11 +++++++++++ 4 files changed, 21 insertions(+), 9 deletions(-) create mode 100644 tests/unit/remote/ssh/test_pool.py diff --git a/dvc/remote/ssh/pool.py b/dvc/remote/ssh/pool.py index 2a519c02e..a40f00606 100644 --- a/dvc/remote/ssh/pool.py +++ b/dvc/remote/ssh/pool.py @@ -13,6 +13,7 @@ def ssh_connection(*conn_args, **conn_kwargs): yield conn except BaseException: conn.close() + raise else: pool.release(conn) diff --git a/tests/conftest.py b/tests/conftest.py index c87b4e2d5..1d983ea7b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -83,17 +83,18 @@ key_path = os.path.join(here, "{0}.key".format(user)) def ssh_server(): users = {user: key_path} with mockssh.Server(users) as s: + s.test_creds = { + "host": s.host, + "port": s.port, + "username": user, + "key_filename": key_path, + } yield s @pytest.fixture def ssh(ssh_server): - yield SSHConnection( - ssh_server.host, - username=user, - port=ssh_server.port, - key_filename=key_path, - ) + yield SSHConnection(**ssh_server.test_creds) @pytest.fixture diff --git a/tests/func/test_data_cloud.py b/tests/func/test_data_cloud.py index ef6558939..d074feacf 100644 --- a/tests/func/test_data_cloud.py +++ b/tests/func/test_data_cloud.py @@ -31,8 +31,6 @@ from dvc.utils import file_md5 from dvc.utils.stage import load_stage_file, dump_stage_file from tests.basic_env import TestDvc -from tests.conftest import user -from tests.conftest import key_path from tests.utils import spy @@ -460,10 +458,11 @@ class TestRemoteSSHMocked(TestDataCloudBase): self.method_name = request.function.__name__ def _get_url(self): + user = self.ssh_server.test_creds["username"] return get_ssh_url_mocked(user, self.ssh_server.port) def _get_keyfile(self): - return key_path + return self.ssh_server.test_creds["key_filename"] def _should_test(self): return True diff --git a/tests/unit/remote/ssh/test_pool.py b/tests/unit/remote/ssh/test_pool.py new file mode 100644 index 000000000..fb9e61a58 --- /dev/null +++ b/tests/unit/remote/ssh/test_pool.py @@ -0,0 +1,11 @@ +import pytest + +from dvc.remote.ssh.pool import ssh_connection + + +def test_doesnt_swallow_errors(ssh_server): + class MyError(Exception): + pass + + with pytest.raises(MyError), ssh_connection(**ssh_server.test_creds): + raise MyError