Temporary uploaded file API (TemporaryFile) is inadequate

Jonathan,

Since the UploadedFile objects aren't necessarily files on the file system, we cannot just provide all of the methods that are available to the same object as open('myfile', 'r').

However, that doesn't mean we can't try to provide as much as possible. It's just that the more we provide, the more limiting we are on what UploadedFile can do -- it's a balancing act.

In this attached patch, the following changes are made, which will hopefully help what you are trying to accomplish:

1. TemporaryUploadFile inherits from both UploadedFile and the class of the object that tempfile.NamedTemporaryFile() returns.
2. All UploadedFile objects now support __iter__(), readlines(), and xreadlines().

In addition to this, the patch also updates some of the API and attempts to fix #7614.

Please try this patch and let me know if it solves any issue you are having.

Cheers,
Mike

Overall cleanups for r7814...addressing all API changes that I know of.

Thanks!

It looks like the patch solved my issues, except I wasn't able to completely test it.

There are a few issues in the patch that I had to resolve: In uploadedfile.py, you have:

if settings.FILE_UPLOAD_TEMP_DIR:

It will fail with 'module' object has no attribute 'FILE_UPLOAD_TEMP_DIR'. You should probably do getattr(settings, 'FILE_UPLOAD_TEMP_DIR', None) instead.

I think the same applies to settings.FILE_UPLOAD_MAX_MEMORY_SIZE

I also get an error for temporary files

  217.         obj.__class__ = type('TemporaryUploadedFile', (obj.__class__, UploadedFile), obj.__dict__)

Exception Type: TypeError at /db/job/2/run/
Exception Value: __class__ must be set to a class

I couldn't figure out how to resolve this.

In the code

        return InMemoryUploadedFile(
            file = self.file,
            field_name = self.field_name,
            file_name = self.file_name,
            content_type = self.content_type,
            file_size = file_size,
            charset = self.charset
        )

file_name should be name and file_size should be size, otherwise it errors out.

Both FILE_UPLOAD_* settings constants exist in django/conf/global_settings.py. So you shouldn't ever see "module has no attribute" errors unless your Django installation isn't correct or you're manually configuring settings and didn't pass in a default (in which case the error is in your code, not Django's).

Well, then the specific problem is that django.core.files.uploadedfile.py does:

import settings

Rather than

from django.conf import settings

Replying to Jonathan Hseu <vomjom@gmail.com>:

Well, then the specific problem is that django.core.files.uploadedfile.py does: {{{ import settings }}} Rather than {{{ from django.conf import settings }}}

I noticed this too a couple days ago, but unfortunately haven't been in internet contact.

As for the type() usage, I noticed that the methods dictionary should be {}, but that shouldn't cause the problem you're seeing... what version of Python are you using?

Replying to axiak:

As for the type() usage, I noticed that the methods dictionary should be {}, but that shouldn't cause the problem you're seeing... what version of Python are you using?

2.5.2. I tried reproducing it, and this is what I've found:

>>> class A(object):
...   pass
... 
>>> a = A()
>>> a.__class__ = type('foo', (object,), {})
>>> a
<__main__.foo object at 0x7f5d3e62e4d0>
>>> class B:
...   pass
... 
>>> b = B()
>>> b.__class__ = type('bar', (object,), {})
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: __class__ must be set to a class

So I imagine the problem is:

>>> import tempfile
>>> f = tempfile.NamedTemporaryFile(mode='w', suffix='.upload')
>>> f.__class__
<class tempfile._TemporaryFileWrapper at 0x7f414f12b1d0>

And in my tempfile.py, _TemporaryFileWrapper has no parent class.

Update to the patch...thanks vomjom!

This is being fixed with the same patch as #7614, so I'm marking this as a duplicate. It's not, exactly, but it'll get fixed at the same time.

(In [7859]) Fixed #7614: the quickening has come, and there now is only one UploadedFile?. On top of that, UploadedFile?'s interface has been improved:

• The API now more closely matches a proper file API. This unfortunately means a few backwards-incompatible renamings; see BackwardsIncompatibleChanges. This refs #7593.
• While we were at it, renamed chunk() to chunks() to clarify that it's an iterator.
• Temporary uploaded files now property use the tempfile library behind the scenes which should ensure better cleanup of tempfiles (refs #7593 again).

Thanks to Mike Axiak for the bulk of this patch.

Thanks Mike for the new SVN patch. It works great except for a few minor things:

1. Please put a tell() function in TemporaryUploadedFile?

2. Please also map the file functions to the StringIOs in InMemoryUploadedFile? and SimpleUploadedFile?.

I currently have an awkward situation where my project works well if the file is large enough (and so goes to a temporary file), but fails on small files (and becomes an InMemoryUploadedFile?). It also fails in my unit tests (SimpleUploadedFile?).

#7769 is tracking these improvements. Jonathan, in general, please don't reopen fixed tickets to add more patches; new tickets are far preferred.