remove on create initial metadata#461
Conversation
Signed-off-by: Pengyuan Bian <bianpengyuan@google.com>
jplevyak
left a comment
There was a problem hiding this comment.
I was going to change the ABI to include initial metadata in the proxy_grpc_call and proxy_grpc_stream ABI calls and then handle the create call internally. This would be just an additional Pairs argument to the grpcCall SDK. WDYT? I can take over this PR and make that change.
|
We don't need to actually. There is already a option in GrpcService that adds the initial metadata. https://github.com/envoyproxy/envoy/blob/9186ebf374b1bd7d7c65ed1cec748377f88ec0a7/api/envoy/api/v2/core/grpc_service.proto#L225 With that I think we could just remove this function from ABI without adding that param. |
|
While the initial metadata can indeed be set in the GrpcService proto, you need to modify proto for each request and you're mixing service and per-stream metadata in a weird way, IMHO. I think we should still add the initial metadata to the calls, like @jplevyak mentioned, and as we discussed in the past. What do you think? |
|
Yeah that makes sense to me. John talked to me offline and he is going to work on the PR to add that param. He mentioned the other reason though that GrpcService proto is Envoy specific and we want to make ABI proxy neutral. |
Signed-off-by: Pengyuan Bian bianpengyuan@google.com
For an explanation of how to fill out the fields, please see the relevant section
in PULL_REQUESTS.md
Description:
Risk Level:
Testing:
Docs Changes:
Release Notes:
[Optional Fixes #Issue]
[Optional Deprecated:]