Tweak subprocess_run_helper. #23110
Tweak subprocess_run_helper. #23110
Conversation
There was a problem hiding this comment.
The change is reasonable. Given that it has just been introduced and given that we haven't advertised it in a changelog or similar, the change of somebody using this is minimal. So I'm fine with changing without a deprecation even though it's technically public API.
I'm not clear for the whole testing subpackage whether we consider this public API or only internal helpers. I'd tend to say that subprocess_run_helper() could be private as well.
|
I agree, did not think through this API carefully enough. |
|
Passing env is required, not sure if you intended that @anntzer . |
|
That was a mistake on my side, fixed. |
On general grounds, an API like `subprocess_run_helper(func, *args, timeout, **extra_env)` is problematic because it prevents one from passing an environment variable called "timeout". Instead, pass the extra environment variables as a dict, without unpacking. (Technically this has been released in 3.5.2 as public API, but 1) I'm not really sure it should have been a public API to start with (should we deprecate it and make it private?), and 2) hopefully tweaking that in 3.5.3 with no deprecation is not going to disrupt anyone... I can still put in a changelog entry if that's preferred.)
|
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free to suggest an improvement. |
On general grounds, an API like
subprocess_run_helper(func, *args, timeout, **extra_env)is problematic because it prevents one from passing an environment
variable called "timeout".
Instead, pass the extra environment variables as a dict, without
unpacking.
(Technically this has been released in 3.5.2 as public API, but 1) I'm
not really sure it should have been a public API to start with (should
we deprecate it and make it private?), and 2) hopefully tweaking that in
3.5.3 with no deprecation is not going to disrupt anyone... I can still
put in a changelog entry if that's preferred.)
PR Summary
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).