After r10737, ImageField sets height and width on model creation, every lookup

[Michael Newman]

I am going to give this a little more time, but I wanted to file quickly.

r10737 was a large commit that changes the way that files variables are handled. On every lookup of ImageField, the image is opened on the OS. This causes huge IO overhead. Also if the file is unavailable, an uncaught OSError is raised, creating errors in templates if the filesystem is unavailable.

[Michael Newman]

(Contrived) failing test

[Michael Newman]

So after a little digging I have figured out that it has to do with the ImageFileDescriptor, ​http://code.djangoproject.com/browser/django/trunk/django/db/models/fields/files.py#L297 , where set is called on creation of the instance, but also on database lookups down the line, meaning that if you have the height and width field defined, any time you want to use the ImageField on a model, it opens the Image file.

[Armin Ronacher]

untested proposal for a quick fix.

[Michael Newman]

My test, which checks if the dimensions are accessed after the python object is initialized, passes with this patch, but the second problem, which is that the image height and width are calculated every time the object is puled from the database, isn't fixed.

I am going to look into writing more tests for this problem.

[Jeremy Dunck]

Patch was backwards, fixed now with passing tests.

[onemoreryan]

Just ran up against this issue today, came to file a ticket, and found this one.

I have a Photo model, and one record's ImageField referenced a .jpg that was corrupt. The file was unable to open, so even bringing up the changelist in the admin would trigger a TypeError of can't unpack non-sequence. This .jpg won't upload via normal Django admin functions, but a process that creates a Photo record without going through the normal upload can (and did) still get it into the system. Obviously something we need to address, but previous to r10737, this would only cause an error on the single record rather than borking the entire changelist.

This TypeError was thrown by line 327 here -- ​http://code.djangoproject.com/browser/django/trunk/django/db/models/fields/files.py#L327 -- because it couldn't open the file to grab a height, width tuple. Removing the width_field and height_field args from my model's ImageField makes the problem disappear, naturally, as does adding an Except within ImageFileDescriptor. Not suggesting these are good long-term solutions, just trying to provide more information about this behavior. Will attach a broken.jpg here in case anyone wants to use it in testing.

[onemoreryan]

Example of a corrupt .jpg; an ImageField that references this file will cause a TypeError on that model's admin changelist

[Gary Wilson]

(In [10858]) Changes to ImageFileDescriptor and ImageField to fix a few cases of setting image dimension fields.

• Moved dimension field update logic out of ImageFileDescriptor.__set__ and into its own method on ImageField.
• New ImageField.update_dimension_fields method is attached to model instance's post_init signal so that:
  • Dimension fields are set when defined before the ImageField.
  • Dimension fields are set when the field is assigned in the model constructor (fixes #11196), but only if the dimension fields don't already have values, so we avoid updating the dimensions every time an object is loaded from the database (fixes #11084).
• Clear dimension fields when the ImageField is set to None, which also causes dimension fields to be cleared when ImageFieldFile.delete() is used.
• Added many more tests for ImageField that test edge cases we weren't testing before, and moved the ImageField tests out of file_storage and into their own module within model_fields.

[ccahoon]

(In [10987]) Changes to ImageFileDescriptor and ImageField to fix a few cases of setting image dimension fields.

• Moved dimension field update logic out of ImageFileDescriptor.__set__ and into its own method on ImageField.
• New ImageField.update_dimension_fields method is attached to model instance's post_init signal so that:
  • Dimension fields are set when defined before the ImageField.
  • Dimension fields are set when the field is assigned in the model constructor (fixes #11196), but only if the dimension fields don't already have values, so we avoid updating the dimensions every time an object is loaded from the database (fixes #11084).
• Clear dimension fields when the ImageField is set to None, which also causes dimension fields to be cleared when ImageFieldFile.delete() is used.
• Added many more tests for ImageField that test edge cases we weren't testing before, and moved the ImageField tests out of file_storage and into their own module within model_fields.

[Jacob]

Milestone 1.1 deleted