Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error when trying to access width/height after url in templates #350

Closed
a1tus opened this issue Dec 15, 2015 · 19 comments · Fixed by #384
Closed

Error when trying to access width/height after url in templates #350

a1tus opened this issue Dec 15, 2015 · 19 comments · Fixed by #384
Labels

Comments

@a1tus
Copy link
Contributor

a1tus commented Dec 15, 2015

Error (see traceback below) occurs if you generate image with assignment tag and then access width or height after url. If you use width/height before url - everything goes fine. Also error disappears if you try to access already generated images.

All the related settings are the default ones.

Examples:

from django.template import Template, Context
tpl = "{% load imagekit %}{% generateimage 'spec_name' source=obj.photo as im %} {{ im.url }} {{ im.width }}"
Template(tpl).render(Context({'obj': obj}))  # error
# but no error for this:
tpl = "... {{ im.width }} {{ im.url }}"
# and also no error if thumb was already generated before render call

It's quite strange and I'm not sure if that problem existed before because even in docs there's an example that is doing exactly the same things.

Traceback:

.../lib/python3.4/site-packages/django/core/files/images.py in _get_width(self)
     16     """
     17     def _get_width(self):
---> 18         return self._get_image_dimensions()[0]
     19     width = property(_get_width)
     20

.../lib/python3.4/site-packages/django/core/files/images.py in _get_image_dimensions(self)
     26         if not hasattr(self, '_dimensions_cache'):
     27             close = self.closed
---> 28             self.open()
     29             self._dimensions_cache = get_image_dimensions(self, close=close)
     30         return self._dimensions_cache

.../lib/python3.4/site-packages/imagekit/files.py in open(self, mode)
     57     def open(self, mode='rb'):
     58         self._require_file()
---> 59         self.file.open(mode)
     60
     61     def _get_closed(self):

.../lib/python3.4/site-packages/django/core/files/base.py in open(self, mode)
    139             self.file = open(self.name, mode or self.mode)
    140         else:
--> 141             raise ValueError("The file cannot be reopened.")
    142
    143     def close(self):

PS. Confirmed for ImageKit 3.3 / Django 1.8-1.9 / Python 3.4.3

@Terr
Copy link

Terr commented Dec 21, 2015

I have encountered the same issue with ImageKit 3.3, Python 2.7.5 and Django 1.9.

I haven't been able to trace it completely, but it seems that after the cache creation the File object that gets passed on points to a temporary file (in /tmp, in my case) instead of the newly created cache file.

This temporary file gets deleted sometime during the process which triggers the exception.

@a1tus
Copy link
Contributor Author

a1tus commented Dec 21, 2015

@Terr, exactly! The simple workaround is to use width before src like <img width="{{ img.width }}" src="{{ img.url }}"> but sometimes it's not handy (or even possible) because it can be used in different tags etc. Also I've found that this issue doesn't appear in generate tag without assignment only because its attributes is filled with dimension earlier (before src). I've swapped the lines and got the same error.

@vstoykov
Copy link
Collaborator

I investigated the issue and found the line causing the problem.

When image is generated then temporary file is created and attached to the ImageCacheFile instance to prevent call to the storage backend on next access the file attribute. The problem is that this temporary file is removed from the file system when it is closed and when django try to reopen it then it raise an error.

I will try to see a better way of generating thumbnails that will fix the issue but if possible without extra calling the storage backend for performance reason.

For now you can use the workaround with putting the width before the src in the markup until fix is ready.

@vstoykov vstoykov added the bug label Dec 27, 2015
@troygrosfield
Copy link

Any updates on this? I've encountered the same thing continually running into the after upgrading to imagekit 3.3:

raise ValueError("The file cannot be reopened.")

Without much luck for a workaround. In my case, I'm trying to generate two thumbnails from a single image. When setting the models ProcessedImageField to the file value prior to imagekit 3.3, everything worked as expected. After the update, I see the error throughout my test suite runs. This is an issue because I'm in the middle of upgrading to django 1.9 and imagekit < 3.3 doesn't work with django 1.9. :(

@troygrosfield
Copy link

To further elaborate on the issue, my model roughly looks like the following:

from imagekit.models.fields import ProcessedImageField

class MyModel(models.Model):
    original = ProcessedImageField(upload_to='somepath',
                                   processors=[XLargeImageProcessor()],
                                   blank=True, null=True)
    thumbnail = ProcessedImageField(
        upload_to='somepath',
        processors=[ResizeToFit(width=500,
                                height=200,
                                upscale=True)],
        options={'quality': 90},
        blank=True,
        null=True)
    thumbnail_square = ProcessedImageField(
        upload_to='somepath',
        processors=[ResizeToFill(200, 200)],
        blank=True,
        null=True)

    def save(self, *args, **kwargs):
        if not self.thumbnail:
            self.thumbnail = self.original.file

        if self.thumbnail and \
           hasattr(self.thumbnail, 'file') and \
           not self.thumbnail_square:
            self.thumbnail_square = self.thumbnail.file

        super(MyModel, self).save(*args, **kwargs)

When saving, that's when I see the:

raise ValueError("The file cannot be reopened.")

This worked without issues with django 1.8.8, imagekit 3.2.7.

@vstoykov
Copy link
Collaborator

Ok I will check this. Thanks for the info.

@troygrosfield
Copy link

No problem. I'm in the middle of an upgrade right now so let me know if I can help in any way. Thanks.

@vstoykov
Copy link
Collaborator

@troygrosfield Your code works with Django==1.8.8 and django-imagekit==3.2.7 and does not work with same Django but django-imagekit==3.3?

@a1tus Can you confirm that your code works with django-imagekit==3.2.7 and not with 3.3 or it does not work with 3.2.7 also?

@troygrosfield
Copy link

I was mistaken, the error actually happened with the change between django-imagekit 3.2.6 and 3.2.7. I just assumed it was the 3.3 version because I jumped from 3.2.6 to 3.3. Here's the tests that show the issue:

Models.py

from imagekit.models.fields import ProcessedImageField
from pilkit.processors.resize import ResizeToFill
from pilkit.processors.resize import ResizeToFit

class MyModel(models.Model):

    original = ProcessedImageField(blank=True, null=True)
    thumbnail = ProcessedImageField(
        processors=[ResizeToFit(width=500,
                                height=200,
                                upscale=True)],
        options={'quality': 90},
        blank=True,
        null=True)
    thumbnail_square = ProcessedImageField(
        processors=[ResizeToFill(200, 200)],
        blank=True,
        null=True)

    def save(self, *args, **kwargs):
        if not self.thumbnail:
            self.thumbnail = self.original.file

        if self.thumbnail and \
           hasattr(self.thumbnail, 'file') and \
           not self.thumbnail_square:
            self.thumbnail_square = self.original.file

        super(MyModel, self).save(*args, **kwargs)

Tests

class ModelTests(TestCase):

    def test_my_model(self):
        img = open('/Users/troy/tmp/test-pic.jpg', 'rb')
        spec = ImageSpec(source=img)

        with NamedTemporaryFile(delete=True) as tmp:
            tmp.write(spec.generate().read())
            tmp.flush()
            obj = MyModel.objects.create(original=ImageFile(tmp))

        self.assertIsNotNone(obj.original)
        self.assertIsNotNone(obj.thumbnail)
        self.assertIsNotNone(obj.thumbnail_square)

All test passed when running against:

pip install -U django==1.8.8 django-imagekit==3.2.6

However, when running tests against:

pip install -U django==1.8.8 django-imagekit==3.2.7

I get the following error:

Traceback (most recent call last):
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/imagekit/specs/__init__.py", line 149, in generate
    img = open_image(self.source)
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/pilkit/utils.py", line 21, in open_image
    target.seek(0)
  File "/usr/local/Cellar/python3/3.4.3/Frameworks/Python.framework/Versions/3.4/lib/python3.4/tempfile.py", line 399, in func_wrapper
    return func(*args, **kwargs)
ValueError: seek of closed file

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/Users/troy/github/django-core/tests/test_models.py", line 37, in test_my_model
    obj = MyModel.objects.create(original=ImageFile(tmp))
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/django/db/models/manager.py", line 127, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/django/db/models/query.py", line 348, in create
    obj.save(force_insert=True, using=self.db)
  File "/Users/troy/github/django-core/tests/test_objects/models.py", line 107, in save
    super(MyModel, self).save(*args, **kwargs)
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/django/db/models/base.py", line 734, in save
    force_update=force_update, update_fields=update_fields)
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/django/db/models/base.py", line 762, in save_base
    updated = self._save_table(raw, cls, force_insert, force_update, using, update_fields)
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/django/db/models/base.py", line 846, in _save_table
    result = self._do_insert(cls._base_manager, using, fields, update_pk, raw)
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/django/db/models/base.py", line 885, in _do_insert
    using=using, raw=raw)
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/django/db/models/manager.py", line 127, in manager_method
    return getattr(self.get_queryset(), name)(*args, **kwargs)
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/django/db/models/query.py", line 920, in _insert
    return query.get_compiler(using=using).execute_sql(return_id)
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/django/db/models/sql/compiler.py", line 973, in execute_sql
    for sql, params in self.as_sql():
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/django/db/models/sql/compiler.py", line 931, in as_sql
    for obj in self.query.objs
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/django/db/models/sql/compiler.py", line 931, in <listcomp>
    for obj in self.query.objs
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/django/db/models/sql/compiler.py", line 929, in <listcomp>
    ) for f in fields
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/django/db/models/fields/files.py", line 314, in pre_save
    file.save(file.name, file, save=False)
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/imagekit/models/fields/files.py", line 12, in save
    content = generate(spec)
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/imagekit/utils.py", line 134, in generate
    content = generator.generate()
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/imagekit/specs/__init__.py", line 153, in generate
    self.source.open()
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/django/db/models/fields/files.py", line 81, in open
    self.file.open(mode)
  File "/Users/troy/.virtualenvs/py34/lib/python3.4/site-packages/django/core/files/base.py", line 141, in open
    raise ValueError("The file cannot be reopened.")
ValueError: The file cannot be reopened.

@vstoykov
Copy link
Collaborator

Thanks a lot for bisecting this and giving to me a code with which I will reproduce the problem. When I have time I will check this to identify the issue.

@udanieli
Copy link

Had the same issue with 3.3, I don't use the template tag.
Downgrading to 3.2.6 seems the solution.

@jakob-o
Copy link

jakob-o commented May 31, 2016

Same issue here, not using the template tag, imagekit 3.2.7

@vstoykov
Copy link
Collaborator

@a1tus @Terr @troygrosfield @dbortolz @jakob-o Hello guys and sorry for taking so long. Can some of you test the code in #380 to see if the problem persists?

I was unable to recreate a test case with this exact problem but when I was trying to port django-imagekit to Django 1.10 I received an error related to some temporary file (created from imagekit) which I suspect is the reason for this problem.

If some of you confirm that the problem no longer persists then we will have "two rabbits with one bullet".

@vstoykov vstoykov mentioned this issue Jul 16, 2016
@vstoykov
Copy link
Collaborator

Hi guys I was able to reproduce the issue outside of the test and found that the regression happened in 3.2.7 by this PR #335.

The issue is only happening if cached thumbnail was not present in the system and need to be generated on the fly.

I'm trying to see for a solution for this

@matthewwithanm
Copy link
Owner

While you're thinking about it, it was a long-standing issue that width and height should be cached too 😊

vstoykov added a commit to vstoykov/django-imagekit that referenced this issue Jul 17, 2016
…ter url

If the file is closed and something is calling `open` now the file will be opened correctly event if it was already closed
@vstoykov
Copy link
Collaborator

@matthewwithanm when I prepared the patch for enabling Django 1.10 support I found that probably they need to be cached for performance improvement but still the underling issue need to be solved first.

I think that now I solved the problem in #384 (this time for real). If someone can confirm we can merge it and probably release a new version.

@matthewwithanm
Copy link
Owner

👍 Keep up there good work!

@mgalgs
Copy link

mgalgs commented Jul 19, 2016

Thanks @vstoykov. I'm getting hundreds of server error emails on this per day, so hopefully you (or someone else) can reap this sweet bounty :)

Speaking of which, @matthewwithanm, any chance you'll be setting up a flattr or gratipay or anything like that for imagekit? I have a moderately-sized web site with millions of images managed under imagekit and would love to drop a few bucks to help support development.

@vstoykov
Copy link
Collaborator

@mgalgs can you try the changes in #384 to test if the problem is solved?

vstoykov added a commit that referenced this issue Aug 1, 2016
Fixed #350: Error when trying to access width/height after url
vstoykov added a commit that referenced this issue Feb 22, 2017
* release/4.0:
  Bump version to 4.0
  Universal wheels!
  Replaces #301 autodiscover works with AppConfig
  Ignore autogenerated CTags file
  Do not try south modelinspector when not needed
  Make it possible to configure IMAGEKIT_CACHE_TIMEOUT
  Test against Django 1.11
  Close the file only if it has been opened locally
  Cleanup caching configuration
  updated readme.rst with a svg badge
  honor post_save's update_fields and only fire the source_saved signal when needed
  Ignore VSCode workspace config files
  Fixed #350: Error when trying to access width/height after url
  Fixes #382: Tests no longer leave junk
  Fixes #379 Support for Django 1.10
  Ignore .idea from git
  Include the test suite in the sourcetarball but do not install it.
  Make travis happy
  Drop support for older Django and Python versions
  Replace Lenna image in tests with a truly free alternative.
  Move compat module outside of templatetags package
  Fix test requirements for django 1.9 and Python3.5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants