This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Title: Deprecate PIPE with subprocess.check_call() and call()
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.6
Status: closed Resolution: out of date
Dependencies: Superseder:
Assigned To: Nosy List: akira, berker.peksag, iritkatriel, juj, martin.panter, mbussonn, vstinner
Priority: normal Keywords: patch

Created on 2014-09-19 13:56 by juj, last changed 2022-04-11 14:58 by admin. This issue is now closed.

File name Uploaded Description Edit akira, 2014-09-21 08:10 deprecate review
Messages (14)
msg227095 - (view) Author: juj (juj) Date: 2014-09-19 13:56
On Windows, write

import subprocess

def ccall(cmdline, stdout, stderr):
  proc = subprocess.Popen(['python', ''], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
  if proc.returncode != 0: raise subprocess.CalledProcessError(proc.returncode, cmdline)
  return 0

# To fix subprocess.check_call, uncomment the following, which is functionally equivalent:
# subprocess.check_call = ccall

subprocess.check_call(['python', ''], stdout=subprocess.PIPE, stderr=subprocess.PIPE)
print 'Finished!'

Then write

import sys

str = 'aaa'
for i in range(0,16): str = str + str
for i in range(0,2): print >> sys.stderr, str
for i in range(0,2): print str

Finally, run 'python'. The application will hang. Uncomment the specicied line to fix the execution.

This is a documented failure on the python subprocess page, but why not just fix it up directly in python itself?

One can think that modifying stdout or stderr is not the intent for subprocess.check_call, but python certainly should not hang because of that.
msg227096 - (view) Author: juj (juj) Date: 2014-09-19 14:05
The same observation applies to as well.
msg227138 - (view) Author: Akira Li (akira) * Date: 2014-09-20 00:35
> This is a documented failure on the python subprocess page,
> but why not just fix it up directly in python itself?

If you want to discard the output; you could use:

  check_call(args, stdin=DEVNULL, stdout=DEVNULL, stderr=STDOUT)

check_call() passes its parameters to Popen() as is.
The only parameter it knows about is args that is used to raise
an exception.

Do you want check_call() to inspect the parameters and to do 
something about stdout=PIPE, stderr=PIPE?

Where "something" could be:

- nothing -- the current behavior: everything works until the child 
  process produces enough output to fill any of OS pipe buffers as documented
- call proc.communicate() -- store (unlimited) output in memory instead of
  just hanging: everything works slowly until the system runs out of memory
- replace with DEVNULL -- "do what I mean" behavior: inconsistent with the
  direct Popen() call
- raise ValueError with informative error message (about DEVNULL option)
  after issueing a DeprecationWarning for a release: it fixes this particular
  misuse of check_call(). Are there other common "wrong in every case"
  check_call() parameters?
msg227148 - (view) Author: juj (juj) Date: 2014-09-20 08:51
Very good question akira. In one codebase where I have fixed this kind of bug, see

the intended usage by the original author had certainly been to throw in a PIPE just to mute both stdout and stderr output, and there was no intent to capture the results or anything. I think passing PIPE to those is meaningless, since they effectively behave as "throw the results away", since they are not returned.

Throwing an exception might be nice, but perhaps that would break existing codebases and therefore is not good to add(?). Therefore I think the best course of action would be to do what is behaviorally as developer intends: "please treat as if stdout and stderr had been captured to a pipe, and throw those pipes away, since they aren't returned.", so your third option, while inconsistent with direct Popen(), sounds most correct in practice. What do you think?

I am not currently aware of other such cases, although it would be useful to go through the docs and recheck the commit history of when that documentation note was added in to see if there was more related discussions that occurred.
msg227206 - (view) Author: Akira Li (akira) * Date: 2014-09-21 08:10
> What do you think?

I would prefer to deprecate PIPE argument for
issue DeprecationWarning in 3.5 and raise ValueError in 3.6+

I've uploaded a patch that issues the warning.
msg227208 - (view) Author: juj (juj) Date: 2014-09-21 08:32
Hmm, that path does it for stdout=PIPE in only? It could equally apply to stderr=PIPE in as well, and also to both stdout=PIPE and stderr=PIPE in subprocess.check_call?
msg227217 - (view) Author: Akira Li (akira) * Date: 2014-09-21 14:03
@juj: DeprecationWarning is generated if PIPE is passed to call() as
any positional or keyword argument in particular stdin, stdout, stderr.
It also applies to check_call() that uses call() internally.
msg227224 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-09-21 19:16
The first place to warn uses about dangerous function calls is the documentation, and your patch doesn't touch the documentation.

You can for example suggest to use check_output(), getstatusouptut() or getoutput().
msg227251 - (view) Author: Akira Li (akira) * Date: 2014-09-22 04:02
Victor, the message in my patch is copied almost verbatim from the 
current subprocess' documentation [1]


People use `call(cmd, stdout=PIPE)` as a *broken* way to suppress 
output i.e., when they actually want `call(cmd, stdout=DEVNULL)`

The issue with `call(cmd, stdout=PIPE)` that it *appears* to work
if cmd doesn't produce much output i.e., it might work in tests but
may hang in production.

It is unrelated to check_output(), getstatusouptut() or getoutput().
msg243615 - (view) Author: juj (juj) Date: 2015-05-19 18:11
This issue still reads open, but there has not been activity in a long time. May I ask what is the latest status on this?

Also, any chance whether this will be part of Python 2.x?
msg244644 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2015-06-02 02:48
I agree with the deprecation idea. The parameter checking logic doesn’t seem right though; see Reitveld. Also, I would have made the warning specify exactly what is deprecated, in case the stack trace doesn’t identify the function, which I think would always happen with check_call(). Also be less specific about future changes, unless there is clear consensus to make this change in 3.6. Maybe something like:

"Passing PIPE to call() and check_call() is deprecated; use DEVNULL instead to discard output or provide empty input"

Since 3.5 is now in the beta phase, would adding this deprecation be allowed, or should it be deferred to the 3.6 branch? Also, I’m not sure what the policy is for Python 2. Maybe it would be acceptable as a Python 3 compatibility warning, triggered by the “python2 -3” option; I dunno.
msg244645 - (view) Author: Matthias Bussonnier (mbussonn) * Date: 2015-06-02 04:16
3.5 have ``[1] that is much saner to use, and what you want to use in most cases. `call` and `check_call` docs even mention run.

msg245586 - (view) Author: Akira Li (akira) * Date: 2015-06-21 09:44
Martin, thank you for the review. As Matthias mentioned, the introduction of perhaps deprecates this issue: old api should be left  alone to avoid breaking old code, new code should use new api, those who need old api (e.g., to write 2/3 compatible code) are expected to read the docs that already contain necessary warnings).
msg380796 - (view) Author: Irit Katriel (iritkatriel) * (Python committer) Date: 2020-11-11 23:21
Any objections to closing this?

