Add run_process(), a modern replacement for execute().
Review Request #12564 — Created Aug. 23, 2022 and submitted
Our
execute()
function, used for calling out into other processes and
capturing results, has grown out of control over the years, providing 4
sets of inputs that produce different outputs in different combinations.
An attempt to add type annotations for these combinations quickly grew
out of control, and along with causing numerous type-related regressions
in the codebase over the years, it's finally become time to put
execute()
to rest.This change introduces the successor,
run_process()
. The interface is
like a slimmed-down version ofexecute()
, allowing basic options like
the working directory and environment to be set, stderr to be redirected
to stdout, expected output encoding, and control over error handling and
logging.There is no control over typing. Instead, this always returns a
RunProcessResult
object, which stores a string version of the command
that was run, the exit code, and byte streams for the standard output
and error output.From there, a caller can access the raw bytes (via
stdout_bytes.read()
andstderr_bytes.read()
), Unicode-decoded content (viastdout.read()
andstderr.read()
), or line-based versions (via.readlines()
on
any of the streams).Results are always byte strings. Decoding to Unicode strings happens
on-the-fly viaio.TextIOWrapper
(which is what Python uses for
sys.stdout
andsys.stderr
). These wrappers are created only on
access.While these are all stream objects, they don't stream output from the
program. They always contain the full results from the program. If we
were to add streaming someday for some SCM, we could do this with a new
argument torun_process()
and new flags onRunProcessResult
without
a redesign.Like with
execute()
, all errors (non-0 exit codes) or selective errors
can be ignored. Unlikeexecute()
, the caller can still get results
when an error is raised. The exception,RunProcessError
, contains the
results as an attribute (result
), containing all the same information
that would normally be returned. Given that, the support for ignoring
errors is really just a convenience around catching an exception.This new object has full type safety, so type checkers can make sure
that consumers are processing results correctly.
run_process()
usesrun_process_exec()
to perform the actual execution
of a command. This is a wrapper aroundsubprocess.run()
that takes just
a few flags and returns a tuple of(exit_code, stdout, stderr)
. Spies
should connect to this instead ofrun_process()
, as it's as low-level
as a test would need to go, and the results from this are checked against
the parameters passed torun_process()
.Logging has been improved to provide better output, distinguishing non-0
exit codes representing failure from those with ignored errors (to help
alleviate concerns when people look at debug logs).Old Python 2 compatibility code around
subprocess
have been removed,
and new defaults (likeclose_fds
) utilized.
execute()
has been updated to wraprun_process()
. This function is
now deprecated, though still widely used in the codebase. Upcoming work
will transition over torun_process()
instead.
All unit tests pass on Python 3.7-3.11.
Summary | ID |
---|---|
1290949c07fc9ea0765bfe999262036f3cd11262 |
Description | From | Last Updated |
---|---|---|
'rbtools.deprecation.RemovedInRBTools50Warning' imported but unused Column: 1 Error code: F401 |
reviewbot | |
local variable 'e' is assigned to but never used Column: 5 Error code: F841 |
reviewbot | |
This should be Raises: and should list the TypeError for the command arg. Should we also list Exception in here … |
maubin | |
Wrap run_process with :py:func:. |
maubin | |
Shouldn't these 2 come before RunProcessError? |
maubin | |
unicode -> string |
david | |
This seems a little mangled. |
david |
- Change Summary:
-
Updated
ProcessRunResult
's constructor to supply defaults, to help with unit testing. - Commits:
-
Summary ID 6b06016cc52c03a58d53f190b8b821ef68fc98f9 7c4bccdf5facf0d9c9413d3b52db2ac14f65e900
Checks run (2 succeeded)
- Change Summary:
-
Added documentation, tests, and logging for
PermissionError
,FileNotFoundError
, and general exceptions when running a command. - Commits:
-
Summary ID 7c4bccdf5facf0d9c9413d3b52db2ac14f65e900 973715d8ed5bbcba1b4ec09628f19267c0604b87
- Change Summary:
-
- Normalized results when
stdout
orstderr
isNone
. - Added
run_process_exec()
, which spies should connect to in order to return results.
- Normalized results when
- Description:
-
Our
execute()
function, used for calling out into other processes andcapturing results, has grown out of control over the years, providing 4 sets of inputs that produce different outputs in different combinations. An attempt to add type annotations for these combinations quickly grew out of control, and along with causing numerous type-related regressions in the codebase over the years, it's finally become time to put execute()
to rest.This change introduces the successor,
run_process()
. The interface islike a slimmed-down version of execute()
, allowing basic options likethe working directory and environment to be set, stderr to be redirected to stdout, expected output encoding, and control over error handling and logging. There is no control over typing. Instead, this always returns a
RunProcessResult
object, which stores a string version of the commandthat was run, the exit code, and byte streams for the standard output and error output. From there, a caller can access the raw bytes (via
stdout_bytes.read()
and stderr_bytes.read()
), Unicode-decoded content (viastdout.read()
and stderr.read()
), or line-based versions (via.readlines()
onany of the streams). Results are always byte strings. Decoding to Unicode strings happens
on-the-fly via io.TextIOWrapper
(which is what Python uses forsys.stdout
andsys.stderr
). These wrappers are created only onaccess. While these are all stream objects, they don't stream output from the
program. They always contain the full results from the program. If we were to add streaming someday for some SCM, we could do this with a new argument to run_process()
and new flags onRunProcessResult
withouta redesign. Like with
execute()
, all errors (non-0 exit codes) or selective errorscan be ignored. Unlike execute()
, the caller can still get resultswhen an error is raised. The exception, RunProcessError
, contains theresults as an attribute ( result
), containing all the same informationthat would normally be returned. Given that, the support for ignoring errors is really just a convenience around catching an exception. This new object has full type safety, so type checkers can make sure
that consumers are processing results correctly. + run_process()
usesrun_process_exec()
to perform the actual execution+ of a command. This is a wrapper around subprocess.run()
that takes just+ a few flags and returns a tuple of (exit_code, stdout, stderr)
. Spies+ should connect to this instead of run_process()
, as it's as low-level+ as a test would need to go, and the results from this are checked against + the parameters passed to run_process()
.+ Logging has been improved to provide better output, distinguishing non-0
exit codes representing failure from those with ignored errors (to help alleviate concerns when people look at debug logs). Old Python 2 compatibility code around
subprocess
have been removed,and new defaults (like close_fds
) utilized.execute()
has been updated to wraprun_process()
. This function isnow deprecated, though still widely used in the codebase. Upcoming work will transition over to run_process()
instead. - Commits:
-
Summary ID 973715d8ed5bbcba1b4ec09628f19267c0604b87 a3542d716f1d2d20be84a32a86aceb107cccc014
Checks run (2 succeeded)
- Change Summary:
-
- Added a missing
Returns
section to therun_process()
docs. - Renamed the incorrect
Returns
toRaises
and added missing exceptions.
- Added a missing
- Commits:
-
Summary ID a3542d716f1d2d20be84a32a86aceb107cccc014 cab2f71dae0ef5fb564fcd21db5916ac513546ad