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.

classification
Title: subprocess.Popen.wait() undocumented "endtime" parameter
Type: Stage: resolved
Components: Versions: Python 3.7, Python 3.6, Python 3.5
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Daryl Luna, Mariatta, berker.peksag, georg.brandl, giampaolo.rodola, gps, gregory.p.smith, larry, martin.panter, python-dev, r.david.murray, rnk
Priority: normal Keywords: patch

Created on 2014-02-09 10:23 by giampaolo.rodola, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
issue20572.patch Mariatta, 2016-11-18 11:57 review
issue20572v2.patch Mariatta, 2016-11-20 08:15 added stacklevel=2 and test case review
Pull Requests
URL Status Linked Edit
PR 552 closed dstufft, 2017-03-31 16:36
Messages (25)
msg210740 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2014-02-09 10:23
http://hg.python.org/cpython/file/29d9638bf449/Lib/subprocess.py#l1144
This was introduced in revision 6b627e121573  and is currently not documented.
I'm not sure whether this is a documentation issue or "endtime" should have been "_endtime" instead.
msg210742 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2014-02-09 10:24
Sorry: revision a161081e8f7c.
msg210770 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-02-09 16:35
Indeed, there's no issue number or NEWS entry.  It's not clear from the limited context why this parameter was added.  It doesn't appear to be consistent with the rest of the stdlib: the application can compute timeout from its desired endtime, which is how all of the other stdlib 'wait' APIs work.
msg210771 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-02-09 17:10
Also, the implementation as it stands is in any case flawed, since specifying both timeout and endtime is allowed by the code, and results in endtime overriding timeout silently, and in the posix version the resulting timeout error message will have the ignored timeout in it.

Unfortunately this has been in here for a while (March 2011) so people may be using it.

Based on the docs in the original commit (c4a0fa6e687c), I suspect endtime on wait was supposed to be an internal parameter....except that it is never used internally.  The endtime is converted back to a timeout in order to call wait.  Ideally we'd just drop it...but it may be in use in wild.
msg210789 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2014-02-09 21:05
I'd be for just getting rid of it for 3.4 now that we still can.
Being that this parameter is 1) not documented and 2) it's not even clear what it does I think it's unlikely that there's people using it.
msg210832 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-02-10 13:07
This isn't a release blocker.  And it's not really viable to remove it.

Since it's been in several releases, I suspect our only option is to throw a deprecation warning.  And, since deprecations aren't turned on by default, maybe the best thing would be to also document it (!) as being deprecated (!!).

Even though rc1 is already tagged and in the process of being released, I will accept a patch with the above suggested change for Python 3.4.  If you want to do something else talk to me first.
msg210833 - (view) Author: Giampaolo Rodola' (giampaolo.rodola) * (Python committer) Date: 2014-02-10 13:27
> Since it's been in several releases

AFAICT only 3.3. 
Reid, are you willing to provide a patch?
msg210835 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-02-10 14:12
I don't think we should document it as deprecated except in What's New.  But a code deprecation in 3.4 would be a good idea.
msg210840 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-02-10 14:34
My suggestion for documenting it was to document the fact that it's unsupported, unrecommended, deprecated, has poor semantics, etc.  If a user discovers it, and finds it's not documented, they'll probably think they can get away with using it.  If we explicitly document it as "don't use" I think that's better.
msg210978 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2014-02-11 17:21
New changeset 73793590d97b by Gregory P. Smith in branch 'default':
Deprecate Popen.wait()'s undocumented endtime parameter. issue20572.
http://hg.python.org/cpython/rev/73793590d97b
msg210979 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2014-02-11 17:22
documented with a deprecation.  that's the best we can do for now.  it can be considered for removal in the 3.5 or 3.6 timeframe.  i doubt many people used it.
msg210981 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-02-11 17:28
Well, a deprecation warning in the code would be nice in case anybody did use it.  Personally I don't see any problem with putting that in 3.4.1 if we don't get to it for 3.4.0.
msg210983 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-02-11 17:33
I'm reopening this as a 3.5 issue to do the actual removal.
msg211006 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2014-02-11 19:35
I suggest that that documentation counts for starting the deprecation cycle, however I would still also accept a patch adding a deprecation warning if the parameter is used.
msg264082 - (view) Author: Berker Peksag (berker.peksag) * (Python committer) Date: 2016-04-24 00:17
Do we still want to remove the endtime parameter?
msg281074 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2016-11-18 03:36
1. So, is it ok to remove the endtime parameter now?
2. Can the attached downloadfile.htm be removed? It's a spam.

Thanks :)
msg281085 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-11-18 09:06
I think other people wanted to add an automated deprecation warning in the code first, before removing it.
msg281102 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2016-11-18 11:57
Thanks, Martin :)
Attached is the patch where deprecation warning is emitted if endtime argument is passed.
Let me know if this works.

Thanks :)
msg281193 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-11-19 03:04
Thanks Mariatta. Some people like to use the warn(stacklevel=2) parameter, then the warning message is potentially more useful.

