Catch silent subproc_execute failures (#2404)

* Made outfile ephemeral Signed-off-by: pryce-turner <[email protected]> * Changed error handling to warn log for pipe with shell commands Signed-off-by: pryce-turner <[email protected]> --------- Signed-off-by: pryce-turner <[email protected]> Signed-off-by: Jan Fiedler <[email protected]>
flyteorg · Jul 25, 2024 · c9dd47f · c9dd47f
1 parent 58448c4
commit c9dd47f
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 0 deletions.
diff --git 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
@@ -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)