I/O operation on closed file #391
Comments
|
Finally some clue. It looks like smething changed in boto3 and the way how django imagekit is playing with files is not very usefull for boto3. Currently I'm very busy with a big project and does not have enough time to investigate the problem with boto. If you can investigate it and prepare a pull request I will be be grateful. |
|
I will definitely spend some time to investigate, but this feels like it goes beyond my capabilities. Nonetheless, will keep this ticket updated if I find anything. |
|
Some findings:
The following line closes the file:
|
|
Narrowed it down to Could it be that boto3 closes the file because it uses a future? The quickfix would be to simply re-open the file again, but that might be bad design? |
|
@philippeluickx After this fix in django-s3-storage etianen/django-s3-storage@0286570 this issue must be gone. Check with |
|
@vstoykov I am however using (the more popular / mature) https://github.com/jschneier/django-storages |
|
Now when I see in django-storages source code I see that the storage does not support reopening of the file. Also as stated in this issue jschneier/django-storages#64 there may be a problem with Is it possible to test with |
|
Here's my import logging
LOG = logging.getLogger(__name__)
def PATCH_IMAGEKIT_IMAGECACHEFILE_GENERATE():
"""
Only needed until this issue gets fixed.
<https://github.com/matthewwithanm/django-imagekit/issues/391>
"""
from imagekit.cachefiles import ImageCacheFile
from imagekit.utils import generate
from django.core.files import File
from imagekit.utils import get_logger
def PATCHED_GENERATE(self):
# Generate the file
content = generate(self.generator)
# PATCHED
def PATCHED_CLOSE():
"""Protect file from being closed"""
LOG.warning('patched close() called - ignoring', stack_info=False)
ORIG_CLOSE = content.close
content.close = PATCHED_CLOSE
# Here content.close() gets called which is what we don't want
actual_name = self.storage.save(self.name, content)
# restore
content.close = ORIG_CLOSE
# /PATCHED
# We're going to reuse the generated file, so we need to reset the pointer.
content.seek(0)
# Store the generated file. If we don't do this, the next time the
# "file" attribute is accessed, it will result in a call to the storage
# backend (in ``BaseIKFile._get_file``). Since we already have the
# contents of the file, what would the point of that be?
self.file = File(content)
if actual_name != self.name:
get_logger().warning(
'The storage backend %s did not save the file with the'
' requested name ("%s") and instead used "%s". This may be'
' because a file already existed with the requested name. If'
' so, you may have meant to call generate() instead of'
' generate(force=True), or there may be a race condition in the'
' file backend %s. The saved file will not be used.' % (
self.storage,
self.name, actual_name,
self.cachefile_backend
)
)
# Apply main patch
LOG.warning('PATCHING ImageCacheFile._generate from imagekit')
ImageCacheFile._generate = PATCHED_GENERATE
PATCH_IMAGEKIT_IMAGECACHEFILE_GENERATE()Works with: |
|
@jkbrzt can you test with latest version of More info here #335 |
|
@vstoykov unfortunately, this specific project is pretty tied to As for the |
|
Same error happens on saving inlineformset with ImageSpecField. |
|
@shapoglyk are you also using |
|
@vstoykov , no I am using default django file storage. |
|
That's interesting. |
|
As philippeluickx pointed out, this seems to be boto3 related as the upload_fileobj closes the file you pass to it. boto/boto3#929 I managed to get around this by passing a clone of the file object to boto3 that it can close and cleanup but keeping the original around & unclosed. class CustomS3Boto3Storage(S3Boto3Storage):
"""
This is our custom version of S3Boto3Storage that fixes a bug in boto3 where the passed in file is closed upon upload.
https://github.com/boto/boto3/issues/929
https://github.com/matthewwithanm/django-imagekit/issues/391
"""
def _save_content(self, obj, content, parameters):
"""
We create a clone of the content file as when this is passed to boto3 it wrongly closes
the file upon upload where as the storage backend expects it to still be open
"""
# Seek our content back to the start
content.seek(0, os.SEEK_SET)
# Create a temporary file that will write to disk after a specified size
content_autoclose = SpooledTemporaryFile()
# Write our original content into our copy that will be closed by boto3
content_autoclose.write(content.read())
# Upload the object which will auto close the content_autoclose instance
super(CustomS3Boto3Storage, self)._save_content(obj, content_autoclose, parameters)
# Cleanup if this is fixed upstream our duplicate should always close
if not content_autoclose.closed:
content_autoclose.close() |
|
@leonsmith I confirm that your custom storage class works. I'll be using it until a fix is made. Thanks. |
|
Hi guys. Can you check if the latest changes in |
|
@vstoykov I have the same problem with: django==1.10.5 |
|
@cedriccarrard thank you for reporting. Can you paste some traceback. |
|
@vstoykov After the submit of my form I use The problem comes from this |
|
I fixed my problem: IMAGEKIT_DEFAULT_CACHEFILE_STRATEGY = 'app.example.imagegenerators.FixJustInTime' You have a better solution ? |
|
@cedriccarrard did you try the workaround from #391 (comment) ? Your solution looks too hacky but if it works for you probably you can still use it. |
|
I'm experiencing the same issue. It occurs when I add an The workaround posted in #391 (comment) works for me. Thanks! |
|
Just another note - I'm having the same issue. |
|
@mgrdcm Is the workaround in #391 (comment) works for you? |
|
Encountered this problem on latest django/imagekit/storages, can confirm workaround #391 worked for me. |
|
#391 (comment) worked for me too. |
It was the good old "I/O operation on closed file" which I've had before when uploading images to S3 and wanting to get imagekit to make thumbnails of them. The solution, as last time was to create a custom class based on `S3Boto3Storage`: matthewwithanm/django-imagekit#391 (comment)
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Thank you @pasevin @vstoykov! class CustomS3Boto3Storage(S3Boto3Storage, ABC):
"""
This is our custom version of S3Boto3Storage that fixes a bug in
boto3 where the passed in file is closed upon upload.
From:
https://github.com/matthewwithanm/django-imagekit/issues/391#issuecomment-275367006
https://github.com/boto/boto3/issues/929
https://github.com/matthewwithanm/django-imagekit/issues/391
"""
def _save(self, name, content):
"""
We create a clone of the content file as when this is passed to
boto3 it wrongly closes the file upon upload where as the storage
backend expects it to still be open
"""
# Seek our content back to the start
content.seek(0, os.SEEK_SET)
# Create a temporary file that will write to disk after a specified
# size. This file will be automatically deleted when closed by
# boto3 or after exiting the `with` statement if the boto3 is fixed
with SpooledTemporaryFile() as content_autoclose:
# Write our original content into our copy that will be closed by boto3
content_autoclose.write(content.read())
# Upload the object which will auto close the
# content_autoclose instance
return super(CustomS3Boto3Storage, self)._save(name, content_autoclose) |
|
I assume the |
|
Note that although the two classes posted by @vstoykov and @mannpy look identical apart from the syntax highlighting, the one from @vstoykov is missing a |
|
@philgyford thank you for your clarification. I just rewrote the @pasevin variant without testing it in real life. @mannpy thank you for fixing the snippet to work correctly. I marked my comment as outdated to stay only your variant because people can copy mine by accident and will struggle with errors. |
|
Hey, just chiming in on this issue. While #391 (comment) works great, it doesn't work well for large files (e.g. > 1GB) unless your machine has a lot of RAM. In that case you'll want to specifically use this custom storage for ImageKit only via To avoid this issue completely, could you defer image generation? Is the root of this problem that the backend storage finishes up before ImageKit is done processing? |
|
@
Can you tell me how to revert back to boto?? Do I uninstall boto3 and install boto?? Is that all?? |
You choose. e.g. Make a file at import os
from storages.backends.s3boto3 import S3Boto3Storage
from tempfile import SpooledTemporaryFile
class CustomS3Boto3Storage(S3Boto3Storage):
"""
This is our custom version of S3Boto3Storage that fixes a bug in
boto3 where the passed in file is closed upon upload.
From:
https://github.com/matthewwithanm/django-imagekit/issues/391#issuecomment-275367006
https://github.com/boto/boto3/issues/929
https://github.com/matthewwithanm/django-imagekit/issues/391
"""
def _save(self, name, content):
"""
We create a clone of the content file as when this is passed to
boto3 it wrongly closes the file upon upload where as the storage
backend expects it to still be open
"""
# Seek our content back to the start
content.seek(0, os.SEEK_SET)
# Create a temporary file that will write to disk after a specified
# size. This file will be automatically deleted when closed by
# boto3 or after exiting the `with` statement if the boto3 is fixed
with SpooledTemporaryFile() as content_autoclose:
# Write our original content into our copy that will be closed by boto3
content_autoclose.write(content.read())
# Upload the object which will auto close the
# content_autoclose instance
return super(CustomS3Boto3Storage, self)._save(name, content_autoclose)Then in your with the path reflecting where you've put the |
|
@philgyford & @mannpy This works for me as well! Thanks. We use Digital Ocean Spaces (S3 compatible) for our media. It seems that the first call to the thumbnail is giving us the error (ie. the uploading), but next calls are fine. |
Thanks you very much for sharing this :). It works with: |
Did you resolve this? I have the same setup and it just broke after updating some versions. I used a custom storageclass like the one @philgyford suggested, which worked fine with versions: Currently it does not work for versions: |
|
@FinnGu, I'm currently using the same code as above successfully with: I don't have |
Well, this is embarrasing... I still used the old version that was proposed here and not the newer version and of course it is working now. For others that have overlooked this as well: |
|
Finally found the solution here. thank you, guys. |
|
I have the same issue with default FileStorage in development. I serve thumbnail image using I'm using the following solution to re-open the file if it was closed after generation: class ImagekitCacheStrategy(JustInTime):
def on_existence_required(self, file: ImageCacheFile):
super().on_existence_required(file)
if file.closed:
file.open() |
hey @mick88 I am curious where did you put this, i am facing the same issue with django-imagekit and the error pops right in that method, can you point me where i can put this to see if this works for me? |
|
@luiscastillocr you can override cache strategy in settings: https://django-imagekit.readthedocs.io/en/latest/configuration.html#django.conf.settings.IMAGEKIT_DEFAULT_CACHEFILE_STRATEGY |
|
any progress? |
|
I still have this issue, the fix with the custom storage class that we have above doesn't work for me on the latest versions of from storages.backends.s3boto3 import S3Boto3Storage, SpooledTemporaryFile
class CustomS3Boto3Storage(S3Boto3Storage):
def _save(self, name, content):
content.seek(0)
with SpooledTemporaryFile() as tmp:
tmp.write(content.read())
return super()._save(name, tmp) |
I have it like this working with @mick88 the trick you suggested did not work for me, ended up implementing the whole custom class as you can see, thanks anyway 👍 |
|
From the underlying issue's comment I can propose updated workaround which will not create temporary file. from io import BufferedReader
from storages.backends.s3boto3 import S3Boto3Storage
class NonCloseableBufferedReader(BufferedReader):
"""
Taken from https://github.com/boto/s3transfer/issues/80#issuecomment-482534256
"""
def close(self):
self.flush()
class CustomS3Boto3Storage(S3Boto3Storage):
"""
This is our custom version of S3Boto3Storage that fixes a bug in
boto3 where the passed in file is closed upon upload.
From:
https://github.com/matthewwithanm/django-imagekit/issues/391#issuecomment-275367006
https://github.com/boto/boto3/issues/929
https://github.com/matthewwithanm/django-imagekit/issues/391
"""
def _save(self, name, content):
"""
We create a clone of the content file as when this is passed to
boto3 it wrongly closes the file upon upload where as the storage
backend expects it to still be open
"""
# Seek our content back to the start
content.seek(0, os.SEEK_SET)
return super(CustomS3Boto3Storage, self)._save(name, NonCloseableBufferedReader(content))I did not test it, but just put it here for reference. |
and others
Getting this issue when creating a new image. I simply have a few specs on an ImageField like this:
Exception:
Later part of the stack trace, somehow the "File" gets set to none?:
I have a setup with S3 and this started happening when I updated the boto library to boto3.
Reverting back seemed to help. I can provide more info if needed.
The text was updated successfully, but these errors were encountered: