Drop chroot setting possibility in __init__ and public property

Design issue

Signed-off-by: Aleksei Stepanov <penguinolog@gmail.com>

(cherry picked from commit 52aa1b3)
Signed-off-by: Aleksei Stepanov <penguinolog@gmail.com>

Author: penguinolog

doc/source/Subprocess.rst

@@ -40,6 +40,17 @@ API: Subprocess
 
         .. versionchanged:: 1.1.0 release lock on exit
 
+    .. py:method:: chroot(path)
+
+        Context manager for changing chroot rules.
+
+        :param path: chroot path or none for working without chroot.
+        :type path: ``typing.Optional[typing.Union[str, typing.Text]``
+        :return: context manager with selected chroot state inside
+        :rtype: typing.ContextManager
+
+        .. Note:: Enter and exit main context manager is produced as well.
+
     .. py:method:: execute_async(command, stdin=None, open_stdout=True, open_stderr=True, verbose=False, log_mask_re=None, **kwargs)
 
         Execute command in async mode and return Popen with IO objects.

exec_helpers/_ssh_client_base.py

@@ -119,8 +119,7 @@ def __call__(  # type: ignore
         password=None,  # type: typing.Optional[typing.Union[str, typing.Text]]
         private_keys=None,  # type: typing.Optional[typing.Iterable[paramiko.RSAKey]]
         auth=None,  # type: typing.Optional[ssh_auth.SSHAuth]
-        verbose=True,  # type: bool
-        chroot_path=None,  # type: typing.Optional[typing.Union[str, typing.Text]]
+        verbose=True  # type: bool
     ):  # type: (...) -> SSHClientBase
         """Main memorize method: check for cached instance and return it.
 
@@ -138,12 +137,10 @@ def __call__(  # type: ignore
         :type auth: typing.Optional[ssh_auth.SSHAuth]
         :param verbose: show additional error/warning messages
         :type verbose: bool
-        :param chroot_path: chroot path (use chroot if set)
-        :type chroot_path: typing.Optional[typing.Union[str, typing.Text]]
         :return: SSH client instance
         :rtype: SSHClientBase
         """
-        if (host, port) in cls.__cache and not chroot_path:  # chrooted connections are not memorized
+        if (host, port) in cls.__cache:
             key = host, port
             if auth is None:
                 auth = ssh_auth.SSHAuth(username=username, password=password, keys=private_keys)
@@ -171,10 +168,8 @@ def __call__(  # type: ignore
             private_keys=private_keys,
             auth=auth,
             verbose=verbose,
-            chroot_path=chroot_path,
         )
-        if not chroot_path:
-            cls.__cache[(ssh.hostname, ssh.port)] = ssh
+        cls.__cache[(ssh.hostname, ssh.port)] = ssh
         return ssh
 
     @classmethod
@@ -272,7 +267,6 @@ def __init__(
         private_keys=None,  # type: typing.Optional[typing.Iterable[paramiko.RSAKey]]
         auth=None,  # type: typing.Optional[ssh_auth.SSHAuth]
         verbose=True,  # type: bool
-        chroot_path=None,  # type: typing.Optional[typing.Union[str, typing.Text]]
     ):  # type: (...) -> None
         """Main SSH Client helper.
 
@@ -290,14 +284,11 @@ def __init__(
         :type auth: typing.Optional[ssh_auth.SSHAuth]
         :param verbose: show additional error/warning messages
         :type verbose: bool
-        :param chroot_path: chroot path (use chroot if set)
-        :type chroot_path: typing.Optional[typing.Union[str, typing.Text]]
 
         .. note:: auth has priority over username/password/private_keys
         """
         super(SSHClientBase, self).__init__(
-            logger=logging.getLogger(self.__class__.__name__).getChild("{host}:{port}".format(host=host, port=port)),
-            chroot_path=chroot_path
+            logger=logging.getLogger(self.__class__.__name__).getChild("{host}:{port}".format(host=host, port=port))
         )
 
         self.__hostname = host
@@ -367,7 +358,7 @@ def __unicode__(self):  # type: () -> typing.Text  # pragma: no cover
             cls=self.__class__.__name__, self=self, username=self.auth.username
         )
 
-    def __str__(self):  # type: () -> bytes  # pragma: no cover
+    def __str__(self):  # type: ignore  # pragma: no cover
         """Representation for debug purposes."""
         return self.__unicode__().encode("utf-8")
 

