jschneier / django-storages

https://django-storages.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
2.75k stars 861 forks source link

Cannot read a file (from s3) twice (ValueError: The file cannot be reopened). #1319

Closed todorvelichkov closed 1 year ago

todorvelichkov commented 1 year ago

I know there has been some issue with the file.closed attribute which has been worked on in 1.14.0 and 1.14.1. But it looks like there is another issue, where a file cannot be re-read after close.

I've made a simple function to illustrate the behavior differences between 1.13.2, 1.14.0 and 1.14.1 (Django==4.2.5).

some_model = SomeModel.objects.last()
myfile = some_mode.some_file

def debug_file(myfile):
    print(f'pre_open is_closed={myfile.closed}')
    with myfile.open(mode="rb") as f:
        print(f'at_open is_closed={myfile.closed}')
        content = f.read()
    print(f'post_open is_closed={myfile.closed}')

1.13.2 - Notice that I can run the function twice.

>>> debug_file(myfile)
pre_open is_closed=True
at_open is_closed=False
post_open is_closed=False
>>> debug_file(myfile)
pre_open is_closed=False
at_open is_closed=False
post_open is_closed=False

1.14.0 - Fails on the second attempt.

>>> debug_file(myfile)
pre_open is_closed=True
at_open is_closed=True
post_open is_closed=True
>>> debug_file(myfile)
pre_open is_closed=True
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "<console>", line 3, in debug_file
  File "~/.pyenv/versions/myvenv/lib/python3.11/site-packages/django/db/models/fields/files.py", line 81, in open
    self.file.open(mode)
  File "~/.pyenv/versions/myvenv/lib/python3.11/site-packages/django/core/files/base.py", line 114, in open
    raise ValueError("The file cannot be reopened.")
ValueError: The file cannot be reopened.

1.14.1 - Again, Fails on the second attempt.

>>> debug_file(myfile)
pre_open is_closed=True
at_open is_closed=False
post_open is_closed=True
>>> debug_file(myfile)
pre_open is_closed=True
Traceback (most recent call last):
  File "<console>", line 1, in <module>
  File "<console>", line 3, in debug_file
  File "~/.pyenv/versions/myvenv/lib/python3.11/site-packages/django/db/models/fields/files.py", line 81, in open
    self.file.open(mode)
  File "~/.pyenv/versions/myvenv/lib/python3.11/site-packages/django/core/files/base.py", line 114, in open
    raise ValueError("The file cannot be reopened.")
ValueError: The file cannot be reopened.
Alexerson commented 1 year ago

I’m seeing the same thing here with django-imagekit. Trying to downgrad to fix the issue. I’ll report back when I managed to do it. If noone else can, I’ll try to look at a workaround on Monday.

jschneier commented 1 year ago

Okay so the behavior in 1.13.2 is obviously wrong; after the closing of a contextmanager we should be reading closed.

The bug here relates to the fact that the base Django File.open assumes the filesystem API.

So the proper fix is something along the lines of #1227.

I wonder if there is a good way to see this fixed upstream (e.g adding a StorageFile class) but for now I suppose we should fix it since it seems that people were relying on the previous behavior.

todorvelichkov commented 1 year ago

Thank you @jschneier