If the old API is going to be deprecated I think that's a topic for another issue.
Date User Action Args
2022-04-11 14:58:08adminsetgithub: 66632
2020-12-13 00:54:48iritkatrielsetstatus: pending -> closed
resolution: out of date
stage: patch review -> resolved
2020-11-11 23:21:05iritkatrielsetstatus: open -> pending
nosy: + iritkatriel
messages: + msg380796

2015-06-21 09:47:32berker.peksagsetnosy: + berker.peksag
stage: needs patch -> patch review

versions: + Python 3.6, - Python 3.5
2015-06-21 09:44:30akirasetmessages: + msg245586
2015-06-02 04:16:30mbussonnsetnosy: + mbussonn
messages: + msg244645
2015-06-02 02:48:42martin.pantersettitle: subprocess.check_call hangs on large PIPEd data. -> Deprecate PIPE with subprocess.check_call() and call()
nosy: + martin.panter

messages: + msg244644

components: - Windows
stage: needs patch
2015-05-19 18:11:46jujsetmessages: + msg243615
2014-09-22 04:02:52akirasetmessages: + msg227251
2014-09-21 19:16:14vstinnersetmessages: + msg227224
2014-09-21 14:03:12akirasetmessages: + msg227217
2014-09-21 08:32:12jujsetmessages: + msg227208
2014-09-21 08:10:13akirasetfiles: +
versions: + Python 3.5, - Python 2.7
messages: + msg227206

keywords: + patch
type: enhancement
2014-09-20 08:51:06jujsetmessages: + msg227148
2014-09-20 00:35:17akirasetnosy: + akira
messages: + msg227138
2014-09-19 14:05:53jujsetmessages: + msg227096
2014-09-19 14:05:24vstinnersetnosy: + vstinner
components: + Windows
2014-09-19 13:56:39jujcreate