> On May 16, 2018, 8:36 p.m., Benjamin Bannier wrote: > > 3rdparty/libprocess/include/process/grpc.hpp > > Line 86 (original), 84-93 (patched) > > <https://reviews.apache.org/r/67154/diff/1/?file=2024140#file2024140line86> > > > > Is this any different than > > > > StatusError(const ::grpc::Status& _status) > > : Error(_status.error_message()), > > status(_status) ... > > > > We currently don't seem to require this to be able to take generic > > types.
This is for perfect forwarding (i.e., copying when passing const ref and moving when passing rvalue ref). If you think this is not needed then I'll take your suggestion. > On May 16, 2018, 8:36 p.m., Benjamin Bannier wrote: > > 3rdparty/libprocess/include/process/grpc.hpp > > Line 122 (original), 135 (patched) > > <https://reviews.apache.org/r/67154/diff/1/?file=2024140#file2024140line138> > > > > This would look like a good use case for a hypothetical `Future` with a > > custom error type (instead of `std::string` aka `Failure`) > > > > template <typename T, typename E> struct Future; > > > > I believe there is a bigger interest in customizing the error type of > > `Try` recently, and it might make sense to think about adding that same > > power to `Future`. I created > > https://issues.apache.org/jira/browse/MESOS-8925 to track such work. This would be a huge work, as it would be very hard to make typed failure to flow through a chain of futures. - Chun-Hung ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67154/#review203273 ----------------------------------------------------------- On May 16, 2018, 7:20 p.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/67154/ > ----------------------------------------------------------- > > (Updated May 16, 2018, 7:20 p.m.) > > > Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and > Zhitao Li. > > > Bugs: MESOS-8924 > https://issues.apache.org/jira/browse/MESOS-8924 > > > Repository: mesos > > > Description > ------- > > The `process::grpc::client::Runtime::call` method currently returns a > `RpcResult<Response>`, which contains both a `::grpc::Status` object > and the resulting response protobuf. However, if the `::grpc::Status` > represents a non-OK status, the gRPC library does not guarantee that > the response protobuf is valid. This patch replaces `RpcResult` with > `Try` to provide better type safety. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/grpc.hpp > 321a46e19c69eafb24012bcef68bb8b0cc6aa436 > 3rdparty/libprocess/src/tests/grpc_tests.cpp > 38cd6c61b54518a1019bb11a3551be13026c3f0d > > > Diff: https://reviews.apache.org/r/67154/diff/1/ > > > Testing > ------- > > make check in libprocess > > NOTE: Mesos cannot be built with this patch standalone. The tests are done > later in the chain. > > > Thanks, > > Chun-Hung Hsiao > >
