FTP server: upload should support async close

[warner]

As discussed  on the mailing list, the FTP server in current trunk (r24958) does not have a mechanism for handling uploads that are not complete the moment the last byte is accepted by the consumer. This makes it difficult, for example, to implement an FTP frontend on a virtual filesystem that takes some amount of time/work to finish the upload process.

The proposed solution is to enhance ftp:IWriteFile to add a close() method, which will be called after the consumer has been disconnected, but before the '226 Transfer Complete' response is sent. close() can return a Deferred, and the response will not be sent until after it fires. The value produced by this Deferred is ignored.

Other proposed solutions were:

• modify unregisterProducer to allow it to return a Deferred
• do something at the VFS level

A patch to implement this change is attached. It does not yet have specific test coverage, but of course the existing tests still pass.

[warner]

patch to implement the interface+implementation change

[exarkun]

Seems like a good start. Can you add unit tests?

[warner]

new patch, adds tests

[warner]

ok, new patch uploaded with unit tests. exarkun gave it a quick once-over and didn't scream.

[terrycojones]

I applied the latest path, examined the diff and the patched code, didn't see anything to object to. Ran trial twisted without problem.

Thanks for the beer, Brian.

[spiv]

I like this idea, and this implementation of it. Just a couple of small nits:

1. Given that you're reformatting the test_write docstring anyway, fix the typo of "with fires" (should be "which fires").
2. I think docstrings are preferred to comments for test descriptions (even though test_ftp is pretty confused on this point). So change the initial comment of FTPCloseTest.test_write to a docstring.
3. (Optional: perhaps a docstring on FTPCloseTest itself would be a good idea too, something like "Tests that the server invokes IWriteFile.close.")
4. This needs a news file.

[warner]

new patch incorporating spiv's recommendations

[spiv]

Looks like warner forgot to reinstate the 'review' keyword.

[spiv]

Looks ok to me. I'll put it on a branch and see what the buildbot thinks.

[spiv]

(In [28450]) Branching to ftp-async-close-3462.

[spiv]

(In [28453]) Merge ftp-async-close-3462

Author: warner Reviewer: spiv Fixes: #3462

twisted.protocol.ftp.IWriteFile now has a close() method, which can return a Deferred. Previously a STOR command would finish immediately upon the receipt of the last byte of the uploaded file. With close(), the backend can delay the finish until it has performed some other slow action (like storing the data to a virtual filesystem).