Skip to content

[WIP] bpo-42648: Add exec_raise parameter to subprocess.Popen #23790

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

Closed
wants to merge 3 commits into from
Closed

[WIP] bpo-42648: Add exec_raise parameter to subprocess.Popen #23790

wants to merge 3 commits into from

Conversation

vstinner
Copy link
Member

@vstinner vstinner commented Dec 15, 2020

@gpshead gpshead marked this pull request as draft December 15, 2020 23:32
except OSError as exc:
if exec_raise:
raise
self.returncode = exc.errno
Copy link
Member

Choose a reason for hiding this comment

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

I think it is dangerous to assign errno as returncode as someone will wind up depending on that behavior even though they have no way of knowing if it was an errno or an actual exit code of the process.

The user said they didn't want an exception with full information, so lets not muck things up by potentially clobbering other information they could've had. Return a static value out of range of any possible actual child return code on all platforms, or if we want to get this complicated at all: Let the user specify the value to use as returncode.

Agreed that exec_raise and exec_* aren't great names for the parameter. oserror_via_returncode perhaps as a way to be explicit? But how many users even think in terms of OSError anyways given that is a parent class of the actual (FileNotFoundError, NotADirectoryError, PermissionError)'s they're likely to encounter?

@vstinner vstinner changed the title bpo-42648: Add exec_raise parameter to subprocess.Popen [WIP] bpo-42648: Add exec_raise parameter to subprocess.Popen Dec 16, 2020
@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Jan 15, 2021
@vstinner vstinner closed this Sep 21, 2021
@vstinner vstinner deleted the exec_raise branch September 21, 2021 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting core review stale Stale PR or inactive for long period of time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants