#10300: Custom File Storage Backend broken by recent SVN commit.
-------------------------------------------+--------------------------------
Reporter: erikcw | Owner: nobody
Status: new | Milestone:
Component: File uploads/storage | Version: SVN
Resolution: | Keywords: file upload
storage s3
Stage: Accepted | Has_patch: 0
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
-------------------------------------------+--------------------------------
Changes (by kmtracey):
* stage: Unreviewed => Accepted
Comment:
I looked at this a little, though I cannot recreate since I do not have
access to S3 (let alone the modified backend...the modifications may be
relevant here). However, taking the S3 backend you point to and stubbing
it out to not actually attempt to call S3 services but just log what it's
called to do, I see a difference pre- and post- r9766 involving calls to
the backend's `size` method. Specifically, prior to r9766 the S3Storage
`size` method is not called when a file is uploaded (via admim). Running
with r9766, however, it is called. Ultimately the call comes from the
len(content) call in the !FieldFile save method in
django/db/models/fields/files.py:
{{{
#!python
def save(self, name, content, save=True):
name = self.field.generate_filename(self.instance, name)
self._name = self.storage.save(name, content)
setattr(self.instance, self.field.name, self.name)
# Update the filesize cache
print 'Prior to updating filesize cache:
content.__class__.__bases__ is: %s' % str(content.__class__.__bases__)
self._size = len(content)
# Save the object because it has changed, unless save is False
if save:
self.instance.save()
save.alters_data = True
}}}
Though this method itself did not change, that print (added by me for
debug) shows that the bases for content has changed. Prior to r9766 it
prints:
{{{
Prior to updating filesize cache: content.__class__.__bases__ is: (<class
'django.core.files.uploadedfile.UploadedFile'>,)
}}}
Post-r9766 it prints:
{{{
Prior to updating filesize cache: content.__class__.__bases__ is: (<class
'django.core.files.uploadedfile.InMemoryUploadedFile'>, <class
django.db.models.fields.files.FieldFile'>)
}}}
The effect of this difference is that instead of django/core/files/base.py
File's _get_size:
{{{
#!python
def _get_size(self):
if not hasattr(self, '_size'):
if hasattr(self.file, 'size'):
self._size = self.file.size
elif os.path.exists(self.file.name):
self._size = os.path.getsize(self.file.name)
else:
raise AttributeError("Unable to determine the file's
size.")
return self._size
def _set_size(self, size):
self._size = size
size = property(_get_size, _set_size)
}}}
being called to determine len(content) (with _size having already been set
previously at a point in the code I didn't track down),
django/db/models/fields/files.py !FieldFile's _get_size:
{{{
#!python
def _get_size(self):
self._require_file()
return self.storage.size(self.name)
size = property(_get_size)
}}}
is called instead.
Now the `_save` method in the S3 storage backend you point to does not
rely on len(content) when it saves the data, but if your modified version
does then it likely has a problem. If I put some prints into `_save`:
{{{
#!python
print 'in _save: type(content) = %s, content.__class__.__bases__
is: %s' % (str(type(content)), str(content.__class__.__bases__))
print 'in _save: len(content): %d' % len(content)
}}}
Prior to r9766 I get:
{{{
in _save: type(content) = <class
'django.core.files.uploadedfile.InMemoryUploadedFile'>,
content.__class__.__bases__ is: (<class
'django.core.files.uploadedfile.UploadedFile'>,)
in _save: len(content): 818
}}}
Post r9766 I get:
{{{
in _save: type(content) = <class
'django.db.models.fields.files.InMemoryUploadedFile'>,
content.__class__.__bases__ is: (<class
'django.core.files.uploadedfile.InMemoryUploadedFile'>, <class
'django.db.models.fields.files.FieldFile'>
}}}
and my stubbed backend's `size` raises an exception because it is called
to return the length of a file it hasn't saved yet.
So it seems there is a fairly non-obvious side-effect of r9766 involving
len(content) for the content passed into the backend `_save`:
* Prior to r9766 this would resolve to django/core/files/base.py File's
_get_size method
* Post-r9766 the backend's own `size` method will get called, which isn't
likely to work since `_save` itself hasn't had a chance to save the file
yet.
But I don't actually know for sure that this is the cause of the problem
reported here, since I don't know that the modified backend relies on
len(content) in its `_save` method. It does seem to be a problem we need
to fix, though, since it seems valid for a backend `_save` to call len on
the passed content, and it used to work...now it will find itself called
to answer the len() question, which likely isn't going to work.
This is likely related to #10249...that one is reporting inability to
determine a method resolution order while this one seems to be resulting
from the fact that the method resolution order has changed.
Feedback from someone with a better understanding of r9766 would be
helpful here.
--
Ticket URL: <http://code.djangoproject.com/ticket/10300#comment:3>
Django <http://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 post to this group, send email to django-updates@googlegroups.com
To unsubscribe from this group, send email to
django-updates+unsubscr...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/django-updates?hl=en
-~----------~----~----~----~------~----~------~--~---
