Skip to content

Commit

Permalink
Added warning for command list and shell true (flyteorg#2653)
Browse files Browse the repository at this point in the history
Signed-off-by: mao3267 <chenvincent610@gmail.com>
  • Loading branch information
pryce-turner authored and mao3267 committed Aug 9, 2024
1 parent ffc030e commit ac8563d
Showing 1 changed file with 15 additions and 6 deletions.
21 changes: 15 additions & 6 deletions flytekit/extras/tasks/shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,26 @@ def subproc_execute(command: typing.Union[List[str], str], **kwargs) -> ProcessR

kwargs = {**defaults, **kwargs}

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"):
if kwargs.get("shell"):
if "|" in command:
logger.warning(
"""Found a pipe in the command and shell=True.
This can lead to silent failures if subsequent commands
succeed despite previous failures."""
)
if type(command) == list:
logger.warning(
"""Found `command` formatted as a list instead of a string with shell=True.
With this configuration, the first member of the list will be
executed and the remaining arguments will be passed as arguments
to the shell instead of to the binary being called. This may not
be intended behavior and may lead to confusing failures."""
)

try:
# Execute the command and capture stdout and stderr
result = subprocess.run(command, **kwargs)
result.check_returncode()

# Access the stdout and stderr output
return ProcessResult(result.returncode, result.stdout, result.stderr)
Expand Down

0 comments on commit ac8563d

Please sign in to comment.