#11857 closed Bug (fixed)
TemporaryFile class is missing the closed - attribute
Reported by: | andi100 | Owned by: | |
---|---|---|---|
Component: | File uploads/storage | Version: | |
Severity: | Normal | Keywords: | TemporaryFile closed windows |
Cc: | petar.maric@…, jdunck@…, pombredanne@…, christopher.r.adams@… | Triage Stage: | Accepted |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
Hi,
the TemporaryFile-class (core/files/temp.py), is missing the 'closed'-attribute. It inherits from FileProxyMixin, but this class also doesn't have a closed-attribute.
If I want to access the 'width' attribute of an ImageFieldFile, I get an "'TemporaryFile' object has no attribute 'closed'" - error.
Traceback:
File "D:\websites\picality\src\picalityproject\picality\models\image.py" in save 98. print self.image.width File "D:\websites\django-trunk\django\core\files\images.py" in _get_width 15. return self._get_image_dimensions()[0] File "D:\websites\django-trunk\django\core\files\images.py" in _get_image_dimensions 24. close = self.closed File "D:\websites\django-trunk\django\db\models\fields\files.py" in _get_closed 126. return file is None or file.closed File "D:\websites\django-trunk\django\core\files\base.py" in _get_closed 51. return not self.file or self.file.closed Exception Type: AttributeError at /upload/ Exception Value: 'TemporaryFile' object has no attribute 'closed'
But somehow this error occurs only if I want to upload 2 image files in the same form -> because if I upload only one image, the self.image.file.file is a StringIO object, which has a 'closed' attribute.
Attachments (1)
Change History (36)
comment:1 by , 15 years ago
Has patch: | set |
---|---|
Needs tests: | set |
Owner: | changed from | to
Patch needs improvement: | set |
Status: | new → assigned |
by , 15 years ago
Attachment: | quickfix patch for 11857 against Django 1.2 pre-alpha SVN-11477.diff added |
---|
comment:2 by , 15 years ago
Cc: | added |
---|
comment:3 by , 15 years ago
Triage Stage: | Unreviewed → Accepted |
---|
follow-up: 8 comment:4 by , 15 years ago
This issue is still present, as I've had the same problem.
However, the patch attached will not work if inserted as presented. 'temp.py' has had a lot of it's contents moved into 'tempfile.py' and there's apparently different classes now for temporary files based on whether you're using 'nt' or not. I am developing on Windows so this is of concern to me.
comment:5 by , 14 years ago
Severity: | → Normal |
---|---|
Type: | → Bug |
comment:6 by , 13 years ago
Easy pickings: | unset |
---|---|
UI/UX: | unset |
Version: | SVN |
Bug still present! Django 1.3
File "C:\Python26\lib\site-packages\django\forms\forms.py" in full_clean 269. self._post_clean() File "C:\Python26\lib\site-packages\django\forms\models.py" in _post_clean 308. self.instance = construct_instance(self, self.instance, opts.fields, opts.exclude) File "C:\Python26\lib\site-packages\django\forms\models.py" in construct_instance 53. f.save_form_data(instance, cleaned_data[f.name]) File "C:\Python26\lib\site-packages\django\db\models\fields\files.py" in save_form_data 281. setattr(instance, self.name, data) File "C:\Python26\lib\site-packages\django\db\models\fields\files.py" in __set__ 314. self.field.update_dimension_fields(instance, force=True) File "C:\Python26\lib\site-packages\django\db\models\fields\files.py" in update_dimension_fields 382. width = file.width File "C:\Python26\lib\site-packages\django\core\files\images.py" in _get_width 15. return self._get_image_dimensions()[0] File "C:\Python26\lib\site-packages\django\core\files\images.py" in _get_image_dimensions 24. close = self.closed File "C:\Python26\lib\site-packages\django\db\models\fields\files.py" in _get_closed 127. return file is None or file.closed File "C:\Python26\lib\site-packages\django\core\files\base.py" in _get_closed 51. return not self.file or self.file.closed Exception Type: AttributeError at /photo/upload/ Exception Value: 'TemporaryFile' object has no attribute 'closed'
comment:7 by , 13 years ago
Note it's is not necessary or or particularly helpful to post that still-open bugs are still recreatable. This bug is open, there is no indication anywhere in it that I can see that it has been fixed, so it's expected that it's still present. If you find an open bug that can no longer be recreated, that would be something to post (and possibly close the ticket), but posting that open bugs are still there is just noise.
comment:8 by , 13 years ago
Easy pickings: | set |
---|---|
Patch needs improvement: | unset |
Replying to danostrowski:
This issue is still present, as I've had the same problem.
However, the patch attached will not work if inserted as presented. 'temp.py' has had a lot of it's contents moved into 'tempfile.py' and there's apparently different classes now for temporary files based on whether you're using 'nt' or not. I am developing on Windows so this is of concern to me.
The patch works fine for me, and I don't see why it wouldn't for you. What issues are you having with it? Also, tempfile is not part of Django, it is imported from the Python Standard Library.
temp.py just tells Django what class to use for temporary files. If you are using a Windows NT OS, temp.py will define a custom TemporaryFile class for Django to use. If you are not using a Windows NT OS, temp.py will tell Django to just use the tempfile.NamedTemporaryFile class. The reason for this is that NamedTemporaryFile cannot be reopened with a Windows NT OS.
This bug is caused because the custom TemporaryFile object Django defines for NT does not have a closed property, but self.closed is accessed when using the inherited open method. If you are not using NT, this bug won't affect you.
The patch seems to add the property with the expected behavior. In addition to the changes made by the patch, it seems like it would be cleaner to only use the custom TemporaryFile object regardless of OS. It's a pretty simple class and I don't see any platform specific notes for the Standard Library functionality used by it.
Either way, just applying the patch should work. It just needs a test.
I'll try to get to it if I have time, but I'm pretty busy at the moment.
comment:9 by , 13 years ago
The patch works fine for me.
Environment - Windows 2008 server, Apache + mod_wsgi, Django 1.3
comment:11 by , 13 years ago
Keywords: | windows added |
---|
comment:14 by , 12 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:15 by , 12 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:16 by , 12 years ago
Cc: | added |
---|
comment:17 by , 12 years ago
I experienced the same issue on win7 and django 1.5.1.
Susan, do you want a patch?
comment:18 by , 12 years ago
Cc: | added |
---|
comment:19 by , 12 years ago
Sure, I'll appreciate. Since Im running macOS, and the bug appears only on windowsOS, I think I may have difficulty replicating this bug.
comment:21 by , 12 years ago
Owner: | changed from | to
---|
comment:22 by , 12 years ago
Since Python 2.6, tempfile.NamedTemporaryFile
has a new attribute: delete=True/False
which likely removes the needs entirely to have a django.core.temp.NamedTemporaryFile
wrapper specifically for 'nt'. I will investigate this since 2.6 is a requirement for 1.5.x. The lack of a closed attribute is something that happens only in some contexts and is something that is injected elsewhere and I am still trying to understand where/when exactly.
comment:23 by , 11 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:24 by , 11 years ago
I am unassigning this ticket from me, as I am not able to work on it in the short term ... but this is still on my radar
comment:25 by , 11 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:26 by , 11 years ago
Needs tests: | unset |
---|
comment:27 by , 11 years ago
Submitted a pull request that should solve this at https://github.com/django/django/pull/1566. This could probably easily be made into a bug fix for previous versions of Django, as it's just a few lines of code and the issue seems to be very old.
comment:28 by , 11 years ago
- TemporaryFile now minimally mocks the API of the Python standard library class tempfile.NamedTemporaryFile to avoid AttributeError exceptions.
- The symbol django.core.files.NamedTemporaryFile is actually assigned as a different class on different operating systems.
- The bug only occurred if Django is running on Windows, hence why it was hard to diagnose.
- All tests pass with ./runtests.py --settings=test_sqlite
comment:29 by , 11 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:30 by , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Triage Stage: | Accepted → Unreviewed |
comment:31 by , 11 years ago
Resolution: | fixed |
---|---|
Status: | closed → new |
Triage Stage: | Unreviewed → Accepted |
Please don't close a ticket until the fix has been committed to master.
comment:33 by , 11 years ago
Owner: | set to |
---|---|
Resolution: | → fixed |
Status: | new → closed |
Run into the same error, under different conditions.
I made a quick fix (works for me!) as I'm a bit busy these days to do things properly. When I catch a breath I'll improve the patch with comments and tests.