Change output_of to execute commands with shell_tools.run #5541
Conversation
Implement using shell_tools.run instead of run_cmd slated for removal.
dev_tools/shell_tools.py
Outdated
| code and the `check` flag was True (default). | ||
| """ | ||
| result = cast(str, run_cmd(*cmd, log_run_to_stderr=False, out=TeeCapture(), **kwargs).out) | ||
| cmdpure = [w for w in cmd if w is not None] |
There was a problem hiding this comment.
nit: Should this check be inside run()? Do all callers of run() need to check this?
There was a problem hiding this comment.
I'd like to have a minimum difference between behavior of shell_tools.run and subprocess.run, so I'd rather not do any filtering of the cmd arguments.
For consistency sake it is perhaps better to have output_of() handle arguments in the same way as run() and do the argument filtering in the caller. I have checked the references to output_of and it seems easily doable.
dev_tools/shell_tools.py
Outdated
| Args: | ||
| *cmd: Components of the command to execute, e.g. ["echo", "dog"]. | ||
| **kwargs: Extra arguments for asyncio.create_subprocess_shell, such as | ||
| Arguments set to None are disregarded in command execution. |
There was a problem hiding this comment.
What if we change the type of cmd to str instead of Optional[str]?
There was a problem hiding this comment.
Good point - see my previous comment. I am inclined to use the same argument interface in output_of() as is in run().
Will do a follow-up commit.
Pass command arguments to shell_tools.run unmodified. Remove filtering of the None items in command argument list; leave this up to the caller.
output_of requires command-line arguments in a list. Also do the filtering of an optional None items before the call.
|
@verult - updated and ready for review. |
pavoljuhas
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
automerge
…#5541) Stop using `shell_tools.run_cmd()` which is slated for removal. Get command output with `shell_tools.run()` instead. Update `output_of()` to receive command-line arguments in a list or string in order to have the same interface as `run()`. Remove filtering of the optional None arguments, leave it up to callers to take care of that. Partially implements quantumlib#4394
4 participants
Stop using
shell_tools.run_cmd()which is slated for removal.Get command output with
shell_tools.run()instead.Update
output_of()to receive command-line arguments ina list or string in order to have the same interface as
run().Remove filtering of the optional None arguments,
leave it up to callers to take care of that.
Partially implements #4394