From 803a1288dfe320c1272fd05ff29deaa2f4e918db Mon Sep 17 00:00:00 2001 From: pryce-turner <31577879+pryce-turner@users.noreply.github.com> Date: Thu, 27 Jun 2024 10:19:43 -0700 Subject: [PATCH] Catch silent subproc_execute failures (#2404) * Made outfile ephemeral Signed-off-by: pryce-turner * Changed error handling to warn log for pipe with shell commands Signed-off-by: pryce-turner --------- Signed-off-by: pryce-turner Signed-off-by: bugra.gedik --- flytekit/extras/tasks/shell.py | 8 ++++++++ tests/flytekit/unit/extras/tasks/test_shell.py | 14 ++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/flytekit/extras/tasks/shell.py b/flytekit/extras/tasks/shell.py index 03f6b61ebc..ef9cd0c0e1 100644 --- a/flytekit/extras/tasks/shell.py +++ b/flytekit/extras/tasks/shell.py @@ -79,6 +79,14 @@ def subproc_execute(command: typing.Union[List[str], str], **kwargs) -> ProcessR try: # Execute the command and capture stdout and stderr result = subprocess.run(command, **kwargs) + print(result.check_returncode()) + + if "|" in command and kwargs.get("shell"): + logger.warning( + """Found a pipe in the command and shell=True. + This can lead to silent failures if subsequent commands + succeed despite previous failures.""" + ) # Access the stdout and stderr output return ProcessResult(result.returncode, result.stdout, result.stderr) diff --git a/tests/flytekit/unit/extras/tasks/test_shell.py b/tests/flytekit/unit/extras/tasks/test_shell.py index bb06f1a5dc..65a7a50e39 100644 --- a/tests/flytekit/unit/extras/tasks/test_shell.py +++ b/tests/flytekit/unit/extras/tasks/test_shell.py @@ -370,3 +370,17 @@ def test_subproc_execute_with_shell(): subproc_execute(cmd, shell=True) cont = open(opth).read() assert "hello" in cont + + +def test_subproc_execute_missing_dep(): + cmd = ["non-existent", "blah"] + with pytest.raises(Exception) as e: + subproc_execute(cmd) + assert "executable could not be found" in str(e.value) + + +def test_subproc_execute_error(): + cmd = ["ls", "--banana"] + with pytest.raises(Exception) as e: + subproc_execute(cmd) + assert "Failed with return code" in str(e.value)