exec_helpers/api.py

@@ -50,38 +50,39 @@
 )
 
 
-class _ChRootContext(object):
+# noinspection PyProtectedMember
+class _ChRootContext(object):  # pylint: disable=protected-access
     """Context manager for call commands with chroot.
 
     .. versionadded:: 1.12.0
     """
 
-    __slots__ = ("__conn", "__chroot_status", "__path")
+    __slots__ = ("_conn", "_chroot_status", "_path")
 
     def __init__(
         self,
         conn,  # type: ExecHelper
         path=None,  # type: typing.Optional[typing.Union[str, typing.Text]]
-    ):  # type: (...) -> None
+    ):  # type: (...) -> None  # pylint: disable=protected-access
         """Context manager for call commands with sudo.
 
         :param conn: connection instance
         :type conn: ExecHelper
         :param path: chroot path or None for no chroot
         :type path: typing.Optional[str]
         """
-        self.__conn = conn  # type: ExecHelper
-        self.__chroot_status = conn.chroot_path  # type: typing.Optional[typing.Union[str, typing.Text]]
-        self.__path = path  # type: typing.Optional[typing.Union[str, typing.Text]]
+        self._conn = conn  # type: ExecHelper
+        self._chroot_status = conn._chroot_path  # type: typing.Optional[typing.Union[str, typing.Text]]
+        self._path = path  # type: typing.Optional[typing.Union[str, typing.Text]]
 
     def __enter__(self):  # type: () -> None
-        self.__conn.__enter__()
-        self.__chroot_status = self.__conn.chroot_path
-        self.__conn.chroot_path = self.__path
+        self._conn.__enter__()
+        self._chroot_status = self._conn._chroot_path  # pylint: disable=protected-access
+        self._conn._chroot_path = self._path  # pylint: disable=protected-access
 
     def __exit__(self, exc_type, exc_val, exc_tb):  # type: (typing.Any, typing.Any, typing.Any) -> None
-        self.__conn.chroot_path = self.__chroot_status
-        self.__conn.__exit__(exc_type=exc_type, exc_val=exc_val, exc_tb=exc_tb)  # type: ignore
+        self._conn._chroot_path = self._chroot_status  # pylint: disable=protected-access
+        self._conn.__exit__(exc_type=exc_type, exc_val=exc_val, exc_tb=exc_tb)  # type: ignore
 
 
 class ExecHelper(six.with_metaclass(abc.ABCMeta, object)):
@@ -92,17 +93,14 @@ class ExecHelper(six.with_metaclass(abc.ABCMeta, object)):
     def __init__(
         self,
         logger,  # type: logging.Logger
-        log_mask_re=None,  # type: typing.Optional[typing.Text]
-        chroot_path=None  # type: typing.Optional[typing.Union[str, typing.Text]]
+        log_mask_re=None  # type: typing.Optional[typing.Text]
     ):  # type: (...) -> None
         """Global ExecHelper API.
 
         :param logger: logger instance to use
         :type logger: logging.Logger
         :param log_mask_re: regex lookup rule to mask command for logger.
                             all MATCHED groups will be replaced by '<*masked*>'
-        :param chroot_path: chroot path (use chroot if set)
-        :type chroot_path: typing.Optional[str]
         :type log_mask_re: typing.Optional[str]
 
         .. versionchanged:: 1.2.0 log_mask_re regex rule for masking cmd
@@ -112,24 +110,24 @@ def __init__(
         self.__lock = threading.RLock()
         self.__logger = logger
         self.log_mask_re = log_mask_re  # type: typing.Optional[typing.Text]
-        self.__chroot_path = chroot_path  # type: typing.Optional[typing.Union[str, typing.Text]]
+        self.__chroot_path = None  # type: typing.Optional[typing.Union[str, typing.Text]]
 
     @property
     def logger(self):  # type: () -> logging.Logger
         """Instance logger access."""
         return self.__logger
 
     @property
-    def chroot_path(self):  # type: () -> typing.Optional[typing.Union[str, typing.Text]]
+    def _chroot_path(self):  # type: () -> typing.Optional[typing.Union[str, typing.Text]]
         """Path for chroot if set.
 
         :rtype: typing.Optional[typing.Text]
         .. versionadded:: 1.12.0
         """
         return self.__chroot_path
 
-    @chroot_path.setter
-    def chroot_path(self, new_state):  # type: (typing.Optional[typing.Union[str, typing.Text]]) -> None
+    @_chroot_path.setter
+    def _chroot_path(self, new_state):  # type: (typing.Optional[typing.Union[str, typing.Text]]) -> None
         """Path for chroot if set.
 
         :param new_state: new path
