Upgrade tonic to v0.5.0

[tdyas]

Upgrade the Tonic crate to v0.5.0 and Prost to v0.8.0.

Notes:

• tonic::Interceptor was removed. The with_interceptor functions now takes a FnMut(tonic::Request<()>) -> Result<tonic::Request<()>, Status> type for the interceptor. Moreover, FooClient::new and FooClient::with_interceptor now return different types and so the return values from each function cannot be stored in the same field any more. Also, with_interceptor now returns an InterceptedService which encodes the type of the FnMut into its type signature. Thus, when I tried using an "identity interceptor" function alongside the actual header interceptor function, the use of InterceptedService failed to compile because each closure had a different type and trying to box and/or use an Arc just ended up in type hell.

  • The solution was to introduce a SetRequestHeadersLayer based on the tower-http crate and to update layered_service (and LayeredService) to always take a set of headers to apply.

• tonic::Status is no longer Clone. The derivations of Clone in testutil/mock for various types have been removed.

• Fixed a header setup issue in src/rust/engine/process_execution/src/remote.rs where user agent may not have been passed through into one of the channels because of the way in which the header map is modified in place.

[Eric-Arellano]

Thanks!

[illicitonion]

Looks good!

[illicitonion]

If you're happy to only return the first error, rather than wanting to collect them all, you can collect directly to Result<Vec<_>, _> rather than Vec<Result<_, _>> which would allow you to avoid needing to then partition as well. And then actually, a HeaderMap is directly collectable-to; i.e. this function could literally be:

pub fn headers_to_http_header_map(headers: &BTreeMap<String, String>) -> Result<HeaderMap, String> {
 headers
    .iter()
    .map(|(key, value)| {
      let header_name = HeaderName::from_str(key.as_str())
        .map_err(|err| format!("Invalid header name {}: {}", key, err))?;

      let header_value = HeaderValue::from_str(value.as_str())
        .map_err(|err| format!("Invalid header value {}: {}", value, err))?;

      Ok((header_name, header_value))
    })
    .collect::<Result<_, String>>()
    .map_err(|err| format!("header conversion error: {}", err))
}

Not saying you necessarily should, just letting you know that you could :)

[tdyas]

Implicit deref conversion works. Switched to that.

[tdyas]

Cleaned up the header conversion a little, but left in the ability to report multiple errors.