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

S3Boto3Storage: fix for file-like objects without name, multipart upload support #195

Closed
wants to merge 3 commits into from

Conversation

pacahon
Copy link

@pacahon pacahon commented Aug 30, 2016

In fact, it's partially solves problem. If we pass subclass of File class and forgot override __bool__ method - an error based on empty name still can occur.

@linuxlewis
Copy link
Contributor

👍 on seeing the SHA error. We've worked around it by ensuring we pass a django.core.files.base.File object with name defined.

@pacahon
Copy link
Author

pacahon commented Aug 31, 2016

@linuxlewis Hi, can you show me where it's already fixed?
UPD. Did you talk about django-storages internals btw?

@linuxlewis
Copy link
Contributor

@pacahon The "fix" is just a workaround in our code. We do something like this...

from cStringIO import StringIO

from django.core.files.storage import default_storage
from django.core.files.base import File

default_storage.save('filename.txt', File(StringIO('content'), name='filename.txt'))

@pacahon
Copy link
Author

pacahon commented Sep 1, 2016

@linuxlewis Thanks, that was my first thought. But Storage class accepts any file-object, so I'm looking for solution. Of course, my PR in its current state is just another workaround.

upload_fileobj method supports multipart upload while put method limits to ~5gb

Avoid django-specific logic on saving python file-like objects without name
@codecov-io
Copy link

codecov-io commented Sep 6, 2016

Codecov Report

Merging #195 into master will increase coverage by 0.53%.

@@            Coverage Diff             @@
##           master     #195      +/-   ##
==========================================
+ Coverage   59.32%   59.85%   +0.53%     
==========================================
  Files          17       17              
  Lines        1694     1684      -10     
==========================================
+ Hits         1005     1008       +3     
+ Misses        689      676      -13
Impacted Files Coverage Δ
storages/backends/s3boto3.py 82.24% <100%> (+0.51%)
storages/backends/hashpath.py 84.61% <ø> (-1.1%)
storages/backends/s3boto.py 86.09% <ø> (-0.05%)
storages/backends/overwrite.py 0% <ø> (ø)
storages/backends/mogile.py 0% <ø> (ø)
storages/backends/image.py 0% <ø> (ø)
storages/backends/couchdb.py 0% <ø> (ø)
storages/backends/symlinkorcopy.py 0% <ø> (ø)
storages/backends/database.py 0% <ø> (ø)
... and 1 more

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 72cd5d6...7f230f5. Read the comment docs.

@pacahon
Copy link
Author

pacahon commented Sep 6, 2016

Should check if already was documented that upload_fileobj waits for bytes or seekable file object in binary mode (the same as old put method)

@pacahon pacahon changed the title S3Boto3Storage: fix for file-like objects without name S3Boto3Storage: fix for file-like objects without name, multipart upload support Sep 14, 2016
@jleclanche
Copy link
Contributor

PR needs rebasing but looks good to merge. @jschneier

@jschneier
Copy link
Owner

Updated and will be applied in #368. Thanks for the report and patch. Apologies for the delay.

nitely pushed a commit to satellogic/django-storages that referenced this pull request Jul 30, 2018
nitely pushed a commit to satellogic/django-storages that referenced this pull request Jul 30, 2018
nitely pushed a commit to satellogic/django-storages that referenced this pull request Jul 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants