Skip to content

Feat: Add Streaming functionalities to file uploads #174

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

Merged
merged 9 commits into from
Dec 11, 2020

Conversation

anisov
Copy link
Contributor

@anisov anisov commented Nov 26, 2020

When receiving a large file from a request(use aiohttp) and then transferring it through Graphql, it was necessary to read the data from the request and we can load it into RAM or save it to disk (to avoid memory leaks), and only after that it was possible to transfer it through graphql(use io.BytesIO or open()). In this branch, I tried to solved this problem.

@leszekhanusz
Copy link
Collaborator

Thanks for your PR, that's definitely something I wanted to add.

Could you please:

  • fix it so that the tests are passing if we do not use the aiohttp optional depency
  • add docs on how to use it (new section in docs/usage/file_upload.rst). You can test the docs locally by running make docs

@codecov-io
Copy link

codecov-io commented Nov 26, 2020

Codecov Report

Merging #174 (e5a9508) into master (d3f79e7) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #174   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           14        14           
  Lines          946       949    +3     
=========================================
+ Hits           946       949    +3     
Impacted Files Coverage Δ
gql/transport/aiohttp.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3f79e7...e5a9508. Read the comment docs.

@anisov
Copy link
Contributor Author

anisov commented Nov 26, 2020

I have embedded your suggestions into the code.

@leszekhanusz
Copy link
Collaborator

Looks good!

What I would also like to integrate in this PR is the possibility to send large local files using an asynchronous generator as described here.

Then we would need some test for those two functionalities.

@anisov
Copy link
Contributor Author

anisov commented Nov 27, 2020

Looks good!

What I would also like to integrate in this PR is the possibility to send large local files using an asynchronous generator as described here.

Then we would need some test for those two functionalities.

Thanks, I'll do it on the weekend

@leszekhanusz
Copy link
Collaborator

Very nice!

I just made some small modifications to the docs but cannot push it here.
Could you please check the box:
"Allow edits and access to secrets by maintainers"
Thanks

@leszekhanusz leszekhanusz added the type: feature A new feature label Nov 29, 2020
@anisov
Copy link
Contributor Author

anisov commented Nov 29, 2020

Very nice!

I just made some small modifications to the docs but cannot push it here.
Could you please check the box:
"Allow edits and access to secrets by maintainers"
Thanks

Thanks! The checkbox in the "Allow maintainers to edit" field was already there, I rearranged it again. Or need to do something else?

@leszekhanusz
Copy link
Collaborator

Thanks! The checkbox in the "Allow maintainers to edit" field was already there, I rearranged it again. Or need to do something else?

It seems to work now... I've added my modifications.

@leszekhanusz leszekhanusz changed the title Feat: Add processing aiohttp StreamReader for proxying data from a request. Feat: Add Streaming functionalities to file uploads Nov 29, 2020
@leszekhanusz leszekhanusz merged commit c17959c into graphql-python:master Dec 11, 2020
@snowPu
Copy link

snowPu commented Jan 20, 2021

When I try this

result = await client.execute(query, variable_values=params, upload_files=True)

I get an error: Object of type async_generator is not JSON serializable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature A new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants