Skip to content

Commit

Permalink
Patch to enable SSH-based remote command executor for windows clients
Browse files Browse the repository at this point in the history
This patch aims to fix a hanging Python sessions after the execution
of an SSH remote command call with no particular stdin input.

Interpretation from #68

    Without overriding stdin, the subprocess, i.e. ssh, and python share
    the same file pointer. It seems that stdin is configured in a way that
    unexpected by the interpreter and messes with python's way to read from
    sys.stdin.

This patch passes an explicit `b''` as `stdin` to the SSH client
execution process to effectively achieve a separate fiel descriptor for
that client process.

This patch should not interfere with the implementation of the `sshrun`
command in datalad-core. It uses a dedicated not-None value for any
execution. However, the compatibility and interference of this patch
should be subject to a thorough investigation and widespread testing
before this changeset is proposed for datalad-core.

Closes #68
  • Loading branch information
mih committed Sep 18, 2023
1 parent 3fb2307 commit 88401a6
Show file tree
Hide file tree
Showing 4 changed files with 66 additions and 0 deletions.
3 changes: 3 additions & 0 deletions datalad_ria/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
]
)

# patch datalad-core
import datalad_ria.patches.enabled

from ._version import get_versions
__version__ = get_versions()['version']
del get_versions
3 changes: 3 additions & 0 deletions datalad_ria/patches/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import logging

lgr = logging.getLogger('datalad.ext.ria.patches')
3 changes: 3 additions & 0 deletions datalad_ria/patches/enabled.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
from . import (
ssh_exec,
)
57 changes: 57 additions & 0 deletions datalad_ria/patches/ssh_exec.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
"""
With this patch ...
"""

import logging

from datalad.support.sshconnector import (
StdOutErrCapture,
NoCapture,
)

from datalad_next.patches import apply_patch
from datalad_next.utils import on_windows

# use same logger as -core
lgr = logging.getLogger('datalad.support.sshconnector')


# This method interface/original implementation is taken from
# datalad-core@58b8e06317fe1a03290aed80526bff1e2d5b7797
# datalad/support/sshconnector.py:BaseSSHConnection
def _exec_ssh(self, ssh_cmd, cmd, options=None, stdin=None, log_output=True):
cmd = self._adjust_cmd_for_bundle_execution(cmd)

for opt in options or []:
ssh_cmd.extend(["-o", opt])

# THIS IS THE PATCH
if on_windows and stdin is None:
# SSH on windows requires a special stdin handling. If we'd let
# stdin=None do its normal thing, the Python process would hang,
# because it looses touch with its own file descriptor.
# See https://github.com/datalad/datalad-ria/issues/68
stdin = b''

# build SSH call, feed remote command as a single last argument
# whatever it contains will go to the remote machine for execution
# we cannot perform any sort of escaping, because it will limit
# what we can do on the remote, e.g. concatenate commands with '&&'
ssh_cmd += [self.sshri.as_str()] + [cmd]

lgr.debug("%s is used to run %s", self, ssh_cmd)

# TODO: pass expect parameters from above?
# Hard to explain to toplevel users ... So for now, just set True
out = self.runner.run(
ssh_cmd,
protocol=StdOutErrCapture if log_output else NoCapture,
stdin=stdin)
return out['stdout'], out['stderr']


apply_patch(
'datalad.support.sshconnector', 'BaseSSHConnection', '_exec_ssh',
_exec_ssh,
)

0 comments on commit 88401a6

Please sign in to comment.