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 streaming HttpBody proto? #5540

Closed
npuichigo opened this issue Jan 8, 2019 · 11 comments · Fixed by #10673
Closed

Support streaming HttpBody proto? #5540

npuichigo opened this issue Jan 8, 2019 · 11 comments · Fixed by #10673
Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help!

Comments

@npuichigo
Copy link

The support for handling google.api.HttpBody is cool, especially when I want to directly reply http response with audio content_type. I notice that streaming case is remained to be done, so may I ask you when will that be supported? It's really use for me in implementing speech synthesis because l can transfer large audio data to client by clipping them into audio clips with chunked audio/mp3 content_type.

@mattklein123 mattklein123 added the question Questions that are neither investigations, bugs, nor enhancements label Jan 8, 2019
@mattklein123
Copy link
Member

@lizan

@lizan lizan added enhancement Feature requests. Not bugs or questions. help wanted Needs help! and removed question Questions that are neither investigations, bugs, nor enhancements labels Jan 8, 2019
@lizan
Copy link
Member

lizan commented Jan 8, 2019

@npuichigo we don't have specific timeline to implement that. That is useful so if you're interested feel free to open a PR and I can help you with that.

@npuichigo
Copy link
Author

@dio

@npuichigo
Copy link
Author

I tried to add the logic of httpbody streaming in grpc-gateway repo, and maybe I can try to implement that in envoy. (https://github.com/grpc-ecosystem/grpc-gateway/pull/1088/files) Can you give me some advice?

@belyalov
Copy link
Contributor

belyalov commented Apr 2, 2020

Is this issue about this to-be-done part?

// TODO(dio): Add support for streaming case.
if (has_http_body_response_) {
buildResponseFromHttpBodyOutput(*response_headers_, data);
return Http::FilterDataStatus::StopIterationAndBuffer;
}

@belyalov
Copy link
Contributor

belyalov commented Apr 3, 2020

@lizan - ping,
if this is correct - please assign this to me - I'll be working on it.

@qiwzhang
Copy link
Contributor

qiwzhang commented Apr 3, 2020

Yes, I think it is referring to support server streaming of HttpBody.

But HttpBody has content-type, will we allow different content-type from different HttpBody? I don't think different chunk can have different content-type.

@belyalov
Copy link
Contributor

belyalov commented Apr 3, 2020

Yep, I think it is suppose to be either:

  • the same for all gRPC stream response messages
  • first message has content-type set, the rest is unset

As for workaround for "misbehaves" upstream - seems we can just ignore content-type for messages except first?

@lizan
Copy link
Member

lizan commented Apr 3, 2020

@belyalov yes the part of code you're referring is right.

As for workaround for "misbehaves" upstream - seems we can just ignore content-type for messages except first?

I think this is fine, as long as we clearly document it.

@npuichigo
Copy link
Author

@belyalov Many thanks if you can help to fix this.😀

lizan pushed a commit that referenced this issue Apr 20, 2020
…ming (#10673)

Description: Add support for `httpBody` for response streaming use case.
Risk Level: Low
Testing: New unit / integration test
Docs Changes: Added
Release Notes: Added
Fixes #5540 

Signed-off-by: Konstantin Belyalov <arsenicus@mail.ru>
@npuichigo
Copy link
Author

@lizan by the way, is this kind of transcoder can be directly used in ambassador or istio?

penguingao pushed a commit to penguingao/envoy that referenced this issue Apr 22, 2020
…ming (envoyproxy#10673)

Description: Add support for `httpBody` for response streaming use case.
Risk Level: Low
Testing: New unit / integration test
Docs Changes: Added
Release Notes: Added
Fixes envoyproxy#5540

Signed-off-by: Konstantin Belyalov <arsenicus@mail.ru>
Signed-off-by: pengg <pengg@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests. Not bugs or questions. help wanted Needs help!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants