This is an automated email from the ASF dual-hosted git repository.
pabloem pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/beam.git
The following commit(s) were added to refs/heads/master by this push:
new 04db8f5 [BEAM-3072] Python command error handling improvements (#8158)
04db8f5 is described below
commit 04db8f58e42a05ea6ebafb331bad041422698def
Author: Niklas Hansson <niklas.sven.hans...@gmail.com>
AuthorDate: Mon Apr 15 23:41:41 2019 +0200
[BEAM-3072] Python command error handling improvements (#8158)
* [BEAM-3072] updates to that the error handling and collected the failed
message. Updated to catch both if the command is not present and if there are
some arguemtns that are wrong
* [BEAM-3072] updates to that the error handling and collected the failed
message. Updated to catch both if the command is not present and if there are
some arguemtns that are wrong
* [BEAM-3059] fixed the failing tests. However it is not clear how to
collect the errors from subprocess.Open() since it is running in a new process
* [BEAM-3072] fixed formatting isssue that failed build test
* [BEAM-3072] commited some echo lines by misstake in last commit
* [BEAM-3072] updated the error handling futher to clearly state which
python package that was not installed succesfully
* [BEAM-3072] tmp commit to check master
* [BEAM-3072] added test for the processing updates concerning the error
handling and returning better error messages
* [BEAM-3072] updated in order for the tests to be compatible with python3
* [BEAM-3072] fixed more with the test that i foregot last time
* [BEAM-3072] fixed wrong indentation
---
sdks/python/apache_beam/utils/processes.py | 51 +++++++++-
sdks/python/apache_beam/utils/processes_test.py | 120 ++++++++++++++++++++++++
2 files changed, 167 insertions(+), 4 deletions(-)
diff --git a/sdks/python/apache_beam/utils/processes.py
b/sdks/python/apache_beam/utils/processes.py
index ccc0aa4..cfd82bd 100644
--- a/sdks/python/apache_beam/utils/processes.py
+++ b/sdks/python/apache_beam/utils/processes.py
@@ -24,6 +24,7 @@ from __future__ import absolute_import
import platform
import subprocess
+import traceback
# On Windows, we need to use shell=True when creating subprocesses for binary
# paths to be resolved correctly.
@@ -38,22 +39,64 @@ CalledProcessError = subprocess.CalledProcessError
def call(*args, **kwargs):
if force_shell:
kwargs['shell'] = True
- return subprocess.call(*args, **kwargs)
+ try:
+ out = subprocess.call(*args, **kwargs)
+ except OSError:
+ raise RuntimeError("Executable {} not found".format(args[0]))
+ except subprocess.CalledProcessError as error:
+ if isinstance(args, tuple) and (args[0][2] == "pip"):
+ raise RuntimeError( \
+ "Full traceback: {}\n Pip install failed for package: {} \
+ \n Output from execution of subprocess: {}" \
+ .format(traceback.format_exc(), args[0][6], error. output))
+ else:
+ raise RuntimeError("Full trace: {}\
+ \n Output of the failed child process: {} " \
+ .format(traceback.format_exc(), error.output))
+ return out
def check_call(*args, **kwargs):
if force_shell:
kwargs['shell'] = True
- return subprocess.check_call(*args, **kwargs)
+ try:
+ out = subprocess.check_call(*args, **kwargs)
+ except OSError:
+ raise RuntimeError("Executable {} not found".format(args[0]))
+ except subprocess.CalledProcessError as error:
+ if isinstance(args, tuple) and (args[0][2] == "pip"):
+ raise RuntimeError( \
+ "Full traceback: {} \n Pip install failed for package: {} \
+ \n Output from execution of subprocess: {}" \
+ .format(traceback.format_exc(), args[0][6], error.output))
+ else:
+ raise RuntimeError("Full trace: {} \
+ \n Output of the failed child process: {}" \
+ .format(traceback.format_exc(), error.output))
+ return out
def check_output(*args, **kwargs):
if force_shell:
kwargs['shell'] = True
- return subprocess.check_output(*args, **kwargs)
+ try:
+ out = subprocess.check_output(*args, **kwargs)
+ except OSError:
+ raise RuntimeError("Executable {} not found".format(args[0]))
+ except subprocess.CalledProcessError as error:
+ if isinstance(args, tuple) and (args[0][2] == "pip"):
+ raise RuntimeError( \
+ "Full traceback: {} \n Pip install failed for package: {} \
+ \n Output from execution of subprocess: {}" \
+ .format(traceback.format_exc(), args[0][6], error.output))
+ else:
+ raise RuntimeError("Full trace: {}, \
+ output of the failed child process {} "\
+ .format(traceback.format_exc(), error.output))
+ return out
-def Popen(*args, **kwargs): # pylint: disable=invalid-name
+def Popen(*args, **kwargs):
if force_shell:
kwargs['shell'] = True
return subprocess.Popen(*args, **kwargs)
diff --git a/sdks/python/apache_beam/utils/processes_test.py
b/sdks/python/apache_beam/utils/processes_test.py
index 123c124..ef5c219 100644
--- a/sdks/python/apache_beam/utils/processes_test.py
+++ b/sdks/python/apache_beam/utils/processes_test.py
@@ -18,6 +18,7 @@
from __future__ import absolute_import
+import subprocess
import unittest
import mock
@@ -103,5 +104,124 @@ class Exec(unittest.TestCase):
other_arg=True)
+class TestErrorHandlingCheckCall(unittest.TestCase):
+ @classmethod
+ def setup_class(cls):
+ cls.mock_get_patcher = mock.patch(\
+ 'apache_beam.utils.processes.subprocess.check_call')
+ cls.mock_get = cls.mock_get_patcher.start()
+
+ @classmethod
+ def teardown_class(cls):
+ cls.mock_get_patcher.stop()
+
+ def test_oserror_check_call(self):
+ # Configure the mock to return a response with an OK status code.
+ self.mock_get.side_effect = OSError("Test OSError")
+ with self.assertRaises(RuntimeError):
+ processes.check_call(["lls"])
+
+ def test_oserror_check_call_message(self):
+ # Configure the mock to return a response with an OK status code.
+ self.mock_get.side_effect = OSError()
+ cmd = ["lls"]
+ try:
+ processes.check_call(cmd)
+ except RuntimeError as error:
+ self.assertIn('Executable {} not found'.format(str(cmd)),\
+ error.args[0])
+
+ def test_check_call_pip_install_non_existing_package(self):
+ returncode = 1
+ package = "non-exsisting-package"
+ cmd = ['python', '-m', 'pip', 'download', '--dest', '/var',\
+ '{}'.format(package),\
+ '--no-deps', '--no-binary', ':all:']
+ output = "Collecting {}".format(package)
+ self.mock_get.side_effect = subprocess.CalledProcessError(returncode,\
+ cmd, output=output)
+ try:
+ output = processes.check_call(cmd)
+ self.fail("The test failed due to that\
+ no error was raised when calling process.check_call")
+ except RuntimeError as error:
+ self.assertIn("Output from execution of subprocess: {}".format(output),\
+ error.args[0])
+ self.assertIn("Pip install failed for package: {}".format(package),\
+ error.args[0])
+
+
+class TestErrorHandlingCheckOutput(unittest.TestCase):
+ @classmethod
+ def setup_class(cls):
+ cls.mock_get_patcher = mock.patch(\
+ 'apache_beam.utils.processes.subprocess.check_output')
+ cls.mock_get = cls.mock_get_patcher.start()
+
+ def test_oserror_check_output_message(self):
+ self.mock_get.side_effect = OSError()
+ cmd = ["lls"]
+ try:
+ processes.check_output(cmd)
+ except RuntimeError as error:
+ self.assertIn('Executable {} not found'.format(str(cmd)),\
+ error.args[0])
+
+ def test_check_output_pip_install_non_existing_package(self):
+ returncode = 1
+ package = "non-exsisting-package"
+ cmd = ['python', '-m', 'pip', 'download', '--dest', '/var',\
+ '{}'.format(package),\
+ '--no-deps', '--no-binary', ':all:']
+ output = "Collecting {}".format(package)
+ self.mock_get.side_effect = subprocess.CalledProcessError(returncode,\
+ cmd, output=output)
+ try:
+ output = processes.check_output(cmd)
+ self.fail("The test failed due to that\
+ no error was raised when calling process.check_call")
+ except RuntimeError as error:
+ self.assertIn("Output from execution of subprocess: {}".format(output),\
+ error.args[0])
+ self.assertIn("Pip install failed for package: {}".format(package),\
+ error.args[0])
+
+
+class TestErrorHandlingCall(unittest.TestCase):
+ @classmethod
+ def setup_class(cls):
+ cls.mock_get_patcher = mock.patch(\
+ 'apache_beam.utils.processes.subprocess.call')
+ cls.mock_get = cls.mock_get_patcher.start()
+
+ def test_oserror_check_output_message(self):
+ self.mock_get.side_effect = OSError()
+ cmd = ["lls"]
+ try:
+ processes.call(cmd)
+ except RuntimeError as error:
+ self.assertIn('Executable {} not found'.format(str(cmd)),\
+ error.args[0])
+
+ def test_check_output_pip_install_non_existing_package(self):
+ returncode = 1
+ package = "non-exsisting-package"
+ cmd = ['python', '-m', 'pip', 'download', '--dest', '/var',\
+ '{}'.format(package),\
+ '--no-deps', '--no-binary', ':all:']
+ output = "Collecting {}".format(package)
+ self.mock_get.side_effect = subprocess.CalledProcessError(returncode,\
+ cmd, output=output)
+ try:
+ output = processes.call(cmd)
+ self.fail("The test failed due to that\
+ no error was raised when calling process.check_call")
+ except RuntimeError as error:
+ self.assertIn("Output from execution of subprocess: {}".format(output),\
+ error.args[0])
+ self.assertIn("Pip install failed for package: {}".format(package),\
+ error.args[0])
+
+
if __name__ == '__main__':
unittest.main()
