Fix for t.spread.util.LocalAsyncFowarder

[icepick]

LocalAsyncForwarder was returning Deferred's to my when in a
real network it wouldn't, so I fixed it.

[moshez]

This is weird. When deferreds are passed a deferred as a value, it should 
automatically undefer itself...so basically, _stripDeferred should be a no-op. 
Can you find a test case which demonstrates this problem? Because I think the 
solution should be elsewhere.

[icepick]

Sure.  My chord unit test makes use of this:  

http://cryptomonkey.net/dht/chord.py
http://cryptomonkey.net/dht/test_chord.py

If you run it thru 'trial' without this patch you get errors of callback getting
Deferreds with results as the arg.

[moshez]

Can you attempt to minimize the error? I can't run these, because I lack a significant number of dependencies, and I'm pretty certain this can't be related to, say, pycrypto :)

[itamarst]

I think the issue is the code does myDeferred.callback(o) where o might be a
Deferred as well. If o is a Deferred this is incorrect behaviour.

[moshez]

He doesn't seem to call .callback himself, so there would appear to be an error in Twisted somewhere.
Hence, my request for a bug report, rather than a patch.
icepick, in general, we need a bug report before we can evaluate patches. Ideally, a failing test case which we can run is best -- this allows us to add it to the Twisted test suite to ensure the bug doesn't happen again [or to see that you are using an API incorrectly and add a warning to the documentation].

[icepick]

I'll work on a simple test case, probably this weekend.

[icepick]

This was bugging me, and the test is simple.  Here is a patch for the unit test
that shows my problem.

[icepick]

The last patch is incorrect.  Working on a new one.

[moshez]

The following patch would appear to solve the problem correctly.
Assigning to Itamar for review, feel free to assign back to me for
applying the patch, or apply it yourself.

Thanks

Index: twisted/spread/util.py
===================================================================
RCS file: /cvs/Twisted/twisted/spread/util.py,v
retrieving revision 1.10
diff -u -r1.10 util.py
--- twisted/spread/util.py      4 May 2003 12:03:34 -0000       1.10
+++ twisted/spread/util.py      2 Oct 2003 19:01:31 -0000
@@ -77,7 +77,7 @@

     def callRemote(self, method, *args, **kw):
         if hasattr(self.interfaceClass, method):
-            result = defer.execute(self._callMethod, method, *args, **kw)
+            result = defer.maybeDeferred(self._callMethod, method, *args, **kw)
             return result
         elif self.failWhenNotImplemented:
             return defer.fail(
Index: twisted/test/test_spread.py
===================================================================
RCS file: /cvs/Twisted/twisted/test/test_spread.py,v
retrieving revision 1.3
diff -u -r1.3 test_spread.py
--- twisted/test/test_spread.py 11 Feb 2003 23:45:52 -0000      1.3
+++ twisted/test/test_spread.py 2 Oct 2003 19:01:31 -0000
@@ -22,11 +22,14 @@
 from twisted.trial import unittest

 from twisted.spread.util import LocalAsyncForwarder
+from twisted.internet import defer
 from twisted.python.components import Interface

 class IForwarded:
     def forwardMe(self):
         pass
+    def forwardDeferred(self):
+        pass

 class Forwarded:

@@ -39,8 +42,9 @@

     def dontForwardMe(self):
         self.unforwarded = 1
-
-
+
+    def forwardDeferred(self):
+        return defer.succeed(1)

 class SpreadUtilTest(unittest.TestCase):
     def testLocalAsyncForwarder(self):
@@ -50,3 +54,7 @@
         assert f.forwarded
         lf.callRemote("dontForwardMe")
         assert not f.unforwarded
+        rr = lf.callRemote("forwardDeferred")
+        l = []
+        rr.addCallback(l.append)
+        self.assertEqual(l[0], 1)

[itamarst]

I can't get the patch to apply the diff, dunno why. Feel free to do it yourself
and commit.

[moshez]

Checking in twisted/spread/util.py;
/cvs/Twisted/twisted/spread/util.py,v  <--  util.py
new revision: 1.11; previous revision: 1.10
done

Fixed.

[icepick]

I applied the patch to my tree and it works.  Thanks.