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

Add support for multiple files per POST field #1032

Merged

Conversation

euri10
Copy link
Member

@euri10 euri10 commented Jun 24, 2020

fixes #777
inspired by great work done on #891
separates files and data as per @florimondmanca suggesiton in gitter
I commented below a mypy error I ignored because I'm clueless on how to deal with it to be honest

@@ -328,7 +328,8 @@ def _iter_fields(
else:
yield self.DataField(name=name, value=value)

for name, value in files.items():
file_items = files.items() if isinstance(files, dict) else files
for name, value in file_items: # type: ignore
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No idea how to fix the mypy error on this line httpx/_content_streams.py:332: error: Unpacking a string is disallowed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it’s because the type hints is using Mapping but we’re only matching against dict (a specific type of mapping), so mypy assumes that the other branch could also receive other types of mappings, and iterating over a mapping yields its keys (strings).

It’s actually possible to isinstance(obj, typing.Mapping) (since it’s the same Mapping class than in collections.abc), so I think we can go for that?

Alternatively, switch the if to match a tuple (and treat the default case as mapping) - probably a bit less surprising.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed !

Copy link
Member

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

tests/test_content_streams.py Show resolved Hide resolved
docs/advanced.md Outdated Show resolved Hide resolved
docs/advanced.md Outdated Show resolved Hide resolved
Copy link
Member

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @euri10 🎉

@florimondmanca florimondmanca changed the title multiple files upload same field Add support for multiple files per POST field Jun 24, 2020
@florimondmanca florimondmanca merged commit 4d28795 into encode:master Jun 24, 2020
@euri10 euri10 deleted the 771_multiple_files_upload_same_field branch June 24, 2020 17:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for multiple files per POST field
2 participants