Fix for t.spread.util.LocalAsyncFowarder

[twisted-trac]

	icepick reported
Trac ID	trac#286
Type	defect
Created	2003-09-26 09:47:51Z

Attachments:

• t.spread.util.LocalAsyncForwarder.patch (1044 bytes) - added by icepick on 2003-09-26 09:47:51Z -
• test_spread.patch (1032 bytes) - added by icepick on 2003-10-02 18:43:36Z -

Searchable metadata

``` trac-id__286 286 type__defect defect reporter__icepick icepick priority__high high milestone__ branch__ branch_author__ status__closed closed resolution__fixed fixed component__ keywords__ time__1064569671000000 1064569671000000 changetime__1065217915000000 1065217915000000 version__ owner__ cc__itamarst cc__moshez cc__icepick ```

[twisted-trac]

	Automation removed owner

[twisted-trac]

	icepick commented

#!html
<pre>
I'll work on a simple test case, probably this weekend.
</pre>

[twisted-trac]

	icepick commented

#!html
<pre>
I applied the patch to my tree and it works.  Thanks.
</pre>

[twisted-trac]

	icepick commented

#!html
<pre>
LocalAsyncForwarder was returning Deferred's to my when in a
real network it wouldn't, so I fixed it.

</pre>

[twisted-trac]

	@moshez commented

#!html
<pre>
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.
</pre>

[twisted-trac]

	icepick commented

#!html
<pre>
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.
</pre>

[twisted-trac]

	@moshez commented

#!html
<pre>
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 :)
</pre>

[twisted-trac]

	@itamarst commented

#!html
<pre>
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.
</pre>

[twisted-trac]

	@moshez commented

#!html
<pre>
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].
</pre>

[twisted-trac]

	icepick commented

#!html
<pre>
This was bugging me, and the test is simple.  Here is a patch for the unit test
that shows my problem.
</pre>

[twisted-trac]

	icepick commented

#!html
<pre>
The last patch is incorrect.  Working on a new one.
</pre>

[twisted-trac]

	@moshez commented

#!html
<pre>
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)
</pre>

[twisted-trac]

	@itamarst commented

#!html
<pre>
I can't get the patch to apply the diff, dunno why. Feel free to do it yourself
and commit.
</pre>

[twisted-trac]

	@moshez commented

#!html
<pre>
Checking in twisted/spread/util.py;
/cvs/Twisted/twisted/spread/util.py,v  &lt;--  util.py
new revision: 1.11; previous revision: 1.10
done

Fixed.
</pre>