[Django] #28402: Provide 'temporary_file_path' for InMemoryUploadedFile

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[Django] #28402: Provide 'temporary_file_path' for InMemoryUploadedFile

Django
#28402: Provide 'temporary_file_path' for InMemoryUploadedFile
------------------------------------------+------------------------
               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.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/050.dec797ea6566373b5c3a84f85b026b51%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Django] #28402: Provide 'temporary_file_path' for InMemoryUploadedFile

Django
#28402: Provide 'temporary_file_path' for InMemoryUploadedFile
--------------------------------+--------------------------------------
     Reporter:  Thomas Güttler  |                    Owner:  nobody
         Type:  Uncategorized   |                   Status:  new
    Component:  Uncategorized   |                  Version:  1.11
     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
--------------------------------+--------------------------------------

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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.77d653699edacf9af4feefbab06aa487%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Django] #28402: Provide 'temporary_file_path' for InMemoryUploadedFile

Django
In reply to this post by Django
#28402: Provide 'temporary_file_path' for InMemoryUploadedFile
-------------------------------------+-------------------------------------
     Reporter:  Thomas Güttler       |                    Owner:  nobody
         Type:                       |                   Status:  new
  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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.22277c59d3db61551abceafd9335581d%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Django] #28402: Provide 'temporary_file_path' for InMemoryUploadedFile

Django
In reply to this post by Django
#28402: Provide 'temporary_file_path' for InMemoryUploadedFile
-------------------------------------+-------------------------------------
     Reporter:  Thomas Güttler       |                    Owner:  nobody
         Type:                       |                   Status:  new
  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
-------------------------------------+-------------------------------------

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 <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.5d5a288db67c1f126fc24de8dfd76b18%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [Django] #28402: Provide 'temporary_file_path' for InMemoryUploadedFile

Django
In reply to this post by Django
#28402: Provide 'temporary_file_path' for InMemoryUploadedFile
-------------------------------------+-------------------------------------
     Reporter:  Thomas Güttler       |                    Owner:  nobody
         Type:                       |                   Status:  closed
  Cleanup/optimization               |
    Component:  File                 |                  Version:  1.11
  uploads/storage                    |
     Severity:  Normal               |               Resolution:  wontfix
     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>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [hidden email].
To post to this group, send email to [hidden email].
To view this discussion on the web visit https://groups.google.com/d/msgid/django-updates/065.f425546a2279c8fa9810e6315bfb0589%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.
Loading...