Rework locks model on exec_result. (#94)

Deprecate `ExecResult().lock` property.
Bump to 2.2.0

Expect last release before work start on asyncio support and split py34

Author: penguinolog

doc/source/ExecResult.rst

@@ -1,7 +1,7 @@
 .. ExecResult
 
 API: ExecResult
-===========================
+===============
 
 .. py:module:: exec_helpers
 .. py:currentmodule:: exec_helpers
@@ -23,10 +23,18 @@ API: ExecResult
         :param exit_code: Exit code. If integer - try to convert to BASH enum.
         :type exit_code: typing.Union[int, ExitCodes]
 
-    .. py:attribute:: lock
+    .. py:attribute:: stdout_lock
 
         ``threading.RLock``
         Lock object for thread-safe operation.
+        .. versionadded:: 2.2.0
+
+    .. py:attribute:: stderr_lock
+
+        ``threading.RLock``
+        Lock object for thread-safe operation.
+
+        .. versionadded:: 2.2.0
 
     .. py:attribute:: timestamp
 

exec_helpers/__init__.py

@@ -49,7 +49,7 @@
     "ExecResult",
 )
 
-__version__ = "2.1.3"
+__version__ = "2.2.0"
 __author__ = "Alexey Stepanov"
 __author_email__ = "penguinolog@gmail.com"
 __maintainers__ = {

exec_helpers/exec_result.py

@@ -21,6 +21,7 @@
 import logging
 import threading
 import typing
+import warnings
 
 import yaml
 
@@ -32,6 +33,38 @@
 logger = logging.getLogger(__name__)
 
 
+class MultipleRLock:  # pragma: no cover
+    """Handle multiple locks together."""
+
+    __slots__ = ("__locks",)
+
+    def __init__(self, *locks: threading.RLock) -> None:
+        """Multiple locks handler.
+
+        .. note:: temporary class during transition to separate locks.
+        """
+        self.__locks = locks
+
+    def acquire(self, blocking: bool = True) -> None:
+        """Acquire locks.
+
+        .. note:: Timeout is not supported.
+        """
+        for lock in self.__locks:
+            lock.acquire(blocking=blocking)
+
+    __enter__ = acquire
+
+    def release(self) -> None:
+        """Release locks."""
+        for lock in self.__locks:
+            lock.release()
+
+    def __exit__(self, exc_type: typing.Any, exc_val: typing.Any, exc_tb: typing.Any) -> None:
+        """Context manager usage."""
+        self.release()
+
+
 class ExecResult:
     """Execution result."""
 
@@ -47,6 +80,8 @@ class ExecResult:
         "__stdout_brief",
         "__stderr_brief",
         "__lock",
+        "__stdout_lock",
+        "__stderr_lock",
     ]
 
     def __init__(
@@ -70,7 +105,9 @@ def __init__(
         :param exit_code: Exit code. If integer - try to convert to BASH enum.
         :type exit_code: typing.Union[int, proc_enums.ExitCodes]
         """
-        self.__lock = threading.RLock()
+        self.__stdout_lock = threading.RLock()
+        self.__stderr_lock = threading.RLock()
+        self.__lock = MultipleRLock(self.stdout_lock, self.stderr_lock)
 
         self.__cmd = cmd
         if isinstance(stdin, bytes):
@@ -100,13 +137,41 @@ def __init__(
         self.__stderr_brief = None
 
     @property
-    def lock(self) -> threading.RLock:
+    def lock(self) -> MultipleRLock:  # pragma: no cover
+        """Composite lock for stdout and stderr.
+
+        :return: Composite from stdout_lock and stderr_lock
+        :rtype: MultipleRLock
+
+        .. versionchanged:: 2.2.0 Deprecated
+        """
+        warnings.warn(
+            "ExecResult.lock is deprecated and will be removed at version 3. Use stdout_lock and stderr_lock instead.",
+            DeprecationWarning,
+        )
+        return self.__lock
+
+    @property
+    def stdout_lock(self) -> threading.RLock:
         """Lock object for thread-safe operation.
 
-        :return: internal lock
+        :return: internal lock for stdout
         :rtype: threading.RLock
+
+        .. versionadded:: 2.2.0
         """
-        return self.__lock
+        return self.__stdout_lock
+
+    @property
+    def stderr_lock(self) -> threading.RLock:
+        """Lock object for thread-safe operation.
+
+        :return: internal lock for stderr
+        :rtype: threading.RLock
+
+        .. versionadded:: 2.2.0
+        """
+        return self.__stderr_lock
 
     @property
     def timestamp(self) -> typing.Optional[datetime.datetime]:
@@ -226,7 +291,7 @@ def read_stdout(
         if self.timestamp:
             raise RuntimeError("Final exit code received.")
 
-        with self.lock:
+        with self.stdout_lock:
             self.__stdout_str = self.__stdout_brief = None
             self.__stdout += tuple(self.__poll_stream(src, log, verbose))
 
@@ -253,7 +318,7 @@ def read_stderr(
         if self.timestamp:
             raise RuntimeError("Final exit code received.")
 
-        with self.lock:
+        with self.stderr_lock:
             self.__stderr_str = self.__stderr_brief = None
             self.__stderr += tuple(self.__poll_stream(src, log, verbose))
 
@@ -265,7 +330,7 @@ def stdout_bin(self) -> bytearray:
         and for debug purposes we can use this as data source.
         :rtype: bytearray
         """
-        with self.lock:
+        with self.stdout_lock:
             return self._get_bytearray_from_array(self.stdout)
 
     @property
@@ -274,7 +339,7 @@ def stderr_bin(self) -> bytearray:
 
         :rtype: bytearray
         """
-        with self.lock:
+        with self.stderr_lock:
             return self._get_bytearray_from_array(self.stderr)
 
     @property
@@ -283,7 +348,7 @@ def stdout_str(self) -> str:
 
         :rtype: str
         """
-        with self.lock:
+        with self.stdout_lock:
             if self.__stdout_str is None:
                 self.__stdout_str = self._get_str_from_bin(self.stdout_bin)  # type: ignore
             return self.__stdout_str  # type: ignore
@@ -294,7 +359,7 @@ def stderr_str(self) -> str:
 
         :rtype: str
         """
-        with self.lock:
+        with self.stderr_lock:
             if self.__stderr_str is None:
                 self.__stderr_str = self._get_str_from_bin(self.stderr_bin)  # type: ignore
             return self.__stderr_str  # type: ignore
@@ -305,7 +370,7 @@ def stdout_brief(self) -> str:
 
         :rtype: str
         """
-        with self.lock:
+        with self.stdout_lock:
             if self.__stdout_brief is None:
                 self.__stdout_brief = self._get_brief(self.stdout)  # type: ignore
             return self.__stdout_brief  # type: ignore
@@ -316,7 +381,7 @@ def stderr_brief(self) -> str:
 
         :rtype: str
         """
-        with self.lock:
+        with self.stderr_lock:
             if self.__stderr_brief is None:
                 self.__stderr_brief = self._get_brief(self.stderr)  # type: ignore
             return self.__stderr_brief  # type: ignore
@@ -345,7 +410,7 @@ def exit_code(self, new_val: typing.Union[int, proc_enums.ExitCodes]) -> None:
             raise RuntimeError("Exit code is already received.")
         if not isinstance(new_val, int):
             raise TypeError("Exit code is strictly int, received: {code!r}".format(code=new_val))
-        with self.lock:
+        with self.stdout_lock, self.stderr_lock:
             self.__exit_code = proc_enums.exit_code_to_enum(new_val)
             if self.__exit_code != proc_enums.ExitCodes.EX_INVALID:
                 self.__timestamp = datetime.datetime.utcnow()  # type: ignore
@@ -383,7 +448,7 @@ def stdout_json(self) -> typing.Any:
 
         :rtype: typing.Any
         """
-        with self.lock:
+        with self.stdout_lock:
             return self.__deserialize(fmt="json")
 
     @property
@@ -392,7 +457,7 @@ def stdout_yaml(self) -> typing.Any:
 
         :rtype: typing.Any
         """
-        with self.lock:
+        with self.stdout_lock:
             return self.__deserialize(fmt="yaml")
 
     def __dir__(self) -> typing.List[str]:

exec_helpers/ssh_client.py

@@ -36,7 +36,7 @@ class SSHClient(SSHClientBase):
     @staticmethod
     def _path_esc(path: str) -> str:
         """Escape space character in the path."""
-        return path.replace(" ", "\ ")
+        return path.replace(" ", r"\ ")
 
     def mkdir(self, path: str) -> None:
         """Run 'mkdir -p path' on remote.

test/test_exec_result.py

@@ -153,7 +153,7 @@ def test_json(self):
     @mock.patch("exec_helpers.exec_result.logger", autospec=True)
     def test_wrong_result(self, logger):
         """Test logging exception if stdout if not a correct json"""
-        cmd = "ls -la | awk '{print $1\}'"
+        cmd = r"ls -la | awk '{print $1\}'"
         result = exec_helpers.ExecResult(cmd=cmd)
         with self.assertRaises(exec_helpers.ExecHelperError):
             # noinspection PyStatementEffect

test/test_ssh_client.py

@@ -1249,7 +1249,7 @@ def prepare_sftp_file_tests(client):
         )
         return ssh, _sftp
 
-    def test_exists(self, client, policy, logger):
+    def test_exists(self, client, *args):
         ssh, _sftp = self.prepare_sftp_file_tests(client)
         lstat = mock.Mock()
         _sftp.attach_mock(lstat, "lstat")
@@ -1269,7 +1269,7 @@ def test_exists(self, client, policy, logger):
         self.assertFalse(result)
         lstat.assert_called_once_with(dst)
 
-    def test_stat(self, client, policy, logger):
+    def test_stat(self, client, *args):
         ssh, _sftp = self.prepare_sftp_file_tests(client)
         stat = mock.Mock()
         _sftp.attach_mock(stat, "stat")
@@ -1285,7 +1285,7 @@ def test_stat(self, client, policy, logger):
         self.assertEqual(result.st_uid, 0)
         self.assertEqual(result.st_gid, 0)
 
-    def test_isfile(self, client, policy, logger):
+    def test_isfile(self, client, *args):
         class Attrs(object):
             def __init__(self, mode):
                 self.st_mode = mode
@@ -1318,7 +1318,7 @@ def __init__(self, mode):
         self.assertFalse(result)
         lstat.assert_called_once_with(dst)
 
-    def test_isdir(self, client, policy, logger):
+    def test_isdir(self, client, *args):
         class Attrs(object):
             def __init__(self, mode):
                 self.st_mode = mode
@@ -1352,11 +1352,11 @@ def __init__(self, mode):
 
     @mock.patch("exec_helpers.ssh_client.SSHClient.exists")
     @mock.patch("exec_helpers.ssh_client.SSHClient.execute")
-    def test_mkdir(self, execute, exists, client, policy, logger):
+    def test_mkdir(self, execute, exists, *args):
         exists.side_effect = [False, True]
 
         dst = "~/tst dir"
-        escaped_dst = "~/tst\ dir"
+        escaped_dst = r"~/tst\ dir"
 
         # noinspection PyTypeChecker
         ssh = exec_helpers.SSHClient(
@@ -1379,7 +1379,7 @@ def test_mkdir(self, execute, exists, client, policy, logger):
         execute.assert_not_called()
 
     @mock.patch("exec_helpers.ssh_client.SSHClient.execute")
-    def test_rm_rf(self, execute, client, policy, logger):
+    def test_rm_rf(self, execute, *args):
         dst = "~/tst"
 
         # noinspection PyTypeChecker
@@ -1392,7 +1392,7 @@ def test_rm_rf(self, execute, client, policy, logger):
         ssh.rm_rf(dst)
         execute.assert_called_once_with("rm -rf {}".format(dst))
 
-    def test_open(self, client, policy, logger):
+    def test_open(self, client, *args):
         ssh, _sftp = self.prepare_sftp_file_tests(client)
         fopen = mock.Mock(return_value=True)
         _sftp.attach_mock(fopen, "open")
@@ -1437,7 +1437,7 @@ def test_download(self, isdir, remote_isdir, exists, remote_exists, logger, clie
 
     @mock.patch("exec_helpers.ssh_client.SSHClient.isdir")
     @mock.patch("os.path.isdir", autospec=True)
-    def test_upload_file(self, isdir, remote_isdir, client, policy, logger):
+    def test_upload_file(self, isdir, remote_isdir, client, *args):
         ssh, _sftp = self.prepare_sftp_file_tests(client)
         isdir.return_value = False
         remote_isdir.return_value = False
@@ -1455,7 +1455,7 @@ def test_upload_file(self, isdir, remote_isdir, client, policy, logger):
     @mock.patch("os.walk")
     @mock.patch("exec_helpers.ssh_client.SSHClient.isdir")
     @mock.patch("os.path.isdir", autospec=True)
-    def test_upload_dir(self, isdir, remote_isdir, walk, mkdir, exists, client, policy, logger):
+    def test_upload_dir(self, isdir, remote_isdir, walk, mkdir, exists, client, *args):
         ssh, _sftp = self.prepare_sftp_file_tests(client)
         isdir.return_value = True
         remote_isdir.return_value = True