tikv / grpc-rs

The gRPC library for Rust built on C Core library and futures
Apache License 2.0
1.81k stars 253 forks source link

Support sending initial metadata and trailing metadata #460

Open BusyJay opened 4 years ago

BusyJay commented 4 years ago

Currently, only sending initial metadata from client side and receiving initial metadata from server side are supported. The missing parts are:

BusyJay commented 3 years ago

After investigating the design of other languages, I propose the following API designs:

Sending metadata from client

This is supported already in CallOption.

let builder = MetadataBuilder::new();
let opt = CallOption::default().headers(builder.build());

Receiving metadata from client

Introducing a new struct is a way, but it adds extra maintenance. We can add extra methods for two Receivers instead.

pub async fn headers(&mut self) -> Metadata {...}
pub async fn trailer(&mut self) -> Metadata {...}

So only one struct needs to be used and if people are not interested in metadata, they don't need to change their code.

Sending metadata from server

Similar to receiving metadata from client, we can also use separate methods for server sink.

pub async fn send_headers(&mut self, header: Metadata);
pub fn set_headers(&mut self, header: Metadata);
pub fn set_trailer(&mut self, trailer: Metadata);

set_headers and set_trailer set the metadata and sends lazily. send_headers sets the header and send immediately.

Receiving metadata from server

This is supported already.

for (key, val) in ctx.request_headers() {
    ...
}

The interface design are mainly copied from golang, so this can also be friendly for users with similar background. One thing need to take care is that we need extra validation checks like headers set after being sent is meaningless.

What do you think? /cc @christian-oudard

christian-oudard commented 3 years ago

@BusyJay For the use case at MobileCoin, we will need streaming metadata as well. I will start implementing this API in our pull request, including streaming support.

Look for updates in the next couple weeks at https://github.com/tikv/grpc-rs/pull/532

christian-oudard commented 3 years ago

@BusyJay, please take a look at the code I have written so far in PR #543.

I have some questions about how to continue the implementation.

First off, I was unable to make the API spec you gave me work. The problem is that implementing Future on ClientUnaryReceiver means that .await does a move of the receiver into the await call, which does not allow us to do anything else afterwards, such as getting metadata. I solved this by adding an async function, .message(), which waits for the BatchFuture on a borrowed self.

Also, there is a clippy error with my implementation of Metadata::from, which is telling me to do unsafe fn from(..., but this is rejected by the compiler.

Can you help with these issues?

BusyJay commented 3 years ago

...by adding an async function, .message(), which waits for the BatchFuture on a borrowed self

I think it's a fair solution.

...a clippy error with my implementation of Metadata::from...

I commented on the PR.

Let's continue the discussion there.