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

fix problem with empty media tarball name and boto3 s3 storage #201

Closed
wants to merge 1 commit into from
Closed

Conversation

pacahon
Copy link
Contributor

@pacahon pacahon commented Aug 29, 2016

I've switched to new boto3 s3 storage implementation from django-storages and realized, that it doesn't work due to empty tarball name. This PR should fix it. Maybe it's relevant to dbbackup cmd also.

@ZuluPro
Copy link
Contributor

ZuluPro commented Aug 29, 2016

Hello @pacahon,
Thank you very much for your work.
Maybe the comments are useless, did you test with other storages ?

@coveralls
Copy link

coveralls commented Aug 29, 2016

Coverage Status

Coverage increased (+0.005%) to 96.234% when pulling 70c00db on pacahon:master into 9bab10a on django-dbbackup:master.

@pacahon
Copy link
Contributor Author

pacahon commented Aug 30, 2016

@ZuluPro Now I'm not sure it's the right place to fix unpredictable behavior.
More it looks like hack, because django's File.name attribute is The name of the file including the relative path from MEDIA_ROOT.

Django storage save method accepts any file-like object https://code.djangoproject.com/ticket/27145, so all should be ok from our side. But bug is still here. Maybe it can be fixed in Django later https://code.djangoproject.com/ticket/27150#ticket but should investigate the problem deeply first.

I can try to send patch to django-storages first, but really not sure how fast they can reply.

@ZuluPro ZuluPro self-assigned this Aug 30, 2016
@ZuluPro
Copy link
Contributor

ZuluPro commented Aug 30, 2016

@pacahon: Try to send a patch, I am also maintener of Django-Storage...

@pacahon
Copy link
Contributor Author

pacahon commented Aug 30, 2016

@ZuluPro Ok, I'll try! :) Saw you in contributors, but didn't realize you can merge PR too.
Mb any thoughts on jschneier/django-storages#194 ?

@ZuluPro
Copy link
Contributor

ZuluPro commented Aug 19, 2017

Hello @pacahon
Is this issue is still relevant ?

@ZuluPro
Copy link
Contributor

ZuluPro commented Dec 16, 2017

@pacahon Any news ?

@pacahon
Copy link
Contributor Author

pacahon commented Dec 17, 2017

Sorry, but I can't remember in what case I had this problem. Since this PR wasn't merged, but I'm still using dbbackup, I definitely did something with this issue (maybe only on my side)

@ZuluPro ZuluPro closed this Dec 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants