[Twisted-Python] A few comments on deferreds

[Terry Jones]

Hi JP

>>>>> "JP" == Jean-Paul Calderone <exarkun at divmod.com> writes:
JP> On Fri, 6 Mar 2009 12:10:55 +0100, Terry Jones <terry at jon.es> wrote:

>> 1. If you have a deferred d1, and you call d1.chainDeferred(d2) I think
>> it makes sense for there to be a flag inside the deferred class that
>> warns you if you later call d1.addCallbacks. That's because adding more
>> functions to d1's callback/errback chain is probably not useful. (It
>> might conceivably be useful, e.g., to let you know that d1 has been
>> called). The result of d1 has been passed to d2, and because d2.callback
>> and d2.errback don't return it (see point 2 below), it is not passed
>> further down d1's chain. So it's most likely that someone adding a
>> callback to d1 after calling d1.chainDeferred doesn't really know what
>> they're doing.

JP> Indeed.  I can't think of any case where I've wanted to add more
JP> callbacks to a Deferred after chaining it to (from?  with?) another
JP> Deferred.

Right.

>> 2. Deferred.callback and Deferred.errback currently (implicitly) return
>> None.  They could be changed to return their passed argument.  That
>> would make a call to d1.chainDeferred act like a deferred fork - the
>> result would go down d2's callback chain, but also continue down the
>> callback/errback chain of d1.

JP> But it would be catastrophic for error handling.  Suddenly all of your
JP> errors on d1 which are handled perfectly correctly with an errback on
JP> d2 would become unhandled failures.

Ah yes :-)  Oops.

If you really wanted to do this, you could use another helper function
(e.g., Deferred.forkDeferred) which created and added a call/errback
functions to d1 that called d2 call/errback and then returned its arg). I
don't have a use case for that, so I'm not suggesting adding it to Twisted.
Anyone who wanted that kind of deferred forking functionality could write
it for themselves in a few lines.

>> 3. There's also the possibility of adding a function to the Deferred class:
>> 
>> def incorporateDeferred(self, d2):
>> self.callbacks.extend(d2.callbacks)
>> 
>> which might be what a naive reader might expect chainDeferred to do.
>> This raises the question of what you might later do with d2 (probably
>> nothing), but that's just like what happens with d1: if you call
>> d1.chainDeferred you're probably not going to return d1 to anyone. All
>> you can usefully do is fire it.

JP> If this accounted for future changes to d2, it would make more sense.

Yes. But as I mentioned, this mirrors the situation with d1.chainDeferred
in which future changes to d1 are also probably useless. It feels to me
like extending callbacks might be what people are expecting, as d1 then
remains useful.  (This leads to thoughts of a Deferred class that can't be
fired, just used to accumulate callbacks that are solely used to extend
other deferreds, but I guess deferreds are already abstract enough without
going there...)

JP> I think the correct direction to go in is to prevent people from trying
JP> to add more callbacks to d1.

Agreed. It's a simple change and it almost certainly flags a misunderstanding.
I can open an issue and submit a patch if you like.

Terry