~sschwarzer/ftputil#118: 
In-memory downloads

Hi all,

As the title implies, it would be nice to be able to download a file in-memory. Currently I add the following method to ftputil.FTPHost:

def download_in_memory(self, source, callback=None):
    source = ftputil.tool.as_unicode(source)
    source_file = ftputil.file_transfer.RemoteFile(self, source, "rb")
    source_fobj = source_file.fobj()
    target_fobj = io.BytesIO()
    bytes_buffer = None

    try:
        try:
            ftputil.file_transfer.copyfileobj(source_fobj, target_fobj, callback=callback)
            bytes_buffer = target_fobj.getvalue()
        finally:
            target_fobj.close()
    finally:
        source_fobj.close()
        return bytes_buffer


ftputil.FTPHost.download_in_memory = download_in_memory

It returns a bytes buffer or None if the download failed. What do you think of this approach ?

Status
RESOLVED DUPLICATE
Submitter
ftputiluser (unverified)
Assigned to
No-one
Submitted
5 years ago
Updated
5 years ago
Labels
enhancement library

schwa (unverified) 5 years ago · edit

Sorry, I had forgotten your message.

To me, adding a method for this to FTPHost seems a bit too specialized to me to add it to ftputil. Apart from that, your implementation of using RemoteFile and BytesIO makes sense. I wouldn't monkey-patch FTPHost itself, but derive from it and add your method to the derived class.

I think you could use with statements instead of try ... finally. If the download fails, I'd raise a high-level exception (higher than PermanentError, for example) instead of returning None. If you need to distinguish between different kinds of errors, you should use different exception classes (possibly inheriting from a common class), so the caller can handle the error conditions as needed.

Your method name download_in_memory emphasizes the similarities to the download method. What about choosing a method name from the point of view of the code calling your method? With this in mind, your method could be named remote_content, remote_contents or remote_file_data. Given this API change, it may make sense to put the code in a function instead of a method of the FTPHost (or derived) class.

When everything is implemented, add a docstring. Also, don't forget unit tests. :-)

schwa (unverified) 4 years ago · edit

Closing this ticket as duplicate of #128, which has more discussion on the subject.

Register here or Log in to comment, or comment via email.