single input multiple files enabled

[tegkhanna]

Added support for multiple files through a single input!

Fixes #777

[tegkhanna]

Requesting review @florimondmanca

[demonno]

I am having same issue, and saw there is some progress already, based on discussion here #777 problem is already clear and implementation suggestion is also made.

Expecting to get similar API to requests post-multiple-multipart-encoded-files

>>> url = 'https://httpbin.org/post'
>>> multiple_files = [
...     ('images', ('foo.png', open('foo.png', 'rb'), 'image/png')),
...     ('images', ('bar.png', open('bar.png', 'rb'), 'image/png'))]
>>> r = httpx.post(url, files=multiple_files)
>>> r.text

@tegkhanna your suggestion in this Pull Request is different from above example.

Why don't not you follow this suggestion, did it have any issues?

[florimondmanca]

Hi @tegkhanna! Thanks for submitting this PR. Overall the code looks great, but not sure it's what we want in terms of API.

As @demonno mentioned, and as suggested by Tom in the issue thread, we should probably go for "list of (fieldname, value) items" (instead of "dict of {fieldname: }" as currently proposed here), because it is is line with what Requests does.

So the line that needs changing is probably for name, value in files.items(): files would now refer to either a dict, or a list of (name, value) items. So probably this…

fileitems = files.items() if isinstance(files, dict) else files
for name, value in fileitems:
    ...

[florimondmanca]

Went ahead and moved the Fixes #777 note to the PR description, that way #777 is automatically linked. :-)

[tegkhanna]

@florimondmanca Hi! I will commit the changes soon :) Actually, I was confused that should I add the list of tuples support as it was different from the core API. Should I add the support for data = list of (fieldname, value) item as well?

Went ahead and moved the Fixes #777 note to the PR description, that way #777 is automatically linked. :-)

Thanks for that. I thought it gets linked by adding to the Fixes keyword to the title!

[florimondmanca]

@tegkhanna

Should I add the support for data = list of (fieldname, value) item as well?

I don't think so no, at least not here.

Currently data only supports either string-like or a dict:

RequestData = typing.Union[
    dict, str, bytes, typing.Iterator[bytes], typing.AsyncIterator[bytes]
]

Not obvious yet that we need anything else, but if we do then in any case it should be a different issue/PR (as this one is about files).

[tegkhanna]

@florimondmanca One last question, should I then drop the support for multiple files if they are given as part of a dict {fieldName : [list of file objects]} or should I keep it as well?

[florimondmanca]

Yes, let’s drop it in favor of the list of tuple style. :)

[florimondmanca]

Brilliant! One factoring suggestion.

Also, it occurs to me I was mistaken when I wrote #891 (comment): we probably do want to support the "list of (field, value) tuples" style for data too.

Because a/ that's supported by Requests, and b/ this is the URL-encoded data case, and it makes sense to have multiple query params with the same field name. Tom's comment in #777 (comment) was actually exactly about also supporting this style for data, alongside files…

This PR is almost ready so it's probably okay to submit the data part as a separate PR. If it turns out they'd overlap too much then let's put everything in here…

[tegkhanna]

@florimondmanca Sure I will make another PR for the data part! I have added the required change for filetype reusability to this PR :)

[florimondmanca]

LGTM!

[tegkhanna]

@florimondmanca Should I add the data list changes to this PR only? Because a separate PR will cause conflicts if the changes are based on the master

[florimondmanca]

Yes I suppose so... sure, you can add those changes here if that helps. :)

[tegkhanna]

The PR is complete. Files and data both now accepts a list of tuples along with a dict.

[florimondmanca]

Closing this one in favor of #1032, thank you so much for the time you've put into this, @tegkhanna!