Python 3's subprocess.run API is great. We should have it in Python 2 as well so we can write modern Python code compatible with both versions.
<rdar://problem/67487751>
Created attachment 406938 [details] Patch
Comment on attachment 406938 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406938&action=review > Tools/Scripts/libraries/webkitcorepy/webkitcorepy/subprocess_utils.py:39 > + input = kwargs.pop('input', None) > + capture_output = kwargs.pop('capture_output', False) > + timeout = kwargs.pop('timeout', None) > + check = kwargs.pop('check', False) Other than timeout, is there any reason we want to extract other variables out of **kwargs? It looks like the same defaults in the subprocess.run > Tools/Scripts/libraries/webkitcorepy/webkitcorepy/subprocess_utils.py:54 > + return subprocess.run(*popenargs, input=input, capture_output=capture_output, timeout=timeout, check=check, **kwargs) > + return subprocess.run( > + *popenargs, > + input=input, > + capture_output=capture_output, > + timeout=min(timeout or sys.maxsize, int(math.ceil(difference))), > + check=check, > + **kwargs) So the only difference is timeout, maybe we can simplify this by doing: if difference: timeout = min(timeout or sys.maxsize, int(math.ceil(difference))) return subprocess.run(*popenargs, input=input, capture_output=capture_output, timeout=timeout, check=check, **kwargs)
Comment on attachment 406938 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406938&action=review >> Tools/Scripts/libraries/webkitcorepy/webkitcorepy/subprocess_utils.py:39 >> + check = kwargs.pop('check', False) > > Other than timeout, is there any reason we want to extract other variables out of **kwargs? It looks like the same defaults in the subprocess.run Was copying from the function subprocess.run declaration, turns out, no, we don't need to extract the other variables. I thought we needed it to get the correct defaults, for some reason, but we don't. Will Remove. >> Tools/Scripts/libraries/webkitcorepy/webkitcorepy/subprocess_utils.py:54 >> + **kwargs) > > So the only difference is timeout, maybe we can simplify this by doing: > if difference: > timeout = min(timeout or sys.maxsize, int(math.ceil(difference))) > return subprocess.run(*popenargs, input=input, capture_output=capture_output, timeout=timeout, check=check, **kwargs) Thanks for the suggestion! Much cleaner!
Created attachment 406952 [details] Patch for landing
Committed r265957: <https://trac.webkit.org/changeset/265957> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406952 [details].
r=me