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

support HttpBody in streaming responses #958

Closed
srenatus opened this issue Jun 20, 2019 · 11 comments
Closed

support HttpBody in streaming responses #958

srenatus opened this issue Jun 20, 2019 · 11 comments

Comments

@srenatus
Copy link
Contributor

srenatus commented Jun 20, 2019

In http.proto, it says

// This message can be used both in streaming and non-streaming API methods in
// the request as well as the response.

What I would like to achive is to have an rpc MyMethod(Req) (stream HttpBody) return lines of bare content, like, CSV, or JSON streaming.

What we currently get is:

# generate some data
$ curl http://127.0.0.1:8080/v1/example/a_bit_of_everything/bulk -d '{}'
{}
$ curl http://127.0.0.1:8080/v1/example/a_bit_of_everything/bulk -d '{}'
{}
$ curl http://127.0.0.1:8080/v1/example/a_bit_of_everything/bulk -d '{}'
{}
$ curl -v http://127.0.0.1:8080/v1/example/a_bit_of_everything/httpbody
*   Trying 127.0.0.1...
* TCP_NODELAY set
* Connected to 127.0.0.1 (127.0.0.1) port 8080 (#0)
> GET /v1/example/a_bit_of_everything/httpbody HTTP/1.1
> Host: 127.0.0.1:8080
> User-Agent: curl/7.62.0
> Accept: */*
>
< HTTP/1.1 200 OK
< Content-Type: application/json
< Grpc-Metadata-Content-Type: application/grpc
< Grpc-Metadata-Count: 3
< Date: Thu, 20 Jun 2019 11:09:10 GMT
< Transfer-Encoding: chunked
<
{"result":{"content_type":"application/csv-stream","data":"JSFkKHN0cmluZz05ZDg0MTA3MzljMzdhMWIwZGIxNTlkNzY3NzUxOTA1YWQ2M2JmNDBjNDRhMTc3MGMpLDA="}}
{"result":{"content_type":"application/csv-stream","data":"JSFkKHN0cmluZz05ZTg0MTA3MzljMzdhMWIwZGIxNTlkNzY3NzUxOTA1YWQ2M2JmNDBjNDRhMTc3MGMpLDE="}}
{"result":{"content_type":"application/csv-stream","data":"JSFkKHN0cmluZz05Zjg0MTA3MzljMzdhMWIwZGIxNTlkNzY3NzUxOTA1YWQ2M2JmNDBjNDRhMTc3MGMpLDI="}}
* Connection #0 to host 127.0.0.1 left intact

(This was created using that branch)

What would be nice was to get a response like

< Content-Type: application/csv-stream
< Grpc-Metadata-Content-Type: application/grpc
< Grpc-Metadata-Count: 3
UUID1,0
UUID2,1
UUID3,2

i.e., the lines of content, unencoded, and not wrapped in {"result": ... }

@johanbrandhorst
Copy link
Collaborator

@wimspaargaren

I think this should be possible with a type assertion in ForwardResponseStream like we're already doing for unary messages.

@stale
Copy link

stale bot commented Sep 9, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Sep 9, 2019
@stale stale bot closed this as completed Sep 16, 2019
@srenatus
Copy link
Contributor Author

Still seems like a valuable thing to support. That said, I have not immediate need for it and won't be implementing it. So I guess closing is fine with me 😃

@npuichigo
Copy link

@theRealWardo Sorry to bother you. Has this feature already been supported with streaming httpbody?

@johanbrandhorst
Copy link
Collaborator

I don't think this is supported.

@theRealWardo
Copy link
Contributor

so my fork does do this - theRealWardo/grpc-gateway@07b39ba...theRealWardo:master

disclaimer: its been a while since I tried to rebase that code on top of master so... your mileage may vary with all of this.

the custom marshaller I wrote HttpBodyMarshaler detects the HttpBody message and handles it appropriately. in a way this is "supported" because I think there's a way to have a custom marshaller in your code that can be set up using WithMarshalerOption in the runtime package (you probably want to override the MIMEWildcard mime type if you want this for all requests/responses).

@johanbrandhorst
Copy link
Collaborator

Ah cool. Maybe we could add an example to the docs?

@npuichigo
Copy link

@theRealWardo Do you still work on merging your code to master branch?

@npuichigo
Copy link

@johanbrandhorst When will an interface param be added to the ContentType() function of Marshal interface so as to fit HttpBody message better. (set the content type with proper grpc response)

@johanbrandhorst
Copy link
Collaborator

I think the existing httpbody marshaller supports this? https://github.com/grpc-ecosystem/grpc-gateway/blob/master/runtime/marshal_httpbodyproto.go#L29

@johanbrandhorst
Copy link
Collaborator

I've confirmed this works as expected

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants