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 )
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 , 9 years ago
Summary: | Minor FieldFile (proxy for FieldFile) improvement? → Minor FieldFile (proxy for FileField) improvement? |
---|
comment:2 by , 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: | Unreviewed → Accepted |
The
_size
removal looks okay to me: PRNot sure about whether this solves the issue of not requiring Django's
File
subclass, but accepting for further investigation.