Add Connection.shell()

fabric/__init__.py

@@ -1,7 +1,7 @@
 # flake8: noqa
 from ._version import __version_info__, __version__
 from .connection import Config, Connection
-from .runners import Remote, Result
+from .runners import Remote, RemoteShell, Result
 from .group import Group, SerialGroup, ThreadingGroup, GroupResult
 from .tasks import task, Task
 from .executor import Executor

fabric/config.py

@@ -5,7 +5,7 @@
 from invoke.config import Config as InvokeConfig, merge_dicts
 from paramiko.config import SSHConfig
 
-from .runners import Remote
+from .runners import Remote, RemoteShell
 from .util import get_local_user, debug
 
 
@@ -308,7 +308,7 @@ def global_defaults():
             "inline_ssh_env": False,
             "load_ssh_configs": True,
             "port": 22,
-            "runners": {"remote": Remote},
+            "runners": {"remote": Remote, "remote_shell": RemoteShell},
             "ssh_config_path": None,
             "tasks": {"collection_name": "fabfile"},
             # TODO: this becomes an override/extend once Invoke grows execution

fabric/connection.py

@@ -738,6 +738,85 @@ def sudo(self, command, **kwargs):
         """
         return self._sudo(self._remote_runner(), command, **kwargs)
 
+    @opens
+    def shell(self, **kwargs):
+        """
+        Run an interactive login shell on the remote end, as with ``ssh``.
+
+        This method is intended strictly for use cases where you can't know
+        what remote shell to invoke, or are connecting to a non-POSIX-server
+        environment such as a network appliance or other custom SSH server.
+        Nearly every other use case, including interactively-focused ones, will
+        be better served by using `run` plus an explicit remote shell command
+        (eg ``bash``).
+
+        `shell` has the following differences in behavior from `run`:
+
+        - It still returns a `~invoke.runners.Result` instance, but the object
+          will have a less useful set of attributes than with `run` or `local`:
+
+            - ``command`` will be ``None``, as there is no such input argument.
+            - ``stdout`` will contain a full record of the session, including
+              all interactive input, as that is echoed back to the user. This
+              can be useful for logging but is much less so for doing
+              programmatic things after the method returns.
+            - ``stderr`` will always be empty (same as `run` when
+              ``pty==True``).
+            - ``pty`` will always be True (because one was automatically used).
+            - ``exited`` and similar attributes will only reflect the overall
+              session, which may vary by shell or appliance but often has no
+              useful relationship with the internally executed commands' exit
+              codes.
+
+        - This method behaves as if ``warn`` is set to ``True``: even if the
+          remote shell exits uncleanly, no exception will be raised.
+        - A pty is always allocated remotely, as with ``pty=True`` under `run`.
+        - The ``inline_env`` setting is ignored, as there is no default shell
+          command to add the parameters to (and no guarantee the remote end
+          even is a shell!)
+
+        It supports **only** the following kwargs, which behave identically to
+        their counterparts in `run` unless otherwise stated:
+
+        - ``encoding``
+        - ``env``
+        - ``in_stream`` (useful in niche cases, but make sure regular `run`
+          with this argument isn't more suitable!)
+        - ``replace_env``
+        - ``watchers`` (note that due to pty echoing your stdin back to stdout,
+          a watcher will see your input as well as program stdout!)
+
+        Those keyword arguments also honor the ``run.*`` configuration tree, as
+        in `run`/`sudo`.
+
+        :returns: `Result`
+
+        :raises:
+            `.ThreadException` (if the background I/O threads encountered
+            exceptions other than `.WatcherError`).
+
+        .. versionadded:: 2.7
+        """
+        runner = self.config.runners.remote_shell(context=self)
+        # Reinstate most defaults as explicit kwargs to ensure user's config
+        # doesn't make this mode break horribly. Then override a few that need
+        # to change, like pty.
+        allowed = ("encoding", "env", "in_stream", "replace_env", "watchers")
+        new_kwargs = {}
+        for key, value in self.config.global_defaults()["run"].items():
+            if key in allowed:
+                # Use allowed kwargs if given, otherwise also fill them from
+                # defaults
+                new_kwargs[key] = kwargs.pop(key, self.config.run[key])
+            else:
+                new_kwargs[key] = value
+        new_kwargs.update(pty=True)
+        # At this point, any leftover kwargs would be ignored, so yell instead
+        if kwargs:
+            err = "shell() got unexpected keyword arguments: {!r}"
+            raise TypeError(err.format(list(kwargs.keys())))
+        return runner.run(command=None, **new_kwargs)
+
     def local(self, *args, **kwargs):
         """
         Execute a shell command on the local system.

fabric/runners.py

@@ -62,6 +62,9 @@ def start(self, command, shell, env, timeout=None):
                 command = "export {} && {}".format(parameters, command)
             else:
                 self.channel.update_environment(env)
+        self.send_start_message(command)
+
+    def send_start_message(self, command):
         self.channel.exec_command(command)
 
     def run(self, command, **kwargs):
@@ -147,6 +150,11 @@ def handle_window_change(self, signum, frame):
     # * agent-forward close()
 
 
+class RemoteShell(Remote):
+    def send_start_message(self, command):
+        self.channel.invoke_shell()
+
+
 class Result(InvokeResult):
     """
     An `invoke.runners.Result` exposing which `.Connection` was run against.

fabric/testing/base.py

@@ -67,6 +67,25 @@ def __repr__(self):
         # grow more dependencies? Ehhh
         return "<{} cmd={!r}>".format(self.__class__.__name__, self.cmd)
 
+    def expect_execution(self, channel):
+        """
+        Assert that the ``channel`` was used to run this command.
+
+        .. versionadded:: 2.7
+        """
+        channel.exec_command.assert_called_with(self.cmd or ANY)
+
+
+class ShellCommand(Command):
+    """
+    A pseudo-command that expects an interactive shell to be executed.
+
+    .. versionadded:: 2.7
+    """
+
+    def expect_execution(self, channel):
+        channel.invoke_shell.assert_called_once_with()
+
 
 class MockChannel(Mock):
     """
@@ -254,8 +273,8 @@ def sanity_check(self):
         for channel, command in zip(self.channels, self.commands):
             # Expect an open_session for each command exec
             session_opens.append(call())
-            # Expect that the channel gets an exec_command
-            channel.exec_command.assert_called_with(command.cmd or ANY)
+            # Expect that the channel gets an exec_command or etc
+            command.expect_execution(channel=channel)
             # Expect written stdin, if given
             if command.in_:
                 assert channel._stdin.getvalue() == command.in_

integration/connection.py

@@ -1,8 +1,14 @@
 import os
 import time
 
+try:
+    from invoke.vendor.six import StringIO
+except ImportError:
+    from six import StringIO
+
 from invoke import pty_size, CommandTimedOut
 from pytest import skip, raises
+from pytest_relaxed import trap
 
 from fabric import Connection, Config
 
@@ -56,6 +62,20 @@ def simple_command_with_pty(self):
             assert "\r\n" in result.stdout
             assert result.pty is True
 
+    class shell:
+        @trap
+        def base_case(self):
+            result = Connection("localhost").shell(
+                in_stream=StringIO("exit\n")
+            )
+            assert result.command is None
+            # Will also include any shell prompt, etc but just looking for the
+            # mirrored input is most test-env-agnostic way to spot check...
+            assert "exit" in result.stdout
+            assert result.stderr == ""
+            assert result.exited == 0
+            assert result.pty is True
+
     class local:
         def wraps_invoke_run(self):
             # NOTE: most of the interesting tests about this are in

sites/www/changelog.rst

@@ -5,6 +5,20 @@ Changelog
 .. note::
     Looking for the Fabric 1.x changelog? See :doc:`/changelog-v1`.
 
+- :feature:`-` Add `~fabric.connection.Connection.shell`, a belated port of
+  the v1 ``open_shell()`` feature.
+
+  - This wasn't needed initially, as the modern implementation of
+    `~fabric.connection.Connection.run` is as good or better for full
+    interaction than ``open_shell()`` was, provided you're happy supplying a
+    specific shell to execute.
+  - `~fabric.connection.Connection.shell` serves the corner case where you
+    *aren't* happy doing that, eg when you're speaking to network appliances or
+    other targets which are not typical Unix server environments.
+  - Like ``open_shell()``, this new method is primarily for interactive use,
+    and has a slightly less useful return value. See its API docs for more
+    details.
+
 - :feature:`-` Forward local terminal resizes to the remote end, when
   applicable. (For the technical: this means we now turn ``SIGWINCH`` into SSH
   ``window-change`` messages.)

sites/www/upgrading.rst

@@ -804,10 +804,10 @@ differences.
     * - ``open_shell`` for obtaining interactive-friendly remote shell sessions
         (something that ``run`` historically was bad at )
       - Ported
-      - Technically "removed", but only because the new version of
-        ``run`` is vastly improved and can deal with interactive sessions at
-        least as well as the old ``open_shell`` did, if not moreso.
-        ``c.run("/my/favorite/shell", pty=True)`` should be all you need.
+      - Not only is the new version of ``run`` vastly improved and able to deal
+        with interactive sessions at least as well as the old ``open_shell``
+        (provided you supply ``pty=True``), but for corner cases there's also a
+        direct port: `~fabric.connection.Connection.shell`.
 
 ``run``
 ~~~~~~~
@@ -878,6 +878,28 @@ See the 'general' notes at top of this section for most details about the new
         Behavior is much the same: no shell wrapping (as in legacy ``run``),
         just informing the operating system what actual program to run.
 
+``open_shell``
+~~~~~~~~~~~~~~
+
+As noted in the main list, this is now `~fabric.connection.Connection.shell`,
+and behaves similarly to ``open_shell`` (exit codes, if any, are ignored; a PTY
+is assumed; etc). It has some improvements too, such as a return value (which
+is slightly lacking compared to that from `~fabric.connection.Connection.run`
+but still a big improvement over ``None``).
+
+.. list-table::
+    :widths: 40 10 50
+
+    * - ``command`` optional kwarg allowing 'prefilling' the input stream with
+        a specific command string plus newline
+      - Removed
+      - If you needed this, you should instead try the modern version of
+        `~fabric.connection.Connection.run`, which is equally capable of
+        interaction as `~fabric.connection.Connection.shell` but takes a
+        command to execute. There's a small chance we'll add this back later if
+        anybody misses it (there's a few corner cases that could possibly want
+        it).
+
 .. _upgrading-utility:
 
 Utilities

tests/config.py

@@ -2,9 +2,10 @@
 from os.path import join, expanduser
 
 from paramiko.config import SSHConfig
+from invoke import Local
 from invoke.vendor.lexicon import Lexicon
 
-from fabric import Config
+from fabric import Config, Remote, RemoteShell
 from fabric.util import get_local_user
 
 from mock import patch, call
@@ -52,6 +53,12 @@ def overrides_some_Invoke_defaults(self):
         config = Config()
         assert config.tasks.collection_name == "fabfile"
 
+    def amends_Invoke_runners_map(self):
+        config = Config()
+        assert config.runners == dict(
+            remote=Remote, remote_shell=RemoteShell, local=Local
+        )
+
     def uses_Fabric_prefix(self):
         # NOTE: see also the integration-esque tests in tests/main.py; this
         # just tests the underlying data/attribute driving the behavior.