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 ?
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 usingRemoteFile
andBytesIO
makes sense. I wouldn't monkey-patchFTPHost
itself, but derive from it and add your method to the derived class.I think you could use
with
statements instead oftry ... finally
. If the download fails, I'd raise a high-level exception (higher thanPermanentError
, for example) instead of returningNone
. 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 thedownload
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 namedremote_content
,remote_contents
orremote_file_data
. Given this API change, it may make sense to put the code in a function instead of a method of theFTPHost
(or derived) class.When everything is implemented, add a docstring. Also, don't forget unit tests. :-)
Closing this ticket as duplicate of #128, which has more discussion on the subject.