Opened 9 years ago

Last modified 4 years ago

#26367 closed Cleanup/optimization

Assess if FieldFile can work with stdlib File instead of requiring Django's File — at Version 2

Reported by: Cristiano Coelho Owned by: nobody
Component: File uploads/storage Version: 1.9
Severity: Normal Keywords: file storage
Cc: Cristiano Coelho Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description (last modified by Tim Graham)

The FileField docs (https://docs.djangoproject.com/es/1.9/ref/models/fields/#django.db.models.FileField) state that in order to save a file into a FileField (through FieldFile proxy) you need a django File object and not a regular one.

Looking at the code (https://github.com/django/django/blob/28bcff82c5ed4694f4761c303294ffafbd7096ce/django/db/models/fields/files.py#L92 ) it seems like the only reason to have a File object is because it needs the 'size' property which isn't available on every file-like object.
Looking into the same file, it doesn't seem anyone uses _size anymore, only its parent class, BUT, this class actually overrides the size property so it is not calling the parent class .size (and if it did, it would cache it).

Would it be possible to remove any _size from FieldFile and then test against regular file-like objects rather than "File" and if everything works change the docs?
The only other place a File is required is at FileSystemStorage._save, however, since you don't call ._save but instead .save which already converts any file into a File (and it is also stated in the docs that the Storage.save method can accept any file), the requierement for a File seems unnecessary.

There's also a topic here https://groups.google.com/forum/#!topic/django-developers/CjoPZq8lZms that might suggest that the storage and file field classes require a few improvements.

Change History (2)

comment:1 by Cristiano Coelho, 9 years ago

Summary: Minor FieldFile (proxy for FieldFile) improvement?Minor FieldFile (proxy for FileField) improvement?

comment:2 by Tim Graham, 9 years ago

Description: modified (diff)
Summary: Minor FieldFile (proxy for FileField) improvement?Assess if FieldFile can work with stdlib File instead of requiring Django's File
Triage Stage: UnreviewedAccepted

The _size removal looks okay to me: PR

Not sure about whether this solves the issue of not requiring Django's File subclass, but accepting for further investigation.

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