dabeaz / curio

Good Curio!
Other
4.01k stars 240 forks source link

TaskGroup wait #271

Closed kyuupichan closed 4 years ago

kyuupichan commented 5 years ago

First, thanks for curio. It's awesome.

A couple of things regarding TaskGroups:

a) like asyncio.gather, I sometimes find it useful to wait for all subtasks to compete (if successful) but to have the first failed task cause the others to be cancelled and propogate the same exception rather than a TaskGroupError exception. Is this worth another argument to join? b) Your join implementation treats wait=object and wait=all identically. The only difference is in _task_done but that uses self._wait, not the wait passed to join. This is in direct contradiction to the documentation of the "completed" attribute that says it uses what is passed to join()

kyuupichan commented 5 years ago

Any thoughts on those 2 points David?

dabeaz commented 5 years ago

I've been very busy teaching classes. No thoughts at this time. I'll look at it when I get a chance.

dabeaz commented 5 years ago

I'm reviewing this. It's kind of odd that TaskGroups specify a waiting policy both in their constructor and in their associated join() method. I'm inclined to take it away from the join() method entirely.

I'm also thinking about the exception propagation behavior. Maybe it would be more useful to simply report the first exception that gets raised instead of TaskGroupError. Especially since the behavior is to cancel all remaining tasks on error.

dabeaz commented 5 years ago

I have removed the wait argument from TaskGroup.join(). Regarding exception propagation, one tricky part concerns cancellation. For example, if you're waiting for a task to complete and the wait operation gets cancelled, how would you separate that exception from an exception occurring in the managed tasks? I'm not sure how it would be done except via a wrapped exception just as TaskError (as is used by the normal Task.join() method).

kyuupichan commented 5 years ago

Perhaps the exceptions that propagate from inside the container would be non-cancellation exceptions. Inner cancellations would not be excxeptional and just get added to the _finished list like now