From 5fdf20ac93dae332c9601ac5ecc604b45f07e835 Mon Sep 17 00:00:00 2001 From: Aki Sasaki Date: Mon, 1 Feb 2021 17:25:08 -0800 Subject: [PATCH 1/5] surface -11 and -15 exit codes in run_command --- .../src/scriptworker_client/utils.py | 21 +++++++++-- scriptworker_client/tests/test_utils.py | 35 ++++++++++++------- 2 files changed, 42 insertions(+), 14 deletions(-) diff --git a/scriptworker_client/src/scriptworker_client/utils.py b/scriptworker_client/src/scriptworker_client/utils.py index f519a204d..c716b72ff 100644 --- a/scriptworker_client/src/scriptworker_client/utils.py +++ b/scriptworker_client/src/scriptworker_client/utils.py @@ -29,7 +29,7 @@ Union, ) from urllib.parse import unquote, urlparse -from scriptworker_client.exceptions import TaskError +from scriptworker_client.exceptions import ClientError, TaskError log = logging.getLogger(__name__) @@ -166,6 +166,13 @@ def get_log_filehandle(log_path=None): # run_command {{{1 +def _get_exception_kwargs(exception, exitcode, copy_exit_codes): + kwargs = {} + if issubclass(exception, ClientError) and exitcode in copy_exit_codes: + kwargs["exit_code"] = exitcode + return kwargs + + async def run_command( cmd, log_path=None, @@ -175,6 +182,7 @@ async def run_command( env=None, exception=None, expected_exit_codes=(0,), + copy_exit_codes=(-11, -15, 245, 241), # 245 == -11, 241 == -15 output_log_on_exception=False, ): """Run a command using ``asyncio.create_subprocess_exec``. @@ -205,6 +213,9 @@ async def run_command( expected_exit_codes (list, optional): the list of exit codes for a successful run. Only used if ``exception`` is not ``None``. Defaults to ``(0, )``. + copy_exit_codes (list, optional): the list of exit codes that we + set ``exit_code`` to if ``exception`` is an instance of + ``ClientError``. Defaults to ``(-11, -15, 245, 241)``. output_log_on_exception (bool, optional): log the output log if we're raising an exception. @@ -242,8 +253,14 @@ async def run_command( if output_log_on_exception: log_filehandle.seek(0) log_contents = log_filehandle.read() + exc_kwargs = _get_exception_kwargs(exception, exitcode, copy_exit_codes) raise exception( - "%s in %s exited %s!\n%s", log_cmd, cwd, exitcode, log_contents + "%s in %s exited %s!\n%s", + log_cmd, + cwd, + exitcode, + log_contents, + **exc_kwargs, ) log.info("%s in %s exited %d", log_cmd, cwd, exitcode) return exitcode diff --git a/scriptworker_client/tests/test_utils.py b/scriptworker_client/tests/test_utils.py index 975cf5dbd..aeca159f9 100644 --- a/scriptworker_client/tests/test_utils.py +++ b/scriptworker_client/tests/test_utils.py @@ -168,7 +168,7 @@ def test_get_log_filehandle(path, tmpdir): False, ), ( - ["bash", "-c", ">&2 echo bar && echo foo && exit 1"], + ["bash", "-c", ">&2 echo bar && echo foo && exit 3"], 1, ["foo\nbar\n", "bar\nfoo\n"], TaskError, @@ -177,8 +177,8 @@ def test_get_log_filehandle(path, tmpdir): True, ), ( - ["bash", "-c", ">&2 echo bar && echo foo && exit 1"], - 1, + ["bash", "-c", ">&2 echo bar && echo foo && exit -11"], + 245, ["foo\nbar\n", "bar\nfoo\n"], TaskError, True, @@ -189,7 +189,14 @@ def test_get_log_filehandle(path, tmpdir): ) @pytest.mark.asyncio async def test_run_command( - command, status, expected_log, exception, output_log, env, raises, tmpdir + command, + status, + expected_log, + exception, + output_log, + env, + raises, + tmpdir, ): """``run_command`` runs the expected command, logs its output, and exits with its exit status. If ``exception`` is set and we exit non-zero, we @@ -201,14 +208,18 @@ async def test_run_command( log_path = os.path.join(tmpdir, "log") if raises: with pytest.raises(exception): - await utils.run_command( - command, - log_path=log_path, - cwd=tmpdir, - env=env, - exception=exception, - output_log_on_exception=output_log, - ) + try: + await utils.run_command( + command, + log_path=log_path, + cwd=tmpdir, + env=env, + exception=exception, + output_log_on_exception=output_log, + ) + except exception as exc: + assert exc.exit_code == status + raise exc else: assert ( await utils.run_command( From fe7de23956d310572eabb15bd7ca739c56fc417a Mon Sep 17 00:00:00 2001 From: Aki Sasaki Date: Tue, 16 Feb 2021 15:19:11 -0800 Subject: [PATCH 2/5] no -11 or -15 --- scriptworker_client/src/scriptworker_client/client.py | 2 +- scriptworker_client/src/scriptworker_client/utils.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/scriptworker_client/src/scriptworker_client/client.py b/scriptworker_client/src/scriptworker_client/client.py index f62e3ff7c..21e5ad0ec 100644 --- a/scriptworker_client/src/scriptworker_client/client.py +++ b/scriptworker_client/src/scriptworker_client/client.py @@ -181,5 +181,5 @@ async def _handle_asyncio_loop(async_main, config, task): try: await async_main(config, task) except ClientError as exc: - log.exception("Failed to run async_main") + log.exception(f"Failed to run async_main; exiting {exc.exit_code}") sys.exit(exc.exit_code) diff --git a/scriptworker_client/src/scriptworker_client/utils.py b/scriptworker_client/src/scriptworker_client/utils.py index c716b72ff..a3edb5ce0 100644 --- a/scriptworker_client/src/scriptworker_client/utils.py +++ b/scriptworker_client/src/scriptworker_client/utils.py @@ -182,7 +182,7 @@ async def run_command( env=None, exception=None, expected_exit_codes=(0,), - copy_exit_codes=(-11, -15, 245, 241), # 245 == -11, 241 == -15 + copy_exit_codes=(245, 241), # 245 == -11, 241 == -15 output_log_on_exception=False, ): """Run a command using ``asyncio.create_subprocess_exec``. @@ -215,7 +215,7 @@ async def run_command( Defaults to ``(0, )``. copy_exit_codes (list, optional): the list of exit codes that we set ``exit_code`` to if ``exception`` is an instance of - ``ClientError``. Defaults to ``(-11, -15, 245, 241)``. + ``ClientError``. Defaults to ``(245, 241)``. output_log_on_exception (bool, optional): log the output log if we're raising an exception. From 31f9410905f204c43e30e47433b915fda634bff0 Mon Sep 17 00:00:00 2001 From: Aki Sasaki Date: Tue, 16 Feb 2021 15:51:21 -0800 Subject: [PATCH 3/5] test fix --- scriptworker_client/tests/test_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scriptworker_client/tests/test_client.py b/scriptworker_client/tests/test_client.py index 480a59f69..d576c65b3 100644 --- a/scriptworker_client/tests/test_client.py +++ b/scriptworker_client/tests/test_client.py @@ -184,7 +184,7 @@ async def async_error(*args, **kwargs): await client._handle_asyncio_loop(async_error, {}, {}) assert excinfo.value.code == 42 - m.exception.assert_called_once_with("Failed to run async_main") + m.exception.assert_called_once_with("Failed to run async_main; exiting 42") def test_init_config_cli(mocker, tmpdir): From bfc31a1c6d4f4fe013351be0f8bc872e8200eb27 Mon Sep 17 00:00:00 2001 From: Aki Sasaki Date: Wed, 17 Feb 2021 09:35:47 -0800 Subject: [PATCH 4/5] address review comment --- scriptworker_client/src/scriptworker_client/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/scriptworker_client/src/scriptworker_client/utils.py b/scriptworker_client/src/scriptworker_client/utils.py index a3edb5ce0..fc5be872b 100644 --- a/scriptworker_client/src/scriptworker_client/utils.py +++ b/scriptworker_client/src/scriptworker_client/utils.py @@ -182,7 +182,9 @@ async def run_command( env=None, exception=None, expected_exit_codes=(0,), - copy_exit_codes=(245, 241), # 245 == -11, 241 == -15 + # https://www.gnu.org/software/bash/manual/html_node/Exit-Status.html + # Shell exit codes range from 0 to 255. Therefore 245 == -11, 241 == -15 + copy_exit_codes=(245, 241), output_log_on_exception=False, ): """Run a command using ``asyncio.create_subprocess_exec``. From 4d2a65a49c7f6e1c9c59a27124f22ec5b3bf53b9 Mon Sep 17 00:00:00 2001 From: Aki Sasaki Date: Wed, 17 Feb 2021 09:56:59 -0800 Subject: [PATCH 5/5] better link --- scriptworker_client/src/scriptworker_client/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scriptworker_client/src/scriptworker_client/utils.py b/scriptworker_client/src/scriptworker_client/utils.py index fc5be872b..579b2138c 100644 --- a/scriptworker_client/src/scriptworker_client/utils.py +++ b/scriptworker_client/src/scriptworker_client/utils.py @@ -182,7 +182,7 @@ async def run_command( env=None, exception=None, expected_exit_codes=(0,), - # https://www.gnu.org/software/bash/manual/html_node/Exit-Status.html + # https://stackoverflow.com/questions/18731791/determining-if-a-python-subprocess-segmentation-faults # Shell exit codes range from 0 to 255. Therefore 245 == -11, 241 == -15 copy_exit_codes=(245, 241), output_log_on_exception=False,