subprocess.Popen.wait() undocumented "endtime" parameter #64771

giampaolo · 2014-02-09T10:23:20Z

BPO	20572
Nosy	@birkenfeld, @gpshead, @larryhastings, @giampaolo, @bitdancer, @rnk, @berkerpeksag, @vadmium, @Mariatta
PRs	[Do Not Merge] Convert `Misc/NEWS` so that it is managed by towncrier #552
Files	issue20572.patch issue20572v2.patch: added stacklevel=2 and test case

^{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 = None
closed_at = <Date 2016-11-22.12:13:10.669>
created_at = <Date 2014-02-09.10:23:20.136>
labels = ['3.7']
title = 'subprocess.Popen.wait() undocumented "endtime" parameter'
updated_at = <Date 2017-03-31.16:36:35.477>
user = 'https://github.com/giampaolo'

bugs.python.org fields:

activity = <Date 2017-03-31.16:36:35.477>
actor = 'dstufft'
assignee = 'none'
closed = True
closed_date = <Date 2016-11-22.12:13:10.669>
closer = 'martin.panter'
components = []
creation = <Date 2014-02-09.10:23:20.136>
creator = 'giampaolo.rodola'
dependencies = []
files = ['45532', '45555']
hgrepos = []
issue_num = 20572
keywords = ['patch']
message_count = 25.0
messages = ['210740', '210742', '210770', '210771', '210789', '210832', '210833', '210835', '210840', '210978', '210979', '210981', '210983', '211006', '264082', '281074', '281085', '281102', '281193', '281254', '281255', '281310', '281311', '281312', '281320']
nosy_count = 12.0
nosy_names = ['georg.brandl', 'gregory.p.smith', 'larry', 'giampaolo.rodola', 'gps', 'r.david.murray', 'rnk', 'python-dev', 'berker.peksag', 'martin.panter', 'Daryl Luna', 'Mariatta']
pr_nums = ['552']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue20572'
versions = ['Python 3.5', 'Python 3.6', 'Python 3.7']

The text was updated successfully, but these errors were encountered:

giampaolo · 2014-02-09T10:23:20Z

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.

giampaolo · 2014-02-09T10:24:10Z

Sorry: revision a161081e8f7c.

bitdancer · 2014-02-09T16:35:33Z

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.

bitdancer · 2014-02-09T17:10:53Z

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.

giampaolo · 2014-02-09T21:05:56Z

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.

larryhastings · 2014-02-10T13:07:47Z

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.

giampaolo · 2014-02-10T13:27:54Z

Since it's been in several releases

AFAICT only 3.3.
Reid, are you willing to provide a patch?

bitdancer · 2014-02-10T14:12:16Z

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.

larryhastings · 2014-02-10T14:34:22Z

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.

python-dev · 2014-02-11T17:21:12Z

New changeset 73793590d97b by Gregory P. Smith in branch 'default':
Deprecate Popen.wait()'s undocumented endtime parameter. bpo-20572.
http://hg.python.org/cpython/rev/73793590d97b

gpshead · 2014-02-11T17:22:43Z

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.

bitdancer · 2014-02-11T17:28:53Z

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.

bitdancer · 2014-02-11T17:33:45Z

I'm reopening this as a 3.5 issue to do the actual removal.

larryhastings · 2014-02-11T19:35:40Z

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.

berkerpeksag · 2016-04-24T00:17:31Z

Do we still want to remove the endtime parameter?

Mariatta · 2016-11-18T03:36:24Z

So, is it ok to remove the endtime parameter now?
Can the attached downloadfile.htm be removed? It's a spam.

Thanks :)

vadmium · 2016-11-18T09:06:11Z

I think other people wanted to add an automated deprecation warning in the code first, before removing it.

Mariatta · 2016-11-18T11:57:03Z

Thanks, Martin :)
Attached is the patch where deprecation warning is emitted if endtime argument is passed.
Let me know if this works.

Thanks :)

