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

Add option to detect content encoding to fix gzip uploads. Resolves #451 #453

Closed
wants to merge 3 commits into from

Conversation

skruger
Copy link

@skruger skruger commented Jan 20, 2018

The behavior selected in the resolution of #263 has broken django storages when trying to upload normal gzip files without double compressing them or when AWS_IS_GZIPPED=False.

AWS_DETECT_CONTENT_ENCODING has been added to allow the behavior that was introduced last year to be used when desired. If AWS_DETECT_CONTENT_ENCODING=True and AWS_IS_GZIPPED=False gzip encoded files will be stored in their uncompressed form on S3. The detect content encoding default in the storage classes is False which reverses the behavior introduced in pull request #264. This default of False would appear to be the least confusing to someone in a new project trying to use django-storages to upload a .gz file.

This pull request modifies the existing unit test and adds one new unit test to verify the behavior of both AWS_DETECT_CONTENT_ENCODING states in both the boto and boto3 based storage classes. The new configuration option has also been added to the documentation.

@codecov-io
Copy link

codecov-io commented Jan 20, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@87c42d1). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #453   +/-   ##
=========================================
  Coverage          ?   76.08%           
=========================================
  Files             ?       11           
  Lines             ?     1568           
  Branches          ?        0           
=========================================
  Hits              ?     1193           
  Misses            ?      375           
  Partials          ?        0
Impacted Files Coverage Δ
storages/backends/s3boto.py 87.45% <100%> (ø)
storages/backends/s3boto3.py 86.58% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87c42d1...3871a6a. Read the comment docs.

@skruger
Copy link
Author

skruger commented Jan 20, 2018

The Django 1.8 on Python 3.3 unit test appears to be failing because it installed paramiko 2.4.0 which is officially incompatible with Python 3.3. According to the categories metadata on pypi.python.org paramiko 2.3.1 does support python 3.3 so forcing the lower version for python 3.3 installations might work to resolve this unit test failure.

@skruger
Copy link
Author

skruger commented Jan 20, 2018

All tests are passing except for the error resolved by pull request #415

@sww314
Copy link
Contributor

sww314 commented Jul 11, 2018

@skruger can you rebase this fix?

@sww314 sww314 added the s3boto label Jul 11, 2018
@skruger skruger force-pushed the s3_content_encoding branch from 43a5e19 to 726b2ca Compare July 12, 2018 05:42
@skruger
Copy link
Author

skruger commented Jul 12, 2018

@sww314 rebase complete. There were some rough conflicts.

@skruger skruger force-pushed the s3_content_encoding branch from c5c75ad to 3871a6a Compare August 16, 2018 23:00
@skruger
Copy link
Author

skruger commented Aug 16, 2018

@sww314 I saw that there were some new merge conflicts so I rebased again. Still ready for merge or further review.

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 this pull request may close these issues.

4 participants