On 4 December 2010 18:14, Gregory P. Smith <greg@krypto.org> wrote:
>
>
> On Sat, Dec 4, 2010 at 3:45 AM, Antoine Pitrou <solipsis@pitrou.net> wrote:
>>
>> On Sat, �4 Dec 2010 10:10:44 +0100 (CET)
>> gregory.p.smith <python-checkins@python.org> wrote:
>> > Author: gregory.p.smith
>> > Date: Sat Dec �4 10:10:44 2010
>> > New Revision: 87010
>> >
>> > Log:
>> > issue7213 + issue2320: Cause a DeprecationWarning if the close_fds
>> > argument is
>> > not passed to subprocess.Popen as the default value will be changing in
>> > a
>> > future Python to the safer and more often desired value of True.
>>
>> That doesn't seem to be a good idea under Windows, is it?
>>
>> (�Note that on Windows, you cannot set *close_fds* to true and
>> also redirect the standard handles by setting *stdin*, *stdout* or
>> *stderr*.�)
>
> Regardless of what platform you are on I think the warning makes sense, it
> was just worded too strongly. �It is asking people to be explicit with
> close_fds. �Explicitly setting close_fds=False when that is desired is good.
> I modified the docs and warning message to just say that the default
> close_fds behavior will change in the future without specifying exactly what
> it will be. �It could make sense for the default to be a soft-True on
> windows that changes behavior if any of the std handles are set if that
> matches what users expect and find easiest. �We may want to reconsider its
> entire future in the face of the new pass_fds parameter, etc. �Those are 3.3
> decisions.

This sounds like omitting the close_fds parameter is now considered
ill-advised, if not outright wrong. That's silly - I certainly never
specify close_fds, expecting the module to do the correct thing if I
don't know/care enough to say. I use Popen as a convenience function,
so ignoring low-level details is the whole point in my opinion (and
close_fds *is* a low-level detail for what I do, on Windows).

At the very least, the whole of the section "Replacing Older Functions
with the subprocess Module" (with a couple of small exceptions) is in
need of updating, as it is recommending bad practice (not specifying
close_fds) at the moment...

OK, I'm exaggerating a touch here. But can someone point me at the
discussion of this change? Or if there hasn't been one, can we have
one (i.e., to start the ball rolling, can someone justify the change,
please).

Paul.

Making the change was intended to force the discussion. �I'm glad that worked. :)

I don't like the thought of requiring people to specify close_fds either but the default is a bad one (see http://bugs.python.org/issue7213 and�http://bugs.python.org/issue2320) so we should change it.
">

(original) (raw)



On Sat, Dec 4, 2010 at 11:28 AM, Paul Moore <p.f.moore@gmail.com> wrote:

On 4 December 2010 18:14, Gregory P. Smith <greg@krypto.org> wrote:
>
>
> On Sat, Dec 4, 2010 at 3:45 AM, Antoine Pitrou <solipsis@pitrou.net> wrote:
>>
>> On Sat, �4 Dec 2010 10:10:44 +0100 (CET)
>> gregory.p.smith <python-checkins@python.org> wrote:
>> > Author: gregory.p.smith
>> > Date: Sat Dec �4 10:10:44 2010
>> > New Revision: 87010
>> >
>> > Log:
>> > issue7213 + issue2320: Cause a DeprecationWarning if the close\_fds
>> > argument is
>> > not passed to subprocess.Popen as the default value will be changing in
>> > a
>> > future Python to the safer and more often desired value of True.
>>
>> That doesn't seem to be a good idea under Windows, is it?
>>
>> (�Note that on Windows, you cannot set \*close\_fds\* to true and
>> also redirect the standard handles by setting \*stdin\*, \*stdout\* or
>> \*stderr\*.�)
>
> Regardless of what platform you are on I think the warning makes sense, it
> was just worded too strongly. �It is asking people to be explicit with
> close\_fds. �Explicitly setting close\_fds=False when that is desired is good.
> I modified the docs and warning message to just say that the default
> close\_fds behavior will change in the future without specifying exactly what
> it will be. �It could make sense for the default to be a soft-True on
> windows that changes behavior if any of the std handles are set if that
> matches what users expect and find easiest. �We may want to reconsider its
> entire future in the face of the new pass\_fds parameter, etc. �Those are 3.3
> decisions.

This sounds like omitting the close\_fds parameter is now considered
ill-advised, if not outright wrong. That's silly - I certainly never
specify close\_fds, expecting the module to do the correct thing if I
don't know/care enough to say. I use Popen as a convenience function,
so ignoring low-level details is the whole point in my opinion (and
close\_fds \*is\* a low-level detail for what I do, on Windows).

At the very least, the whole of the section "Replacing Older Functions
with the subprocess Module" (with a couple of small exceptions) is in
need of updating, as it is recommending bad practice (not specifying
close\_fds) at the moment...

OK, I'm exaggerating a touch here. But can someone point me at the
discussion of this change? Or if there hasn't been one, can we have
one (i.e., to start the ball rolling, can someone justify the change,
please).

Paul.

Making the change was intended to force the discussion. �I'm glad that worked. :)

I don't like the thought of requiring people to specify close\_fds either but the default is a bad one (see http://bugs.python.org/issue7213 and�http://bugs.python.org/issue2320) so we should change it.

The real question is how should we go about doing the change? �This warning asking people to always specify close\_fds=True is heavy handed. �Other places in the stdlib and docs no doubt still need to be updated to reflect it, etc.


Options that seem worthy of debate:

A) The DeprecationWarning that exists in py3k as of today.

B) Remove the DeprecationWarning, simply document that people should be explicit about it if they care at all and that the default will change in 3.3.


C) Never change close\_fds behavior. �we're stuck with it for life.

D) Deprecate close\_fds so that it becomes a legacy no-op. �the new pass\_fds flag could evolve into this. �this will break existing code that depends on close\_fds one way or another.


I'm in 100% agreement that forcing people to pass close_fds in makes the API less friendly (granted, people already find it unfriendly but why make it worse?).


Option B seems the most friendly to me.

Option D is always on the table but I haven't planned out what a future without it should look like. �I prefer requiring people who need open file descriptors to pass them in; a semaphore for "all fds" could be created and pass\_fds=ALL becomes the new "close\_fds=False" (I think one of the bugs suggests this).

-gps