Opened 9 years ago

Closed 4 years ago

Last modified 4 years ago

#9894 closed New feature (wontfix)

Give the FileField 'upload_to' callable access to an UploadedFile's contents.

Reported by: Pyth Owned by: nobody
Component: File uploads/storage Version: 1.0
Severity: Normal Keywords: feature upload_to FileField
Cc: ashwoods Triage Stage: Accepted
Has patch: no Needs documentation: yes
Needs tests: yes Patch needs improvement: yes
Easy pickings: no UI/UX: no


FileField.upload_to allows for a callable and requests an instance and a filename. Because it may be of interest to the programmer to use the file's content (MD5 sum, MIME type, ID3 tags etc.) in the upload path, it would be handy to be able to access it from the callable method.

One possibility would be to pass an UploadedFile instead of a filename but this would break the compatibility policy in 1.0.X. Temporarily, this functionality could be added via an uploaded_file or content property added to the instance (in, making it accessible from the upload_to(instance, filename) callable with instance.uploaded_file.

I'll throw up some patches if this makes any sense.

Change History (11)

comment:1 Changed 9 years ago by Pyth

Needs documentation: set
Needs tests: set
Patch needs improvement: set
Triage Stage: UnreviewedDesign decision needed

comment:2 Changed 9 years ago by (none)

milestone: post-1.0

Milestone post-1.0 deleted

comment:3 Changed 9 years ago by Maxim Ivanov

Isn't file contents already accessible via inst.<field_name> ?

comment:4 Changed 7 years ago by Chris Beaven

Severity: Normal
Type: New feature

comment:5 Changed 6 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:6 Changed 6 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:7 Changed 5 years ago by Jacob

Triage Stage: Design decision neededAccepted

I think this is worth doing if we can figure out a good (and backwards-compatible) API.

comment:8 Changed 5 years ago by ashwoods

Cc: ashwoods added

comment:9 Changed 4 years ago by Mark Lavin

Resolution: wontfix
Status: newclosed

Spoke with julienphalip and others at the PyCon 2014 sprints and the consensus was that this is a reasonable but rare use case. However, doing this in a backwards compatible when changing the number of expected arguments for upload_to is messy and difficult. One way would be to change to call self.field.generate_filename with the extra content argument, catch the TypeError which would be raised if the upload_to doesn't take the arg and then raise a deprecation and call it again with only the instance and filename args.

    def save(self, name, content, save=True):
            name = self.field.generate_filename(self.instance, name, content)
        except TypeError:
            # Include deprecation warning about content arg
            name = self.field.generate_filename(self.instance, name)

The problem with this is the possibiliy that something inside the upload_to raises a TypeError and it is masked during this deprecation cycle leading to difficult to debug problems. Another possibility would be to use inspect.getargspec but that type of inspection can be prone to error.

Doing this type of name change based on the content is possible with the current code/API. If you don't need the model instance then the storage save gets the content and the name generated by the field. If you do need the instance and the content then you can subclass both FieldFile and FileField to make and FileField.generate_filename compatible with passing the content as an additional argument. While both of these directions require much more work, this use case seems small enough and somewhat custom enough to merit it. Overall the amount of work required to add this new argument doesn't appear to be worth the savings for the few that might need it.

Marking this as wontfix. If someone really wants this new argument and can make a compelling argument for a simple way to maintain backwards compatibility that should be raised on django-developers Otherwise you can try one of the work-arounds described above.

comment:10 Changed 4 years ago by Mark Lavin

To be clear the main problem with catching the TypeError is that it's such a broad error and there isn't a clean way to inspect the exception to know which function caused it. That is you can't know if the problem was generate_filename or another function internal to the callback.

comment:11 Changed 4 years ago by dmckeone

To avoid the issue with potentially discarding a valid TypeError here this implementation could potentially be used in conjunction with a change of the generate_filename signature to include a content= keyword argument:

    def save(self, name, content, save=True):
            inspect.getcallargs(self.field.generate_filename, self.instance, name, content=content)
        except TypeError:
            # Possibly make some kind of Deprecation Warning
            name = self.field.generate_filename(self.instance, name)
            name = self.field.generate_filename(self.instance, name, content=content)

The benefit of this implementation is that an old version of the upload_to= callback will continue to function as it had in the past, but Django can now emit a deprecation warning of some kind (deferring an eventual API change). Using inspect.getcallargs works in both Python 2 and 3, and it only tests the binding of the arguments to the function, without actually calling the function.

Note: See TracTickets for help on using tickets.
Back to Top