Skip to content
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

Add "capture_output=True" option to subprocess.run #76283

Closed
ncoghlan opened this issue Nov 21, 2017 · 4 comments
Closed

Add "capture_output=True" option to subprocess.run #76283

ncoghlan opened this issue Nov 21, 2017 · 4 comments
Assignees
Labels
type-feature

Comments

@ncoghlan
Copy link
Contributor

@ncoghlan ncoghlan commented Nov 21, 2017

BPO 32102
Nosy @warsaw, @gpshead, @ncoghlan, @bbayles
PRs
  • #5149
  • #8374
  • #8720
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = 'https://github.com/gpshead'
    closed_at = <Date 2018-01-30.06:43:14.741>
    created_at = <Date 2017-11-21.06:13:46.996>
    labels = ['type-feature']
    title = 'Add "capture_output=True" option to subprocess.run'
    updated_at = <Date 2018-08-09.22:02:59.493>
    user = 'https://github.com/ncoghlan'

    bugs.python.org fields:

    activity = <Date 2018-08-09.22:02:59.493>
    actor = 'miss-islington'
    assignee = 'gregory.p.smith'
    closed = True
    closed_date = <Date 2018-01-30.06:43:14.741>
    closer = 'gregory.p.smith'
    components = []
    creation = <Date 2017-11-21.06:13:46.996>
    creator = 'ncoghlan'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 32102
    keywords = ['patch']
    message_count = 4.0
    messages = ['306627', '306643', '311243', '311244']
    nosy_count = 4.0
    nosy_names = ['barry', 'gregory.p.smith', 'ncoghlan', 'bbayles']
    pr_nums = ['5149', '8374', '8720']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'commit review'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue32102'
    versions = []

    @ncoghlan
    Copy link
    Contributor Author

    @ncoghlan ncoghlan commented Nov 21, 2017

    I'm hesitant to suggest adding yet-another-option to any subprocess API, but I'm thinking it may be worth it in this case.

    The idea is to make it possible to replace:

        result = subprocess.run(cmd, stdout=PIPE, stderr=PIPE)

    with:

        result = subprocess.run(cmd, capture_output=True)

    That way, it would be possible for the raw stdin/stdout/stderr parameters to be treated as low level implementation details most of the time, since the most common configurations would be:

        # Use parent's stdin/stdout/stderr
        result = subprocess.run(cmd) 
        # Use pipe for stdin, use parent's stdout/stderr
        result = subprocess.run(cmd, input=data)
        # Use parent's stdin, use pipe for stdout/stderr
        result = subprocess.run(cmd, capture_output=True) 
        # Use pipe for stdin/stdout/stderr
        result = subprocess.run(cmd, input=data, capture_output=True)

    @ncoghlan ncoghlan added the type-feature label Nov 21, 2017
    @warsaw
    Copy link
    Member

    @warsaw warsaw commented Nov 21, 2017

    Yeah, I find it pretty common to set both stdout and stderr to PIPE, but I'm with you in hesitating to add YAO to the subprocess API. If you do move forward with this though, I think capture_output would have to be mutually exclusive to stdout and stderr. You should not be allowed to provide either of the latter two if the former is given.

    @gpshead gpshead self-assigned this Jan 30, 2018
    @gpshead
    Copy link
    Member

    @gpshead gpshead commented Jan 30, 2018

    New changeset ce0f33d by Gregory P. Smith (Bo Bayles) in branch 'master':
    bpo-32102 Add "capture_output=True" to subprocess.run (GH-5149)
    ce0f33d

    @gpshead
    Copy link
    Member

    @gpshead gpshead commented Jan 30, 2018

    Thanks Bo. I agree with Nick, this is a readability win for a common annoying to type otherwise case.

    @gpshead gpshead closed this Jan 30, 2018
    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    type-feature
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants