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.

Sign up for GitHub

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

Jump to bottom

Update "seek of closed file" fix for current django-storages #6

Merged
merged 1 commit into from Jul 17, 2020
Merged

Update "seek of closed file" fix for current django-storages #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion CHANGES
View file
Open in desktop
@@ -1,6 +1,9 @@
0.2.1 (2019-06-20)
0.2.2
==================
* Update the seek of closed file fix to work with django-storages 1.9.x.

0.2.1 (2019-06-20)
==================
* Add boto3 file seek fix

0.2.0 (2019-06-19)
Expand Down
33 changes: 18 additions & 15 deletions cacheds3storage/__init__.py
View file
Open in desktop
Expand Up @@ -11,28 +11,31 @@ def __init__(self, *args, **kwargs):
self.local_storage = get_storage_class(
'compressor.storage.CompressorFileStorage')()

# Fix for "ValueError: seek of closed file" problem with boto3
# https://github.com/jschneier/django-storages/issues/382#issuecomment-377174808
def _save_content(self, obj, content, parameters):
# https://github.com/jschneier/django-storages/issues/382
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is removed completely, will it just pick up the one in django-storages?

Copy link
Member Author

@nikolas nikolas Jul 17, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmmm, good point.. maybe the fix is already in I may be getting confused. Let me see if I can reproduce this locally with ./manage.py compress to test that.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, so when testing this locally (actually with ./manage.py collectstatic):

When I remove the _save/_save_content override completely, I get the same error:

  File "/home/nik/src/d/mediathread/ve/lib64/python3.7/site-packages/django/core/files/base.py", line 55, in chunks
    self.seek(0)
ValueError: seek of closed file

With this change.... well it's still running and not showing any output yet. I'll see what happens, but looks promising.

Copy link
Member Author

@nikolas nikolas Jul 17, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, yes. It's actually copying the files successfully with this override (when you do ./manage.py collectstatic -v 3)... which unfortunately still seems necessary.

# #issuecomment-592876060
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
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()
# 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())
# 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(CachedS3BotoStorage, 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()
# Upload the object which will auto close the
# content_autoclose instance
return super(CachedS3BotoStorage, self)._save(
name, content_autoclose)

def save(self, name, content):
name = super(CachedS3BotoStorage, self).save(name, content)
Expand Down