-
Notifications
You must be signed in to change notification settings - Fork 583
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: django-storage 1.14 complains about files being opened twice when copying #1418
fix: django-storage 1.14 complains about files being opened twice when copying #1418
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #1418 +/- ##
=======================================
Coverage 75.21% 75.22%
=======================================
Files 75 75
Lines 3454 3455 +1
Branches 555 555
=======================================
+ Hits 2598 2599 +1
Misses 691 691
Partials 165 165
☔ View full report in Codecov by Sentry. |
@fsbraun Is it possible to have a test with this patch? |
@vinitkumar I know! I've been thinking about it, but I do not see a way of testing this without the specific implementation of Any idea how to test this? |
@fsbraun I don't know. Seems hard to implement in the test. Have you tried testing this patch with a real s3 backend? |
@vinitkumar I tested it on my staging setup with AWS: Without the fix, copying files fails and a copy of the first file remains in the source folder. After fixing it, copying works fine. The fix also does not affect local file storage (which just runs |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Let's get this merged and released 🚢 🚀
Description
While the root cause of the bug #1417 might be in django-storages 1.14, this quick-fix ensures that any ValueError raised opening a file for copying does not crash the file application with a server error.
Related resources
Checklist
master
Slack to find a “pr review buddy” who is going to review my pull request.