Re: [Twisted-Python] ldaptor's LDAPClient.send_multiResponse not dealing with chained deferreds, patch

On Sat, 03 May 2008 19:03:29 -0600, Michael Torrie torriem@gmail.com wrote:

This message is probably for Tv.

I have a situation where the callback to a LDAPClient.send_multiResponse() call needs to do a bunch of work that I want to split up over several functions, using deferred chaining. What I want to do is something like this (this in conjunction with an LDAP proxy server I am writing using Twisted and ldaptor):

d = self.client.send_multiResponse(request, got_response)

then later:

def got_response(response):

# get a list of other things to do with the response d = do_stuff_with_response(response) d.addCallback(finish)

return d

def finish(response): # we're done

return isinstance(response, ( pureldap.LDAPSearchResultDone, pureldap.LDAPBindResponse, ))

However, this doesn't work, because the send_multiResponse() method of LDAPClient isn't expecting the handler to return a deferred, even though if I do return a deferred the reactor does handle it properly, chaining the deferreds and their callbacks.

You don't need to return the Deferred in order for the code and event sources associated with it to cause it to eventually fire. Returning it doesn't do anything more than make it available to the caller. It seems this is what you actually want though, so this is just a change of perspective not a contradiction of your conclusion.

The problem is that in ldapclient.py, line 171, we have:

           # Return true to mark request as fully handled
           if handler(msg.value, *args, **kwargs):
               del self.onwire[msg.id]

When handler() returns a deferred object, after the reactor processes the chain the value is a Deferred object, not True or False, even though the value of the deferred object may be True or False. Hence the del self.onwire[msg.id] always executes, which when dealing with search result entries is a problem as they all share the same id. I made a quick hack to fix this:

[snip]

•            result = handler(msg.value, *args, **kwargs)
  

•            try:
  

•                result = result.result
  

•            except AttributeError:
  

•                pass
  

•           if result:
                del self.onwire[msg.id]
  
  

  ##Bind

---

Is this an acceptable way to do this?

Not really. You need to take the code that depends on the result of the Deferred and put it into a callback which gets attached to that Deferred. Something more like this:

def cbHandler(result): if result: del self.onwire[msg.id]

result = handler(msg.value, *args, **kwargs) if isinstance(result, Deferred): result.addCallback(cbHandler) else: cbHandler(result)

Or, written using the helper function twisted.internet.defer.maybeDeferred,

def cbHandler(result): if result: del self.onwire[msg.id] result = maybeDeferred(handler, msg.value, *args, **kwargs) result.addCallback(cbHandler)

This correctly handles both Deferred and non-Deferred results. I don't have any idea if this change to ldaptor is correct with respect to LDAP semantics/requirements or the particular implementation details of the code in question here, though.

Jean-Paul