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

how to upload file #68

Closed
DogeWatch opened this issue Mar 24, 2020 · 22 comments
Closed

how to upload file #68

DogeWatch opened this issue Mar 24, 2020 · 22 comments
Assignees
Labels
type: feature A new feature
Milestone

Comments

@DogeWatch
Copy link

Is there any example to show how to upload a file to the "Input!"?

@leahein
Copy link

leahein commented Apr 27, 2020

I would love to know whether gql supports this, as well.

@leahein
Copy link

leahein commented Apr 28, 2020

It doesn't look like this library currently supports files. Is there any plan to support the graphql multipart request spec in the future?

Happy to work on a PR for this as well.

@KingDarBoja
Copy link
Contributor

@leahein the library doesn't support file uploads but there is a repo which implements that for Django and Flask servers, would be worth checking it: graphene-file-upload.

@leahein
Copy link

leahein commented May 4, 2020

@leahein the library doesn't support file uploads but there is a repo which implements that for Django and Flask servers, would be worth checking it: graphene-file-upload.

I was looking for file upload support for a python client that uses graphene-file-upload server-side.
I found this one that seems to support file uploads well.

@KingDarBoja
Copy link
Contributor

@leahein best idea would be a separated repo to support this feature as this would be a third party integration with gql library, hence making easier to maintain the core features as there aren't many contributors around here 🤔

@leszekhanusz
Copy link
Collaborator

Now that the aiohttp transport is implemented in #70 It should be relatively easy to incorporate the implementation from aiogqlc

@leszekhanusz
Copy link
Collaborator

leszekhanusz commented May 15, 2020

@leahein

Happy to work on a PR for this as well.

I think this would be a great addition to this library. I will accept your pull request in my own PR #70 if it comes with 100% test code coverage (and documentation).

Note: to create temporary files in the tests, you can use this method

You can create a test which will create a temporary text file, send this file in a mutation to the local created aiohttp server, the server will return the first line from the file.

Then another test with a mutation with a list of files which return a list of first lines.

@Cito Cito added this to the Version 3 milestone May 15, 2020
@Cito Cito added the type: feature A new feature label May 15, 2020
@leszekhanusz
Copy link
Collaborator

@leahein

Now that #70 has been merged, if you are still interested you should create your PR directly from the repo now.

@leahein
Copy link

leahein commented May 21, 2020

@leszekhanusz Thanks for the details! I will work on adding file support 👍

@leszekhanusz
Copy link
Collaborator

@leahein did you make some progress on this ? I'll have some time in the following weeks so I will maybe start to implement this myself.

@leahein
Copy link

leahein commented Jul 12, 2020

I did start on the feature here, but it's not complete.
Right now there are utils to check whether a request contains files, and I started the transformation for sending them with the requests lib.

@KingDarBoja
Copy link
Contributor

KingDarBoja commented Jul 12, 2020

@leahein The link should be https://github.com/leahein/gql/tree/file-support :)

At the end, the base implementation to implement should be https://github.com/jaydenseric/graphql-multipart-request-spec

@sreerajkksd
Copy link

@leszekhanusz would you be able to implement this feature in a month or two ? I'm really looking forward to have this feature in this library.

@leszekhanusz
Copy link
Collaborator

This is now available in release v3.0.0a3. Tests and feedback is welcome.

@maaft
Copy link

maaft commented Sep 20, 2021

Is file-upload support planned for sync transport? I cannot use async io in my application.

@leszekhanusz
Copy link
Collaborator

@maaft
No, it is not planned. But you can use AIOHTTPTransport in a sync manner if you want.

@maaft
Copy link

maaft commented Sep 20, 2021

The issue with AIOHTTPTransport is that it creates a new session everytime you execute a query/mutation. That sounds very inefficient and also I get random "Transport is already connected" Exceptions from time to time. Might be some race conditions maybe.

@leszekhanusz
Copy link
Collaborator

The issue with AIOHTTPTransport is that it creates a new session every time you execute a query/mutation. That sounds very inefficient

Not necessarily, you can execute multiple queries and mutations on the same session (See async usage and async advanced usage). If you want to do it synchronously, it is still possible by running loop.run_until_complete yourself like in Client::execute

and also I get random "Transport is already connected" Exceptions from time to time. Might be some race conditions maybe.

I suspect you try to execute queries from different threads or task with the same transport. If that is not the case, and if you can reproduce a minimal example, please submit an issue.

@maaft
Copy link

maaft commented Sep 20, 2021

I suspect you try to execute queries from different threads or task with the same transport. If that is not the case, and if you can reproduce a minimal example, please submit an issue.

Yes, that is indeed the case. I'm also thinking about rewriting all my code to make use of asyncio (instead of threads & processes) but unfortunately I rely on external libraries (pytorch in this case) that make heavy use of threads and processes already. And there is no way around using pytorch sadly. It's the defacto standard in machine learning. But who knows, maybe it will work just fine with asyncio. I'll have to try.

Thanks!

@leszekhanusz
Copy link
Collaborator

See also #179

And to be clear, it should be possible to support file uploads with the RequestsHTTPTransport. I'll accept a PR if it is well tested.

@leszekhanusz
Copy link
Collaborator

@maaft Could you please check if the PR #244 works well for you?

@maaft
Copy link

maaft commented Sep 29, 2021

@leszekhanusz yes, it works. Thank you!

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

No branches or pull requests

7 participants