-
Notifications
You must be signed in to change notification settings - Fork 280
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
feat: Add support for uploading files as a list #403
base: master
Are you sure you want to change the base?
feat: Add support for uploading files as a list #403
Conversation
Uploading a list of files with the same key is supported by the Requests library. This allows the receiving server to accept a list of files for the same key, i.e. accepting any number of files. Issue: MarketSquare#401
This could also be a candidate for a back-port to the 0.9 version as well. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #403 +/- ##
==========================================
+ Coverage 85.07% 88.79% +3.72%
==========================================
Files 9 9
Lines 469 473 +4
Branches 117 49 -68
==========================================
+ Hits 399 420 +21
+ Misses 65 51 -14
+ Partials 5 2 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
I would also add a unit test to make sure files get closed for both cases list or tuple.
Thanks!
files[k].append(val) | ||
else: | ||
files[k] = val | ||
for k in request.files.keys(): |
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.
you also had to fix httpbin?
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.
Yes, as this only returned one item the way it was written. According to what I found out, you need to use the getlist(key) method to get all files for one key.
files_descriptor_to_close = filter( | ||
is_file_descriptor, list(files.values()) + [data] | ||
) | ||
|
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.
at this point I would extract the logic to close file descriptor in a dedicated function and try to cleanup this part
Uploading a list of files with the same key is supported by the Requests library. This allows the receiving server to accept a list of files for the same key, i.e. accepting any number of files.
Issue: #401