From 9ed39433d9c381697f1f680ea2f310091e0b00f0 Mon Sep 17 00:00:00 2001 From: Jon Clucas Date: Thu, 21 May 2020 17:03:15 -0400 Subject: [PATCH 1/5] :children_crossing: Add stderr to generator when streaming --- spython/utils/terminal.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/spython/utils/terminal.py b/spython/utils/terminal.py index 9331ee87..9a4ec507 100644 --- a/spython/utils/terminal.py +++ b/spython/utils/terminal.py @@ -134,7 +134,12 @@ def stream_command(cmd, no_newline_regexp="Progess", sudo=False, sudo_options=No """ cmd = _process_sudo_cmd(cmd, sudo, sudo_options) - process = subprocess.Popen(cmd, stdout=subprocess.PIPE, universal_newlines=True) + process = subprocess.Popen( + cmd, + stdout=subprocess.PIPE, + stderr=subprocess.STDOUT, + universal_newlines=True + ) for line in iter(process.stdout.readline, ""): if not re.search(no_newline_regexp, line): yield line From 73060fa8dc243a14de8a64396437a671599e0bbd Mon Sep 17 00:00:00 2001 From: Jon Clucas Date: Thu, 21 May 2020 18:09:38 -0400 Subject: [PATCH 2/5] :shirt: Use Black to lint --- spython/utils/terminal.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/spython/utils/terminal.py b/spython/utils/terminal.py index 9a4ec507..5681823c 100644 --- a/spython/utils/terminal.py +++ b/spython/utils/terminal.py @@ -135,10 +135,7 @@ def stream_command(cmd, no_newline_regexp="Progess", sudo=False, sudo_options=No cmd = _process_sudo_cmd(cmd, sudo, sudo_options) process = subprocess.Popen( - cmd, - stdout=subprocess.PIPE, - stderr=subprocess.STDOUT, - universal_newlines=True + cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, universal_newlines=True ) for line in iter(process.stdout.readline, ""): if not re.search(no_newline_regexp, line): From 864a34e0248a7faa0173d74f48573005e60a18a6 Mon Sep 17 00:00:00 2001 From: Jon Clucas Date: Fri, 22 May 2020 10:29:33 -0400 Subject: [PATCH 3/5] :ok_hand: Print stderr iff CalledProcess Error :microscope: Add tests Co-authored-by: vsoch --- spython/tests/test_client.py | 34 ++++++++++++++++++++++++++++++++++ spython/utils/terminal.py | 3 ++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/spython/tests/test_client.py b/spython/tests/test_client.py index 844960f1..a39880e9 100644 --- a/spython/tests/test_client.py +++ b/spython/tests/test_client.py @@ -7,8 +7,11 @@ # with this file, You can obtain one at http://mozilla.org/MPL/2.0/. from spython.main import Client +from spython.utils import write_file import shutil import os +import pytest +from subprocess import CalledProcessError def test_build_from_docker(tmp_path): @@ -50,6 +53,37 @@ def test_execute_with_return_code(docker_container): assert result["return_code"] == 0 +@pytest.mark.parametrize("return_code", [True, False]) +def test_execute_with_called_process_error( + capsys, docker_container, return_code, tmp_path +): + tmp_file = os.path.join(tmp_path, "CalledProcessError.sh") + # "This is stdout" to stdout, "This is stderr" to stderr + script = f"""#!/bin/bash +echo "This is stdout" +>&2 echo "This is stderr" +{"exit 1" if return_code else ""} +""" + write_file(tmp_file, script) + if return_code: + with pytest.raises(CalledProcessError): + for line in Client.execute( + docker_container[1], f"/bin/sh {tmp_file}", stream=True + ): + print(line, "") + else: + for line in Client.execute( + docker_container[1], f"/bin/sh {tmp_file}", stream=True + ): + print(line, "") + captured = capsys.readouterr() + assert "stdout" in captured.out + if return_code: + assert "stderr" in captured.out + else: + assert "stderr" not in captured.out + + def test_inspect(docker_container): result = Client.inspect(docker_container[1]) assert "attributes" in result or "data" in result diff --git a/spython/utils/terminal.py b/spython/utils/terminal.py index 5681823c..943f4c7e 100644 --- a/spython/utils/terminal.py +++ b/spython/utils/terminal.py @@ -135,7 +135,7 @@ def stream_command(cmd, no_newline_regexp="Progess", sudo=False, sudo_options=No cmd = _process_sudo_cmd(cmd, sudo, sudo_options) process = subprocess.Popen( - cmd, stdout=subprocess.PIPE, stderr=subprocess.STDOUT, universal_newlines=True + cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True ) for line in iter(process.stdout.readline, ""): if not re.search(no_newline_regexp, line): @@ -143,6 +143,7 @@ def stream_command(cmd, no_newline_regexp="Progess", sudo=False, sudo_options=No process.stdout.close() return_code = process.wait() if return_code: + print(process.stderr.read()) raise subprocess.CalledProcessError(return_code, cmd) From a57cc172bcc46cdb52e0ecc4318d32b134cac4ea Mon Sep 17 00:00:00 2001 From: Jon Clucas Date: Fri, 22 May 2020 10:45:19 -0400 Subject: [PATCH 4/5] :children_crossing: Move subprocess stderr print from sys.stdout to sys.stderr --- spython/tests/test_client.py | 4 ++-- spython/utils/terminal.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spython/tests/test_client.py b/spython/tests/test_client.py index a39880e9..61d0ee96 100644 --- a/spython/tests/test_client.py +++ b/spython/tests/test_client.py @@ -79,9 +79,9 @@ def test_execute_with_called_process_error( captured = capsys.readouterr() assert "stdout" in captured.out if return_code: - assert "stderr" in captured.out + assert "stderr" in captured.err else: - assert "stderr" not in captured.out + assert "stderr" not in captured.err def test_inspect(docker_container): diff --git a/spython/utils/terminal.py b/spython/utils/terminal.py index 943f4c7e..2856a680 100644 --- a/spython/utils/terminal.py +++ b/spython/utils/terminal.py @@ -143,7 +143,7 @@ def stream_command(cmd, no_newline_regexp="Progess", sudo=False, sudo_options=No process.stdout.close() return_code = process.wait() if return_code: - print(process.stderr.read()) + print(process.stderr.read(), file=sys.stderr) raise subprocess.CalledProcessError(return_code, cmd) From 0475e88c5674f61289ef4fd6ca46c2ad8a9cbbd3 Mon Sep 17 00:00:00 2001 From: vsoch Date: Fri, 22 May 2020 11:17:51 -0600 Subject: [PATCH 5/5] bumping version and updating CHANGELOG to finalize PR Signed-off-by: vsoch --- CHANGELOG.md | 1 + spython/version.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1041d84d..3baeaf1b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ The client here will eventually be released as "spython" (and eventually to singularity on pypi), and the versions here will coincide with these releases. ## [master](https://github.com/singularityhub/singularity-cli/tree/master) + - stream command should print to stdout given CalledProcessError (0.0.81) - USER regular expression should check for USER at start of line (0.0.80) - add singularity options parameters to send to singularity (0.0.79) - add support for library:// urls (0.0.78) diff --git a/spython/version.py b/spython/version.py index 49611ad8..d52fddd7 100644 --- a/spython/version.py +++ b/spython/version.py @@ -5,7 +5,7 @@ # with this file, You can obtain one at http://mozilla.org/MPL/2.0/. -__version__ = "0.0.80" +__version__ = "0.0.81" AUTHOR = "Vanessa Sochat" AUTHOR_EMAIL = "vsochat@stanford.edu" NAME = "spython"