Path: csiph.com!v102.xanadu-bbs.net!xanadu-bbs.net!eternal-september.org!feeder.eternal-september.org!border1.nntp.ams1.giganews.com!nntp.giganews.com!bcyclone01.am1.xlned.com!bcyclone01.am1.xlned.com!newsfeed.xs4all.nl!newsfeed4a.news.xs4all.nl!xs4all!newsgate.cistron.nl!newsgate.news.xs4all.nl!post.news.xs4all.nl!not-for-mail
MIME-Version: 1.0
In-Reply-To: <87siastsby.fsf@Equus.decebal.nl>
References: <87siastsby.fsf@Equus.decebal.nl>
Date: Wed, 20 May 2015 03:47:42 +1000
Subject: Re: Best way to rewrite Popen
From: Chris Angelico <rosuav@gmail.com>
Cc: "python-list@python.org" <python-list@python.org>
Content-Type: text/plain; charset=UTF-8
Precedence: list
Newsgroups: comp.lang.python
Message-ID: <mailman.148.1432057670.17265.python-list@python.org>
Lines: 41
NNTP-Posting-Host: 2001:888:2000:d::a6
Xref: csiph.com comp.lang.python:90881

On Wed, May 20, 2015 at 3:01 AM, Cecil Westerhof <Cecil@decebal.nl> wrote:
> At the moment I am playing with things like:
>     p = subprocess.Popen('ls -l', shell = True, stdout = subprocess.PIPE)
>
> I think that most of the times this are the values I want. So it would
> be nice to overrule the defaults. What is the best way to do this? So
> creating a function that is exactly the same except for the defaults
> for shell and stdout (and maybe stderr).

Well... I would have to start by saying that you probably _don't_ want
to use shell=True by default. Putting it explicitly on the cases where
you need it helps you remember its danger. You also don't need it for
simple cases like that one; improve your reliability by providing a
list instead of a string, and then you can leave shell=False:

p = subprocess.Popen(['ls','-l'], stdout=subprocess.PIPE)

Running everything via the shell is unnecessary, and a dangerous
default. (Maybe it's not a problem when you use a string literal as
the command, but if you make that the default, you'll end up exposing
yourself in some situation where it isn't hard-coded.) With that
change, there's really only one parameter that you're defaulting, so
there's not as much point making the change, but the technique still
works, and maybe you'll add more to the setup:

@functools.wraps(subprocess.Popen)
def Popen(*a, **kw):
    if 'stdout' not in kw: kw['stdout'] = subprocess.PIPE
    return subprocess.Popen(*a, **kw)

That's a simple way to patch in some function defaults. But
personally, I'd probably end up doing something like this:

def capture_stdout(*a, **kw):
    if 'stdout' not in kw: kw['stdout'] = subprocess.PIPE
    return subprocess.Popen(*a, **kw).stdout

so I can directly iterate over the thing. Or something else. One
change isn't really enough to justify the extra layer of indirection.

ChrisA