search

etianen / django-s3-storage

Django Amazon S3 file storage.
BSD 3-Clause "New" or "Revised" License
414 stars 94 forks source link

S3Storage: move `_temporary_file` helper function into a helper method so it's easier to override #131

Closed kevinmarsh closed 2 years ago

kevinmarsh commented 2 years ago

Potentially a bit of a niche change in structure but shouldn't really add any complexity. I've run into a workflow* where I need to the _temporary_file to be a tempfile.TemporaryFile and not a tempfile.SpooledTemporaryFile.

Moving _temporary_file into a method makes it a lot easier to subclass S3Storage like

from tempfile import TemporaryFile
from django_s3_storage.storage import S3Storage

class MyS3Storage(S3Storage):

  def _new_temporary_file(self):
    """Can't use the default SpooledTemporaryFile because XXX"""
    return TemporaryFile()

as opposed to either mocking _temporary_file when opening/saving from S3Storage or having to convert the resulting my_model.my_file_field.field.field into a non "spooled" object (calling SpooledTemporaryFile.rollover() won't help my situation either)

*For reference the workflow is passing in a file stored on S3 into pandas.read_csv which handles SpooledTemporaryFile differently than TemporaryFile (which implements __next__) when converting the file to a pandas.DataFrame since it just assumes that SpooledTemporaryFile is always a buffer and not a real file.

etianen commented 2 years ago

Works for me... but can you remove the underscore prefix of the method name so it's clear it's available for overriding in a subclass, and not a protected internal implementation detail?

kevinmarsh commented 2 years ago

Works for me... but can you remove the underscore prefix of the method name so it's clear it's available for overriding in a subclass, and not a protected internal implementation detail?

Yup have force pushed a new version without the underscore prefix

etianen commented 2 years ago

Cool! That's merged. I'll make a new release next week, hopefully.