vadmium · 2016-11-19T03:04:18Z

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.

Mariatta · 2016-11-20T08:14:24Z

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?

Mariatta · 2016-11-20T08:15:15Z

lol forgot to upload the patch. Here it is :)

python-dev · 2016-11-21T00:31:19Z

New changeset 0e8aa537c565 by Gregory P. Smith in branch '3.6':
Issue bpo-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 bpo-20572: Remove the subprocess.Popen.wait endtime parameter.
https://hg.python.org/cpython/rev/f02422c6110a

gpshead · 2016-11-21T00:32:45Z

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.

python-dev · 2016-11-21T00:35:38Z

New changeset 54b2f377653d by Gregory P. Smith in branch 'default':
bpo-20572: remove the deprecation notice for the deleted endtime parameter.
https://hg.python.org/cpython/rev/54b2f377653d

Mariatta · 2016-11-21T03:47:14Z

Works for me. Thanks :)

Maybe it's ok to close this ticket now?

giampaolo added the release-blocker label Feb 9, 2014

larryhastings removed the release-blocker label Feb 10, 2014

gpshead closed this as completed Feb 11, 2014

bitdancer reopened this Feb 11, 2014

vadmium added the 3.7 (EOL) end of life label Nov 19, 2016

vadmium closed this as completed Nov 22, 2016

ezio-melotti transferred this issue from another repository Apr 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

subprocess.Popen.wait() undocumented "endtime" parameter #64771

subprocess.Popen.wait() undocumented "endtime" parameter #64771

giampaolo commented Feb 9, 2014

giampaolo commented Feb 9, 2014

giampaolo commented Feb 9, 2014

bitdancer commented Feb 9, 2014

bitdancer commented Feb 9, 2014

giampaolo commented Feb 9, 2014

larryhastings commented Feb 10, 2014

giampaolo commented Feb 10, 2014

bitdancer commented Feb 10, 2014

larryhastings commented Feb 10, 2014

python-dev mannequin commented Feb 11, 2014

gpshead commented Feb 11, 2014

bitdancer commented Feb 11, 2014

bitdancer commented Feb 11, 2014

larryhastings commented Feb 11, 2014

berkerpeksag commented Apr 24, 2016

Mariatta commented Nov 18, 2016

vadmium commented Nov 18, 2016

Mariatta commented Nov 18, 2016

vadmium commented Nov 19, 2016

Mariatta commented Nov 20, 2016

Mariatta commented Nov 20, 2016

python-dev mannequin commented Nov 21, 2016

gpshead commented Nov 21, 2016

python-dev mannequin commented Nov 21, 2016

Mariatta commented Nov 21, 2016

subprocess.Popen.wait() undocumented "endtime" parameter #64771

subprocess.Popen.wait() undocumented "endtime" parameter #64771

Comments

giampaolo commented Feb 9, 2014

giampaolo commented Feb 9, 2014

giampaolo commented Feb 9, 2014

bitdancer commented Feb 9, 2014

bitdancer commented Feb 9, 2014

giampaolo commented Feb 9, 2014

larryhastings commented Feb 10, 2014

giampaolo commented Feb 10, 2014

bitdancer commented Feb 10, 2014

larryhastings commented Feb 10, 2014

python-dev mannequin commented Feb 11, 2014

gpshead commented Feb 11, 2014

bitdancer commented Feb 11, 2014

bitdancer commented Feb 11, 2014

larryhastings commented Feb 11, 2014

berkerpeksag commented Apr 24, 2016

Mariatta commented Nov 18, 2016

vadmium commented Nov 18, 2016

Mariatta commented Nov 18, 2016

vadmium commented Nov 19, 2016

Mariatta commented Nov 20, 2016

Mariatta commented Nov 20, 2016

python-dev mannequin commented Nov 21, 2016

gpshead commented Nov 21, 2016

python-dev mannequin commented Nov 21, 2016

Mariatta commented Nov 21, 2016