Author rnk
Recipients abbot, astrand, giampaolo.rodola, guettli, orsenthil, pitrou, r.david.murray, rnk, srid
Date 2010-02-02.03:47:23
SpamBayes Score 1.0697e-13
Marked as misclassified No
Message-id <1265082449.69.0.261022562821.issue5673@psf.upfronthosting.co.za>
In-reply-to
Content
> - why do you say Thread.join() uses a busy loop? is it because it uses
>   Condition.wait()? If so, this will be solved in py3k by issue7316 (which you
>   are welcome to review). Otherwise, I think there should be an upper bound on
>   the sleeping granularity (say 2 seconds).

Yes, that's what I was referring to.  I'm glad to hear the situation will
improve in the future!

> - the "if 'timeout' in kwargs" dance is a bit complicated. Why not simply
>   "kwargs.pop('timeout', None)"?

Good call, done.

> - if it times out, communicate() should raise a specific exception. Bonus points
>   if the exception holds the partial output as attributes (that's what we do for
>   non-blocking IO in py3k), but if it's too difficult we can leave that out. I
>   don't think returning None would be very good.

I agree.  Does subprocess.TimeoutExpired sound good?

It won't be possible with the current implementation to put the partial output
in the exception, because read blocks.  For example, in the Windows threaded
implementation, there's a background thread that just calls self.stdout.read(),
which blocks until its finished.

> - for consistency, other methods should probably raise the same exception. I
>   think we can leave out the more complex scenarios such as "timing out but
>   still processing the beginning of the output".

What do you mean still processing?  I agree, they should all throw the same
exception.  I think call and check_call should clean up after themselves by
killing the child processes they create, while communicate and wait should leave
that to the user.

I'm imagining something like this for communicate:

try:
    (stdout, stderr) = p.communicate(timeout=123)
except subprocess.TimeoutExpired:
    p.kill()
    (stdout, stderr) = p.communicate()  # Should not block long

And nothing special for check_call(cmd=[...], timeout=123).
History
Date User Action Args
2010-02-02 03:47:29rnksetrecipients: + rnk, guettli, astrand, orsenthil, pitrou, giampaolo.rodola, abbot, r.david.murray, srid
2010-02-02 03:47:29rnksetmessageid: <1265082449.69.0.261022562821.issue5673@psf.upfronthosting.co.za>
2010-02-02 03:47:28rnklinkissue5673 messages
2010-02-02 03:47:27rnkcreate