Skip to content

Add RFC for incremental delivery #124

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 1 commit into from
Sep 3, 2020

Conversation

robrichard
Copy link
Contributor

No description provided.


-----
```
* The boundary used is `-` and is passed to the client in the http response's content-type header.
Copy link
Contributor

Choose a reason for hiding this comment

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

I co not see this in the example above.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The content-type: multipart/mixed; boundary="-" header should be in the HTTP response headers. The example is the HTTP response body. Each part of the response body contains it's own set of headers, but the header defining the boundary needs to be on the HTTP response, not in the body.

Copy link
Contributor

Choose a reason for hiding this comment

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

it's worth mentioning

@jaydenseric
Copy link
Contributor

Pretty interesting; I'll have to think about how this fits in with the GraphQL multipart request spec 🤔

@robrichard have you considered how they could be unified? E.g. someone using @defer and @stream directives alongside file uploads.

@robrichard
Copy link
Contributor Author

robrichard commented Jul 14, 2020

@jaydenseric I don't think there will be any conflict with the multipart request spec, since that spec only defines the request format, and this spec only describes the response format. Should be able to use @defer and @stream directives alongside file uploads without any issues.

Copy link
Member

@IvanGoncharov IvanGoncharov left a comment

Choose a reason for hiding this comment

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

I think it ready to be merged.
On last GraphQL WG we agreed to add support @stream/@defer across the stack.


## `content-type: multipart/mixed`

The HTTP response for an incrementally delivered response should contain the `content-type: multipart/mixed; boundary="-"` response header and conform to the [specification of multipart content defined by the W3 in rfc1341](https://www.w3.org/Protocols/rfc1341/7_2_Multipart.html). An example response body will look like:
Copy link
Member

Choose a reason for hiding this comment

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

Must the boundary be "-" specifically? Could other boundaries be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should support arbitrary boundaries. fetch-multipart-graphql currently only supports "-" but I will update that and this document to allow any boundary.

Copy link
Member

Choose a reason for hiding this comment

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

@benjie @robrichard Sorry, I didn't notice this comment before merging.
On the last "GraphQL over HTTP" WG we decided on merging and just forgot to actually do it.
Hope it didn't disrupt your workflow and @benjie suggestion can be addressed as separate PR.

Copy link
Member

Choose a reason for hiding this comment

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

I've filed an issue here: #135 (@IvanGoncharov if you could assign it to @robrichard that would be helpful).

@acao
Copy link
Member

acao commented Jan 22, 2021

@jaydenseric to clarify, i think what @robrichard is saying is that non-file fields can use @defer and @stream with multipart alongside your multipart file spec. in slack today @robrichard confirmed internally they have a client that is successfully able to do as such.

@jaydenseric speaking of which, would love to help advance file spec, and maybe we can revisit how @defer and @stream impact your spec if at all, and revise them if/as needed? I'm going to open an issue in your spec repo to discuss this further!

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.

6 participants