django-storages
django-storages copied to clipboard
jschneier
s3: Fix newline handling for text-mode files
This fixes the newline issue at #1351. Files open in text mode (r) now get newline handling consistent with other python file-like objects.
run these specific tests with
tox -e py3.10-django4.2 -- tests/test_s3.py -k newlines
There were two possible ways of doing this:
- Change the initialization of
self._fileas aNamedTemporaryFileinstead of spooled, so we can reopen the file with the correct mode after the contents have been downloaded. - Wrap the underlying
self._file's binary object withTextIOWrapper
The changes in this PR implement (2).
self._force_mode has been removed, as this change will now correctly read the file with the right mode (Hence not needing to decode a binary when given "r"). The relevant test(s) has been moved down to the S3 test class, so we can correctly test the storage.save() and storage.open() and flow and thus test the S3File._get_file method correctly.
This change looks good but tests are failing. Is this due to 3.7 not being dropped yet? Once I release the next version the intent is to drop that.
yes, looks like moto dropped 3.7 support: https://github.com/getmoto/moto/commit/2fd5e800e4aab49a69856fa71bccf898d541a64a
I've fixed the tests by undoing my previous moto upgrade and instead pinning to <5 so the tests will run
are those gcloud failures expected? I also get them locally. Seems unrelated to the changes in this PR though
are those gcloud failures expected? I also get them locally. Seems unrelated to the changes in this PR though
Seems like a breaking change related to a google cloud dependency.
I'm moving your moto fix and that to a separate PR so I can unblock everything else. Thanks for flagging.
This actually also exposed a bug in Google Cloud Storage handling of gzip files. I was looking into who wrote the tests in such a confusing way and of course it was...me.