-
-
Notifications
You must be signed in to change notification settings - Fork 78
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
Remove S3 upload concurrency #2120
Conversation
@benoit74 Thank you very much for the PR. I will let @audiodude full judge this PR. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2120 +/- ##
==========================================
- Coverage 75.26% 75.25% -0.01%
==========================================
Files 41 41
Lines 3198 3197 -1
Branches 706 706
==========================================
- Hits 2407 2406 -1
Misses 674 674
Partials 117 117 ☔ View full report in Codecov by Sentry. |
@audiodude Can you please review this PR? |
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.
LGTM
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.
LGTM
Fix #2118
Changes:
Nota: we might want to open an issue to support async S3 upload with proper queue management on the medium/long term. I'm not convinced at all this is worth to implement, most (all?) Python scrapers work pretty well without it and I do not expect much benefit (i.e. price to pay is too high)