Alexander Belopolsky <[email protected]> added the comment:
The documentation should mention somewhere that timeout can be a float. For
example, as in time.sleep docstring:
"""
sleep(seconds)
Delay execution for a given number of seconds. The argument may be
a floating point number for subsecond precision.
"""
I would also like to see some discussion of supported precision. Is is the
same as for time.sleep()? Does float precision ever affect timeout precision?
(On systems with nanosleep it may, but probably this has no practical
concequences.)
This can be done as a future enhancement, but I would like to see
datetime.timedelta as an acceptable type for timeout. This can be done by
adding duck-typed code in the error branch which would attempt to call
timeout.total_seconds() to extract a float.
Looking further, it appears that timeout can be anything that can be added to a
float to produce float. Is this an accident of implementation or a design
decision? Note that a result Fraction can be used as timeout but Decimal
cannot.
Zero and negative timeouts are accepted by subprocess.call(), but the result is
not documented. It looks like this still starts the process, but kills it
immediately. An alternative would be to not start the process at all or
disallow negative or maybe even zero timeouts altogether. I don't mind the
current choice, but it should be documented at least in
Popen.wait(timeout=None) section.
+ def wait(self, timeout=None, endtime=None):
"""Wait for child process to terminate. Returns returncode
attribute."""
Docstring should describe timeout and endtime arguments. In fact I don't see
endtime documented anywhere. It is not an obvious choice
that endtime is ignored when timeout is given. An alternative would be to
terminate at min(now + timeout, endtime).
+ delay = 0.0005 # 500 us -> initial delay of 1 ms
I think this should be an argument to wait() and the use of busy loop should be
documented.
+ delay = min(delay * 2, remaining, .05)
Why .05? It would probably be an overkill to make this another argument, but
maybe make it an attribute of Popen, say self._max_busy_loop_delay or a shorter
descriptive name of your choice. If you start it with '_', you don't need to
document it, but users may be able to mess with it if they suspect that 0.05 is
not the right choice.
+ endtime = time.time() + timeout
Did you consider using datetime module instead of time module here? (I know,
you still need time.sleep() later, but you won't need to worry about variable
precision of time.time().)
----------
_______________________________________
Python tracker <[email protected]>
<http://bugs.python.org/issue5673>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
