Opened 11 years ago
Closed 5 years ago
#21238 closed Bug (fixed)
`ImageFieldFile.url` raises an AttributeError exception after retrieving it from cache
Reported by: | Pierre Dulac | Owned by: | Hasan Ramezani |
---|---|---|---|
Component: | Core (Cache system) | Version: | dev |
Severity: | Normal | Keywords: | imagefield, cache |
Cc: | sky.kok@…, lucmult@… | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Hi everyone,
To reproduce the exception, just cache an instance of ImageFieldFile
then retrieve it and call the url
property on this retrieved instance.
Here is a minimal code to reproduce :
from django.core.cache import cache from django.db import models class A(models.Model): pictogram = models.ImageField(upload_to='images') class Meta: app_label = 'test' a = A() a.pictogram = 'fake/test/url.png' original_picto = a.pictogram cache_key = 'picto' cache.set(cache_key, original_picto, 60) cached_picto = cache.get(cache_key) print original_picto.url print cached_picto.url
and you get
AttributeError: 'ImageFieldFile' object has no attribute 'storage'
Didn't tried to fix it yet...
Have a nice day,
Pierre
Attachments (2)
Change History (20)
comment:1 by , 11 years ago
Summary: | `ImageFieldFile.url` raise an AttributeError after retrieving it from cache → `ImageFieldFile.url` raises an AttributeError after retrieving it from cache |
---|
comment:2 by , 11 years ago
Summary: | `ImageFieldFile.url` raises an AttributeError after retrieving it from cache → `ImageFieldFile.url` raises an AttributeError exception after retrieving it from cache |
---|
comment:3 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:4 by , 11 years ago
Here is the pull request (based on master/1.7) that can fix this problem: https://github.com/django/django/pull/1781
comment:5 by , 11 years ago
Has patch: | set |
---|
comment:6 by , 11 years ago
I thought fixing the inheritance chain by having django.core.files.base.File.__init__
call super()
would be the way to fix this. Thoughts?
comment:7 by , 11 years ago
I don't understand. django.core.files.base.File has only one parent, which is django.core.files.utils.FileProxyMixin which does not have __init__
method.
I try to do this in django/core/files/base.py:
def __init__(self, file, name=None): + super(File, self).__init__() self.file = file if name is None: name = getattr(file, 'name', None) self.name = name if hasattr(file, 'mode'): self.mode = file.mode + # or even this, but it does not work + # super(File, self).__init__()
Before resorting to __reduce__
way, I did try to solve the problem by playing with super
and __init__
in some classes related with ImageFieldFile
, either I missed something or it did not work that way.
comment:8 by , 11 years ago
Ok, thanks for the response -- I guess my initial hunch wasn't correct. To be honest, I'm not super familiar with the internals of how pickle and __reduce__
work so I have some reading to do. If you could include a quick explanation of your understanding of the problem and why this solution works, that would be quite helpful for me.
comment:9 by , 11 years ago
Cc: | added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
Version: | 1.5 → master |
Sorry, the original PR does not work with Python 2.7. Also, __reduce__
is prone to error. It's better to use __getstate__
. I'll already updated the PR. https://github.com/django/django/pull/1781
But first, let me give you the lesson of pickling. :) Naturally, without human intervention, the pickling (pickle.dumps) will copy the instance's dict. So why in this case the all ImageFieldFile instance's dict members (notably the storage attribute) do not get copied. The ImageFieldFile class does not define any method that can intervene the pickling process (__getinitargs__
, __getnewargs__
, __getstate__
, __setstate__
, __reduce__
, __reduce_ex__
). But the ancestor of ImageFieldFile, which is FieldFile, does have method that intervenes pickling process.
def __getstate__(self): # FieldFile needs access to its associated model field and an instance # it's attached to in order to work properly, but the only necessary # data to be pickled is the file's name itself. Everything else will # be restored later, by FileDescriptor below. return {'name': self.name, 'closed': False, '_committed': True, '_file'
That's why the storage attribute of ImageFieldFile instance is missing because only name, closed, _committed attributes are copied in the pickling process.
There is an alternative solution for this problem beside my PR, which is removing the __getstate__
method from FieldFile. The downside is the dump result of FieldFile will be unnecessarily bigger (based on my understanding of the comment). I am not sure whether there is another ill effects or not. Attached the patch to do that.
by , 11 years ago
Attachment: | alternative_patch_ticket_21238.patch added |
---|
comment:10 by , 11 years ago
Cc: | added |
---|
comment:11 by , 10 years ago
Cc: | added |
---|---|
Patch needs improvement: | set |
Won't ImageFieldFile
be bigger if we override its __getstate__
? I'm attaching a patch with a test that shows FieldFile
has the same problem (AttributeError: 'FieldFile' object has no attribute 'storage'
when unpickling) so I think we need a solution that covers both classes.
by , 10 years ago
Attachment: | fieldfile_pickle_test.diff added |
---|
comment:12 by , 5 years ago
Owner: | changed from | to
---|---|
Patch needs improvement: | unset |
comment:13 by , 5 years ago
Patch needs improvement: | set |
---|
comment:14 by , 5 years ago
Cc: | removed |
---|
comment:15 by , 5 years ago
Patch needs improvement: | unset |
---|
comment:16 by , 5 years ago
Triage Stage: | Accepted → Ready for checkin |
---|
ImageFieldFile
inherits fromImageFile
andFieldFile
. It looks like the problem is due to the fact thatFieldFile.__init__
isn't called when the object is retrieved from the cache. I think the inheritance chain is broken becauseImageFile.__init__
(inheriting fromdjango.core.files.base.File
) doesn't callsuper
.