Also, I think it would be good to write a brief test case for the warning. There are probably lots of examples in the test suite, e.g. search for DeprecationWarning in revision 0dd263949e41.

Have you checked if there is existing code using the endtime=... parameter? It you run the test suite with -Werror enabled, that would be a good indicator.

Maybe it is okay to remove the parameter completely in 3.7.
msg281254 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2016-11-20 08:14
Thanks Martin :)

Alright, in v2 patch, I added stacklevel=2 parameter and test case for it.
Did not find any usage of the endtime parameter in cpython.

I'm also thinking that it can be removed in 3.7, considering it's been documented as deprecated since 3.4. But maybe I'm not a good judge for this?
msg281255 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2016-11-20 08:15
lol forgot to upload the patch. Here it is :)
msg281310 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-11-21 00:31
New changeset 0e8aa537c565 by Gregory P. Smith in branch '3.6':
Issue #20572: The subprocess.Popen.wait method's undocumented endtime
https://hg.python.org/cpython/rev/0e8aa537c565

New changeset f02422c6110a by Gregory P. Smith in branch 'default':
Issue #20572: Remove the subprocess.Popen.wait endtime parameter.
https://hg.python.org/cpython/rev/f02422c6110a
msg281311 - (view) Author: Gregory P. Smith (gregory.p.smith) * (Python committer) Date: 2016-11-21 00:32
thanks for the patch.  I reworked it slightly including the test.  warning in 3.6, gone in 3.7.  i still need to update the 3.7 docs to remove it.
msg281312 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-11-21 00:35
New changeset 54b2f377653d by Gregory P. Smith in branch 'default':
issue 20572: remove the deprecation notice for the deleted endtime parameter.
https://hg.python.org/cpython/rev/54b2f377653d
msg281320 - (view) Author: Mariatta (Mariatta) * (Python committer) Date: 2016-11-21 03:47
Works for me. Thanks :)

Maybe it's ok to close this ticket now?
History
Date User Action Args
2022-04-11 14:57:58adminsetgithub: 64771
2017-03-31 16:36:35dstufftsetpull_requests: + pull_request1081
2016-11-22 12:13:10martin.pantersetstatus: open -> closed
stage: patch review -> resolved
2016-11-21 03:47:13Mariattasetmessages: + msg281320
2016-11-21 00:35:37python-devsetmessages: + msg281312
2016-11-21 00:32:44gregory.p.smithsetmessages: + msg281311
2016-11-21 00:31:19python-devsetmessages: + msg281310
2016-11-20 08:15:15Mariattasetfiles: + issue20572v2.patch

messages: + msg281255
2016-11-20 08:14:23Mariattasetmessages: + msg281254
2016-11-19 03:04:31martin.pantersetstage: patch review
versions: + Python 3.6, Python 3.7
2016-11-19 03:04:18martin.pantersetmessages: + msg281193
2016-11-18 11:57:03Mariattasetfiles: + issue20572.patch
keywords: + patch
messages: + msg281102
2016-11-18 09:06:10martin.pantersetnosy: + martin.panter
messages: + msg281085
2016-11-18 09:04:52martin.pantersetfiles: - downloadfile.htm
2016-11-18 03:36:24Mariattasetnosy: + Mariatta
messages: + msg281074
2016-04-24 10:12:42Daryl Lunasetfiles: + downloadfile.htm
nosy: + Daryl Luna
status: pending -> open
2016-04-24 00:17:31berker.peksagsetstatus: open -> pending
nosy: + berker.peksag
messages: + msg264082

2014-02-11 19:35:39larrysetmessages: + msg211006
2014-02-11 17:33:45r.david.murraysetstatus: closed -> open

messages: + msg210983
versions: + Python 3.5, - Python 3.3, Python 3.4
2014-02-11 17:28:53r.david.murraysetmessages: + msg210981
2014-02-11 17:22:42gregory.p.smithsetstatus: open -> closed

nosy: + gregory.p.smith
messages: + msg210979

resolution: fixed
2014-02-11 17:21:12python-devsetnosy: + python-dev
messages: + msg210978
2014-02-10 14:34:21larrysetmessages: + msg210840
2014-02-10 14:12:15r.david.murraysetmessages: + msg210835
2014-02-10 13:27:54giampaolo.rodolasetmessages: + msg210833
2014-02-10 13:07:47larrysetpriority: release blocker -> normal

messages: + msg210832
2014-02-09 21:05:56giampaolo.rodolasetpriority: normal -> release blocker
nosy: + larry, georg.brandl
messages: + msg210789

2014-02-09 17:10:53r.david.murraysetmessages: + msg210771
2014-02-09 16:35:33r.david.murraysetnosy: + r.david.murray, gps
messages: + msg210770
2014-02-09 10:24:10giampaolo.rodolasetmessages: + msg210742
2014-02-09 10:23:20giampaolo.rodolacreate