Skip to content

Commit

Permalink
Clarify output handling in sshrun
Browse files Browse the repository at this point in the history
Make it clear in the code, that and why we don't expect any captured
output from the SSH subprocess. So, future changes (and debuggers)
don't falsely assume that
1. Output can simply be captured (with existing protocols) or
2. The returned value would currently be of any use simply b/c it's
   there.
  • Loading branch information
bpoldrack committed Oct 7, 2022
1 parent 4d26f92 commit 2231ba2
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 3 deletions.
4 changes: 3 additions & 1 deletion datalad/support/sshconnector.py
Expand Up @@ -174,11 +174,13 @@ def __call__(self, cmd, options=None, stdin=None, log_output=True):
(probably most) of the available configuration options should not be
set here because they can critically change the properties of the
connection. This exists to allow options like SendEnv to be set.
log_output: bool
Whether to capture and return stdout+stderr.
Returns
-------
tuple of str
stdout, stderr of the command run.
stdout, stderr of the command run, if `log_output` was `True`
"""
raise NotImplementedError

Expand Down
7 changes: 5 additions & 2 deletions datalad/support/sshrun.py
Expand Up @@ -116,8 +116,11 @@ def __call__(login, cmd,
# use an empty temp file as stdin if none shall be connected
stdin_ = tempfile.TemporaryFile() if no_stdin else sys.stdin
try:
out, err = ssh(cmd, stdin=stdin_, log_output=False,
options=options)
# We pipe the SSH process' stdout/stderr by means of
# `log_output=False`. That's necessary to let callers - for example
# git-clone - communicate with the SSH process. Hence, we expect no
# output being returned from this call:
ssh(cmd, stdin=stdin_, log_output=False, options=options)
finally:
if no_stdin:
stdin_.close()

0 comments on commit 2231ba2

Please sign in to comment.