Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: Add raise_error option to run and run_and_log methods in process.py #206

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
20 changes: 17 additions & 3 deletions meltano/edk/process.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ def run(
stdout: None | int | t.IO = subprocess.PIPE,
stderr: None | int | t.IO = subprocess.PIPE,
text: bool = True,
raise_error: bool = True,
**kwargs: t.Any,
) -> subprocess.CompletedProcess:
"""Run a subprocess. Simple wrapper around subprocess.run.
Expand All @@ -77,14 +78,16 @@ def run(
to override these you're likely better served using `subprocess.run` directly.

Lastly note that this method is blocking AND `subprocess.run` is called with
`check=True`. This means that if the subprocess fails a `CalledProcessError`
will be raised.
`check=True` and `raise_error=True`. This means that if the subprocess
fails a `CalledProcessError` will be raised.
Comment on lines +81 to +82
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you confirm if raise_error isn't doing anything at the moment?

Perhaps the same argument name check should be used for this method and just pass it directly to subprocess.run(..., check=check).


Args:
*args: The arguments to pass to the subprocess.
stdout: The stdout stream to use.
stderr: The stderr stream to use.
text: If true, decode stdin, stdout and stderr using the system default.
raise_error: If true, decode stdin, stdout and stderr using the
system default.
**kwargs: Additional keyword arguments to pass to subprocess.run.

Returns:
Expand Down Expand Up @@ -167,6 +170,7 @@ async def _exec(
def run_and_log(
self,
sub_command: str | None = None,
raise_error: bool = True,
*args: ExecArg,
) -> None:
"""Run a subprocess and stream the output to the logger.
Expand All @@ -176,13 +180,23 @@ def run_and_log(

Args:
sub_command: The subcommand to run.
raise_error: If True (default), raises a CalledProcessError if
the subprocess fails.
*args: The arguments to pass to the subprocess.

Raises:
CalledProcessError: If the subprocess failed.
"""
result = asyncio.run(self._exec(sub_command, *args))
if result.returncode:
if result.returncode and raise_error:
raise subprocess.CalledProcessError(
result.returncode, cmd=self.bin, stderr=None
)
elif result.returncode and not raise_error:
log_subprocess_error(
self.bin,
subprocess.CalledProcessError(
result.returncode, cmd=self.bin, stderr=None
),
"Error running subprocess",
)
Loading