[Django] #28402: Provide 'temporary_file_path' for InMemoryUploadedFile
Django
------------------------------------------+------------------------
Reporter: Thomas Güttler | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.11
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------+------------------------
I was hit by this exception:
> AttributeError: 'InMemoryUploadedFile' object has no attribute
'temporary_file_path'
The bad thing: It was in production, this was not detected by our CI .
Next thing I did: I implemented a method which provided my something
similar to `temporary_file_path`.
Why not provide a property `temporary_file_path` for
`InMemoryUploadedFile`.
I think this way you have the advantage of both: No file on disk if you
don't want to and a file on disk if you want it.
What do you think?
--
Ticket URL: <https://code.djangoproject.com/ticket/28402>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Thomas Güttler | Owner: nobody
Type: Uncategorized | Status: new
Component: Uncategorized | Version: 1.11
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Theofanis Despoudis):
Looking at the source code, indeed we don't offer an API for
`temporary_file_path`. However, do we need one? As It does not make sense
to provide a path for an in memory file handler, for this abstraction.
Plus you can get the file name by accessing the `name` property of the
`InMemoryUploadedFile` instance.
--
Ticket URL: <https://code.djangoproject.com/ticket/28402#comment:1>
Django
Reporter: Thomas Güttler | Owner: nobody
Cleanup/optimization |
Component: File | Version: 1.11
uploads/storage |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* type: Uncategorized => Cleanup/optimization
* component: Uncategorized => File uploads/storage
Comment:
Where does the exception come from -- your own code or from Django? I
think adding the method would be backwards incompatible. Consider the code
in
[https://github.com/django/django/blob/adab280cefb15659c39558ac26ea392b0a1e456c/django/core/files/storage.py#L253-L277
FileSystemStorage._save()] - - third party storages may have similar logic
that would break with the proposed change.
--
Ticket URL: <https://code.djangoproject.com/ticket/28402#comment:2>
Django
Reporter: Thomas Güttler | Owner: nobody
Cleanup/optimization |
Component: File | Version: 1.11
uploads/storage |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Thomas Güttler):
Hi Graham, thank you for looking at this.
The code which uses `temporary_file_path` is in my code. This is no bug in
django. This is a feature request.
Yes, naming the property "temporary_file_path" could break code like this.
A new name would be needed or a clear deprecation path.
Do you understand my use case: Get a file_name (as string) which contains
the temporary upload in every case with one method.
I guess there are several hundred implementation which all do the same:
put the data from MemoryUploadedFile into a temporary file.
--
Ticket URL: <https://code.djangoproject.com/ticket/28402#comment:3>
Django
Reporter: Thomas Güttler | Owner: nobody
Cleanup/optimization |
Component: File | Version: 1.11
uploads/storage |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: new => closed
* resolution: => wontfix
Comment:
Depending on your application's requirements (if every uploaded file needs
to be written to disk), perhaps you'd want to define `FILE_UPLOAD_HANDLERS
= ['django.core.files.uploadhandler.TemporaryFileUploadHandler']` which
removes `MemoryFileUploadHandler` from the default list. Alternatively,
you can [https://docs.djangoproject.com/en/1.11/topics/http/file-
uploads/#id1 modify the upload handlers on the fly].
You didn't describe your use case, but I think these solutions suffice for
most use cases. An API that effectively transforms an in-memory uploaded
file into a temporary file seems more complicated.
--
Ticket URL: <https://code.djangoproject.com/ticket/28402#comment:4>