dennisv / django-storage-swift

OpenStack Swift storage backend for Django
MIT License
86 stars 59 forks source link

Adds SWIFT_CONTENT_LENGTH_FROM_FD #100

Closed pomegranited closed 6 years ago

pomegranited commented 6 years ago

Adds SWIFT_CONTENT_LENGTH_FROM_FD with default True, which preserves the current behaviour by inferring the content_length header from the Django File object's size attribute when saving the object.

Using SWIFT_CONTENT_LENGTH_FROM_FD: False will send content_length = None, allowing the SWIFT backend to determine the content length using the content read.

This is a workaround for a bug we encountered with Django ContentFile objects (prior to Django2) which can mis-report their size if changed after initialisation. See added test_no_content_length_from_fd() for example.

Reviewers

codecov-io commented 6 years ago

Codecov Report

Merging #100 into master will increase coverage by 0.02%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #100      +/-   ##
==========================================
+ Coverage   97.89%   97.92%   +0.02%     
==========================================
  Files           1        1              
  Lines         238      241       +3     
==========================================
+ Hits          233      236       +3     
  Misses          5        5
Impacted Files Coverage Δ
swift/storage.py 97.92% <100%> (+0.02%) :arrow_up:

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 27d34cf...4d560fb. Read the comment docs.

clemente commented 6 years ago

Another option would be to track who is doing .write() and make that method update the .size attribute too, to keep it correct. Then we could always send it. But it would involve changing every possible user. Because sending the content length isn't required by SWIFT, better not to send it.

With that documentation, :+1:

pomegranited commented 6 years ago

Hi @dennisv @einarf -- would you be willing to review this PR? Thank you!

pomegranited commented 6 years ago

Thank you @dennisv ! Could we also get 1.2.19 pushed to pypi?