Opened 13 years ago

Closed 12 years ago

#1794 defect closed fixed (fixed)

protocols/ SenderProtocol doesn't implement IFInishableConsumer even though it claims it does

Reported by: thor Owned by:
Priority: normal Milestone:
Component: ftp Keywords:
Cc: spiv Branch:


While trying to transfer a file using FTPClient.storeFile and FileSender in the way suggested here, I noticed that the consumer created by storeFile (a SenderProtocol instance) doesn't fully implement the consumer interface, the attached patch fixes that a bit. I.e., the example from the mailing list then works, and doesn't complain.

--- Twisted-2.4.0.orig/protocols/ 2006-04-10 05:21:33.000000000 +0200
+++ Twisted-2.4.0.patched/protocols/      2006-06-05 20:04:05.000000000 +0200
@@ -1719,11 +1719,11 @@
     def write(self, data):

-    def registerProducer(self):
-        pass
+    def registerProducer(self, producer, streaming):
+        self.transport.registerProducer(producer, streaming)

     def unregisterProducer(self):
-        pass
+        self.transport.unregisterProducer()

     def finish(self):

Attachments (1)

ftp_storeFile_consumer.patch (570 bytes) - added by thor 13 years ago.
The patch

Download all attachments as: .zip

Change History (6)

Changed 13 years ago by thor

The patch

comment:1 Changed 13 years ago by spiv

Cc: spiv added
Component: coreftp
Owner: changed from Glyph to itamarst
Summary: protocols/ SenderProtocol doesn't implement full consumer interfaceprotocols/ SenderProtocol doesn't implement IFInishableConsumer even though it claims it does

You're right, it does look broken.

If the test suite doesn't complain that almost certainly means it isn't tested at all ;)

Anyway, this bug fix needs tests so that I don't accidentally break it later... so to commit this patch it needs tests for the SenderProtocol.

comment:2 Changed 13 years ago by thor

Well, at least the consumer interface SenderProtocol is not fully tested ;-) But writing a test-case for FTPClient.storeFile+FileSender requires really taking apart how works internally, and being but a humble end user who just started to use twisted, this is really not something I have the time and/or skills for at the moment. Maybe later. Anyway, it's not like this minipatch could break any old code, as the parts that were changed simply weren't implemented before.

comment:3 Changed 13 years ago by spiv

Owner: changed from itamarst to spiv

If you don't have time to add tests for your patch, that's ok. I'm assigning this to me to try to get around to this soon.

Thanks for taking the time to make this ticket and the patch!

comment:4 Changed 12 years ago by therve

Resolution: fixed
Status: newclosed

It was fixed in [18892].

comment:5 Changed 8 years ago by <automation>

Owner: spiv deleted
Note: See TracTickets for help on using tickets.