Hi JP

...

...

...

...

"JP" == Jean-Paul Calderone <exarkun@divmod.com> writes: JP> On Fri, 6 Mar 2009 12:10:55 +0100, Terry Jones <terry@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