zio-grpc icon indicating copy to clipboard operation
zio-grpc copied to clipboard

feat: Add client support for response metadata

Open regiskuckaertz opened this issue 2 years ago • 6 comments

@thesamet even though we can now send trailers, there is no way to consume them in the client 😓

I think the generated code should offer support for it but I would like your opinion first. Since it would change the result type, we can either:

  • add *WithMetadata methods on the client impl, or
  • generate an "advanced" client impl with the same method names that yields the metadata

regiskuckaertz avatar Sep 06 '22 12:09 regiskuckaertz

Hi @regiskuckaertz , let's create a seperate advanced client and use same-name methods. I can be convinced otherwise, but my rational for the choice is to keep interface for 99% of the users cleaner. Ideally, the simple client implementation uses the advanced clients internally through an instance it holds, so users should be to call client.withMetadata.myMethod(...`).

Once this works, please add accessor methods too that follow the same naming.

I haven't looked too closely, but I think that the advanced client should have all its methods return streams, even unary calls can return both initial and trailing metadata.

thesamet avatar Sep 06 '22 15:09 thesamet

@thesamet I was of the same opinion

regiskuckaertz avatar Sep 07 '22 13:09 regiskuckaertz

@thesamet I'll add some tests, but so far so good

regiskuckaertz avatar Sep 11 '22 10:09 regiskuckaertz

@thesamet check it out in your own time

regiskuckaertz avatar Sep 16 '22 08:09 regiskuckaertz

@thesamet ping

regiskuckaertz avatar Sep 30 '22 12:09 regiskuckaertz

Sorry for the delay. I'm out of town without desktop access until mid October. Overall looks good, though I'd like to take a closer look at the generated code. Can you prepare a PR for master that will be used for review? We will use this as the backport PR

thesamet avatar Sep 30 '22 12:09 thesamet