-
Notifications
You must be signed in to change notification settings - Fork 72
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: support file upload in router #652
Conversation
a6bbca4
to
6eea35a
Compare
// XXX: This buffer needs to be returned to the pool only | ||
// AFTER we're done with body (retrieved from parser.ReadBody()) | ||
buf := pool.GetBytesBuffer() | ||
defer pool.PutBytesBuffer(buf) | ||
if r.Header.Get("Content-Type") == "application/json" { | ||
body, err = h.operationProcessor.ReadBody(buf, r.Body) |
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.
Have you thought about creating temp files in the system temp dir, stream all multipart files into the file system, read them as streams, and clean up the temp files after the request lifecycle.
I'm concerned that big files will occupy a lot of memory, so ideally, we can stream as much as possible and use the filesystem for temporary storage. How doy ou think about this?
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.
Hum ... this is the regular json request body reading. In fact, in the following lines I trigger the body read from the multipart form using the library you have suggested to me:
If I understood right, this library will handle the file using a stream. Seems very straight forward. The library claims there's no need to use files and have a low memory footprint. Assuming this is true, I wonder how I would pass a stream to the library which actually forward the request.
By the way I have a working naive version for graphql-go-tools
https://github.com/wundergraph/graphql-go-tools/pull/758/files
6e4c862
to
9ce96fd
Compare
13f9360
to
1bde352
Compare
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Motivation and Context
This PR makes possible to support a file upload according to spec:
https://github.com/jaydenseric/graphql-multipart-request-spec
This PR depends on this PR from
graphql-go-tools
:wundergraph/graphql-go-tools#758
This PR parse multiform part http requests with files, saves in the file system and passed the file information along the graphql tools library to be forwarded to a subgraph that supports files uploads. In order to make tests possible, I added file uploads operations to the demo employee subgraph and some enhanced methods to test infrastructure.
TODO