IIRC I've covered by tests the behavior of implementation already
existed at that moment.
At first look Jason's proposal about cancelling all futures in
downstream and raising CancelledError to upstream sounds reasonable to
me,
But cancellation is very subtle matter and I not 100% sure about the change.
I wrote simple script to check Twisted behavior:
#--------------------------
from twisted.internet.defer import Deferred
def pr(v, name):
print(name, v)
return v
def make_canceller(name):
def canceller(d):
print("Cancel", name)
return canceller
outer = Deferred(make_canceller("Outer"))
outer.addBoth(pr, name="Outer")
inner = Deferred(make_canceller("Inner"))
inner.addBoth(pr, name="Inner")
inner.chainDeferred(outer)
inner.cancel()
#--------------------------
It shows (if I understand correctly) that cancelling of inner deferred
doesn't cancel outer one, but outer gets CanceledError exception.
Cancelling of outer deferred doesn't affects inner one.
Maybe writing a document with comprehensive description of future/task
cancellation can help to make good decision?
At least the document can be included as part of
https://docs.python.org/dev/library/asyncio-dev.html That makes a
value in itself.
Search for 'cancellation' in python-tulip finds a lot of
conversations, it's hard concept and some summary would be very
useful.
On Sat, May 31, 2014 at 3:31 AM, Guido van Rossum <[email protected]> wrote:
> I'm actually not sure. I didn't write those tests, Andew Svetlov did. It's
> at least possible that he simply wrote a test for whatever the
> implementation did. But it's also possible that I'm totally forgetting some
> important piece of reasoning that led to this design. I'm also curious what
> Twisted Deferreds do in this case -- if an outer Deferred is waiting for an
> inner Deferred and the inner Deferred is cancelled, is the outer one
> considered cancelled too?
>
>
> On Fri, May 30, 2014 at 9:20 AM, Jason Tackaberry <[email protected]> wrote:
>>
>> On 14-05-29 02:05 PM, Guido van Rossum wrote:
>>
>> Perhaps. Could I impose on you and ask you to (1) file a bug in the Tulip
>> tracker describing the problem, and (2) develop a patch and send it for my
>> review using Rietveld (codereview.appspot.com)?
>>
>>
>> Sure, will try to complete that this weekend.
>>
>> I poked a bit at an initial implementation, but I wanted to clarify
>> something before going too much further.
>>
>> My understanding of the goal is that given a chain of futures, if one of
>> those futures is cancelled, that future and everything downstream is
>> cancelled, while nothing upstream is cancelled and instead B's
>> CancelledError is bubbled up.
>>
>> So e.g. given A -> B -> C -> D, if B is cancelled, then so are C and D,
>> but A isn't, although A could be done with an exception if B's
>> CancelledError is uncaught.
>>
>> Some of the unit tests assume that in this scenario A should be cancelled
>> too. For example, this Task unit test:
>>
>> def test_cancel_inner_future(self):
>> f = asyncio.Future(loop=self.loop)
>>
>> @asyncio.coroutine
>> def task():
>> yield from f
>> return 12
>>
>> t = asyncio.Task(task(), loop=self.loop)
>> test_utils.run_briefly(self.loop) # start task
>> f.cancel()
>> with self.assertRaises(asyncio.CancelledError):
>> self.loop.run_until_complete(t)
>> self.assertTrue(f.cancelled())
>> self.assertTrue(t.cancelled())
>>
>>
>> With the change, t.cancelled() would be False, although t.done() would be
>> True.
>>
>> Earlier you called it a subtle bug, but the unit tests are testing for
>> this behaviour, which means it's actually intentional design. So I wanted
>> to double check before going too far down a rabbit hole. Are you in
>> agreement that this behaviour should be changed, and the affected unit tests
>> updated to reflect this change? Or have I misunderstood the intention?
>>
>> Thanks!
>
>
>
>
> --
> --Guido van Rossum (python.org/~guido)
--
Thanks,
Andrew Svetlov
