better flush iopub with AsyncResults #2255
Conversation
requesting metadata (e.g. ar.data or ar.stdout) will result in flushing iopub if the outputs are incomplete, so separate wait(0) need not be called. This also applies the workaround discussed in ipython#2215
|
I have tested this and it definitely fixes the behavior with accessing ar.data. |
|
|
||
| self._wait_for_outputs(10) | ||
| if timeout is not None and timeout < 0: | ||
| timeout = 10 |
There was a problem hiding this comment.
Any particular reason to hardcode 10 here? Just curious, I'm always a bit reluctant to have hardcoded constants in the middle of execution code...
There was a problem hiding this comment.
10 is putting an upper limit, when the user specified infinite wait. This is to avoid possible bugs where output never indicates itself as being done. Looking at it, the logic is actually slightly wrong, which I will fix now.
|
The code looks good, and since it fixes the problem we should go ahead, but I do have a quick question: do we have None, 0 and -1 all indicating 'infinite' timeout? If so, can we use only one of them? |
0 means no waiting at all (the opposite), but None and -1 both indicate infinite waits (None being more Pythonic, but -1 is inherited from the Pool.AsyncResult API, which we copied exactly). I can unify on -1 for infinite wait, but I think we would have to cast None to -1 anyway, since it makes way more sense for |
|
I should clarify: after this PR, the |
|
OK, thanks for the clarification. I had misunderstood the meaning of 0, and I think your reasoning for allowing None (but keeping compatibility with multiprocessing) is sound. |
|
OK, I'm leaving #2215 open until we can further clarify if this fully closes it, but merging because it does address the problem of |
better flush iopub with AsyncResults; requesting metadata (e.g. ar.data or ar.stdout) will result in flushing iopub if the outputs are incomplete, so separate wait(0) need not be called.
better flush iopub with AsyncResults; requesting metadata (e.g. ar.data or ar.stdout) will result in flushing iopub if the outputs are incomplete, so separate wait(0) need not be called.
requesting metadata (e.g. ar.data or ar.stdout) will result in flushing iopub if the outputs are incomplete, so separate wait(0) need not be called.
This also applies the workaround discussed in #2215
I'm not sure if it should close #2215, because the underlying cause there remains unknown.