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

Jean-Paul Calderone exarkun at divmod.com
Sat May 3 21:28:38 EDT 2008


On Sat, 03 May 2008 19:03:29 -0600, Michael Torrie <torriem at 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




More information about the Twisted-Python mailing list