@@ -138,8 +136,8 @@ def chroot_path(self, new_state):  # type: (typing.Optional[typing.Union[str, ty
         """
         self.__chroot_path = new_state
 
-    @chroot_path.deleter
-    def chroot_path(self):  # type: () -> None
+    @_chroot_path.deleter
+    def _chroot_path(self):  # type: () -> None
         """Remove Path for chroot.
 
         .. versionadded:: 1.12.0
@@ -231,9 +229,9 @@ def _prepare_command(
         chroot_path=None  # type: typing.Optional[typing.Union[str, typing.Text]]
     ):  # type: (...) -> typing.Text
         """Prepare command: cower chroot and other cases."""
-        if any((chroot_path, self.chroot_path)):
+        if any((chroot_path, self._chroot_path)):
             return "chroot {chroot_path} {cmd}".format(
-                chroot_path=chroot_path if chroot_path else self.chroot_path,
+                chroot_path=chroot_path if chroot_path else self._chroot_path,
                 cmd=cmd
             )
         return cmd

exec_helpers/subprocess_runner.py

@@ -59,8 +59,7 @@ class Subprocess(six.with_metaclass(metaclasses.SingleLock, api.ExecHelper)):
 
     def __init__(
         self,
-        log_mask_re=None,  # type: typing.Optional[typing.Text]
-        chroot_path=None,  # type: typing.Optional[typing.Union[str, typing.Text]]
+        log_mask_re=None  # type: typing.Optional[typing.Text]
     ):  # type: (...) -> None
         """Subprocess helper with timeouts and lock-free FIFO.
 
@@ -69,8 +68,6 @@ def __init__(
         :param log_mask_re: regex lookup rule to mask command for logger.
                             all MATCHED groups will be replaced by '<*masked*>'
         :type log_mask_re: typing.Optional[str]
-        :param chroot_path: chroot path (use chroot if set)
-        :type chroot_path: typing.Optional[str]
 
         .. versionchanged:: 1.2.0 log_mask_re regex rule for masking cmd
         .. versionchanged:: 1.9.0 Not singleton anymore. Only lock is shared between all instances.

test/test_ssh_client_execute_async_special.py

@@ -260,20 +260,7 @@ def test_010_check_stdin_closed(paramiko_ssh_client, chan_makefile, auto_add_pol
     log.warning.assert_called_once_with("STDIN Send failed: closed channel")
 
 
-def test_011_execute_async_chroot(ssh, ssh_transport_channel):
-    """Global chroot path."""
-    ssh.chroot_path = "/"
-
-    ssh.execute_async(command)
-    ssh_transport_channel.assert_has_calls(
-        (
-            mock.call.makefile_stderr("rb"),
-            mock.call.exec_command('chroot {ssh.chroot_path} {command}\n'.format(ssh=ssh, command=command)),
-        )
-    )
-
-
-def test_012_execute_async_chroot_cmd(ssh, ssh_transport_channel):
+def test_011_execute_async_chroot_cmd(ssh, ssh_transport_channel):
     """Command-only chroot path."""
     ssh.execute_async(command, chroot_path='/')
     ssh_transport_channel.assert_has_calls(
@@ -284,7 +271,7 @@ def test_012_execute_async_chroot_cmd(ssh, ssh_transport_channel):
     )
 
 
-def test_013_execute_async_chroot_context(ssh, ssh_transport_channel):
+def test_012_execute_async_chroot_context(ssh, ssh_transport_channel):
     """Context-managed chroot path."""
     with ssh.chroot('/'):
         ssh.execute_async(command)
@@ -296,9 +283,9 @@ def test_013_execute_async_chroot_context(ssh, ssh_transport_channel):
     )
 
 
-def test_014_execute_async_no_chroot_context(ssh, ssh_transport_channel):
+def test_013_execute_async_no_chroot_context(ssh, ssh_transport_channel):
     """Context-managed chroot path override."""
-    ssh.chroot_path = "/"
+    ssh._chroot_path = "/"
 
     with ssh.chroot(None):
         ssh.execute_async(command)