[Twisted-Python] Advice sought on application evolution

[Phil Mayers]

glyph at divmod.com wrote:
> On 11:06 am, p.mayers at imperial.ac.uk wrote:
>>>
>>> It's great as a high-level prototyping tool and an orchestration 
>>> language for distributed conversations with large numbers of steps 
>>> and coarse- grained error handling.  It's bad as a way of 
>>> representing state machines or continuous processes.
>>
>> Why?
> 
> Here's something which we probably should have done with a state 
> machine, but instead did with deferredGenerator (the predecessor to 
> inlineCallbacks).
> 
> http://divmod.org/trac/browser/trunk/Quotient/xquotient/grabber.py#L438

Ok.

I would have written it more like this, dump the error handling and let 
it propagate upwards to someone who is interested, and obviously clean 
it up with inlineCallbacks:

@inlineCallbacks
def grabber(self):
   yield self.login(self._username, self._password)

   uids = []
   yield self.listUID(uids.append)

   for idx in range(0, len(uids), batchsize):
       uidset = uids[idx:idx+batchsize]
       L = self.shouldRetrieve(uidset).sorted()

...etc. and I guess what I'm getting at is why is the above worse than:

def grab(self):
   return self.login.addCallbacks(self.login_done, self.login_err)

def login_done(self, loginResult):
   uids = []
   d = self.listUID(uids.append)
   d.addCallback(self.uids_done, uids)
   d.addErrback(self.uids_err)

def uids_done(self, unk, uids):
   dlist = []
   for idx in range(0, len(uids), batchsize):
     uid_slice = uids[idx:idx+batchsize]
     d = self.sometinymethod(uid_slice)
     d.addErrback(self.tinymethod_err)
     dlist.append(d)
   dlist = defer.DeferredList(dlist)
   dlist.addCallback(self.next_method)

...etc.

I'm not disputing the point - I'm trying to figure out why 
inlineCallbacks might be inappropriate because I'm using it for just 
such an application and I'm finding it greatly superior to the previous 
multi-callback chain solution, which frankly was incomprehensible and 
error-prone (oops I forgot to return the deferred. again).

On the other hand, I *removed* inlineCallbacks from some lower-level 
calls in my SNMP libs as it was clearly cleaner, so another way of 
asking the question is "how do I detect that inlineCallbacks is bad code 
smell?"

> 
> The function's really long, and there's a ton of duplication between all 
> of the "except" clauses, and there needs to be a try/except around every 
> single deferred call.  This probably would have been better implemented 
> as a regular state-machine, because that would have allowed decomposing 
> the problem further.
> 
> Granted, you could blame the factoring here on any number of things.  It 
> could have been broken into different steps even with deferredGenerator, 
> it could have been documented better, it could have stored more state on 
> the instance so that you could inspect its state rather than hiding it 
> as local variables... but given that this is general stylistic advice I 
> think that the point holds.
> 
> _______________________________________________
> Twisted-Python mailing list
> Twisted-Python at twistedmatrix.com
> http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python