You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Chun-Hung Hsiao <ch...@apache.org> on 2018/05/16 19:28:11 UTC
Review Request 67158: Updated the CSI client to use the new
libprocess gRPC interface.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67158/
-----------------------------------------------------------
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
-------
This patch makes the following changes:
1. Replace `GRPC_RPC` with `GRPC_CLIENT_METHOD`.
2. Replace `process::grpc::Channel` with
`process::grpc::client::Connection`.
3. Make the CSI client metods return a `Future` of a `Try`.
Diffs
-----
src/csi/client.hpp 5d846740f72125efb3654e14d763bd24634367b9
src/csi/client.cpp 559e8057eccce5bf758918c24e5ca8c561af6592
src/tests/csi_client_tests.cpp f5b9eac38a4079cc2873ce2e2de24eaf315e0bc9
src/tests/mock_csi_plugin.hpp 6f7a5ab327875c0277b4e73ea9500d0fbaea8034
src/tests/mock_csi_plugin.cpp 6983b842aeede917599bfb711197738b95856fe1
Diff: https://reviews.apache.org/r/67158/diff/1/
Testing
-------
sudo make check
Thanks,
Chun-Hung Hsiao
Re: Review Request 67158: Updated the CSI client to use the new
libprocess gRPC interface.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67158/#review203322
-----------------------------------------------------------
Fix it, then Ship it!
src/csi/client.cpp
Lines 31-38 (original), 31-38 (patched)
<https://reviews.apache.org/r/67158/#comment285439>
Maybe more a personal preference, but I preferred the previous wrapping with `.call` and `.then` on their own lines (I'd value being able to consistently and quickly follow the flow higher than being able to always list args to `call` on one line).
Here and below.
Feel free to drop if you disagree.
src/csi/client.cpp
Lines 35-40 (original), 35-40 (patched)
<https://reviews.apache.org/r/67158/#comment285440>
This looks much better now!
- Benjamin Bannier
On May 16, 2018, 9:28 p.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67158/
> -----------------------------------------------------------
>
> (Updated May 16, 2018, 9:28 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
> -------
>
> This patch makes the following changes:
> 1. Replace `GRPC_RPC` with `GRPC_CLIENT_METHOD`.
> 2. Replace `process::grpc::Channel` with
> `process::grpc::client::Connection`.
> 3. Make the CSI client metods return a `Future` of a `Try`.
>
>
> Diffs
> -----
>
> src/csi/client.hpp 5d846740f72125efb3654e14d763bd24634367b9
> src/csi/client.cpp 559e8057eccce5bf758918c24e5ca8c561af6592
> src/tests/csi_client_tests.cpp f5b9eac38a4079cc2873ce2e2de24eaf315e0bc9
> src/tests/mock_csi_plugin.hpp 6f7a5ab327875c0277b4e73ea9500d0fbaea8034
> src/tests/mock_csi_plugin.cpp 6983b842aeede917599bfb711197738b95856fe1
>
>
> Diff: https://reviews.apache.org/r/67158/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 67158: Updated the CSI client to use the new
libprocess gRPC interface.
Posted by Chun-Hung Hsiao <ch...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67158/
-----------------------------------------------------------
(Updated May 17, 2018, 7:38 p.m.)
Review request for mesos, Benjamin Bannier, Benjamin Mahler, Jie Yu, and Zhitao Li.
Changes
-------
Addressed Benjamin's comments.
Bugs: MESOS-8924
https://issues.apache.org/jira/browse/MESOS-8924
Repository: mesos
Description
-------
This patch makes the following changes:
1. Replace `GRPC_RPC` with `GRPC_CLIENT_METHOD`.
2. Replace `process::grpc::Channel` with
`process::grpc::client::Connection`.
3. Make the CSI client metods return a `Future` of a `Try`.
Diffs (updated)
-----
src/csi/client.hpp 5d846740f72125efb3654e14d763bd24634367b9
src/csi/client.cpp 559e8057eccce5bf758918c24e5ca8c561af6592
src/tests/csi_client_tests.cpp f5b9eac38a4079cc2873ce2e2de24eaf315e0bc9
src/tests/mock_csi_plugin.hpp 6f7a5ab327875c0277b4e73ea9500d0fbaea8034
src/tests/mock_csi_plugin.cpp 6983b842aeede917599bfb711197738b95856fe1
Diff: https://reviews.apache.org/r/67158/diff/2/
Changes: https://reviews.apache.org/r/67158/diff/1-2/
Testing
-------
sudo make check
Thanks,
Chun-Hung Hsiao
Re: Review Request 67158: Updated the CSI client to use the new
libprocess gRPC interface.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67158/#review203289
-----------------------------------------------------------
PASS: Mesos patch 67158 was successfully built and tested.
Reviews applied: `['67154', '67155', '67156', '67157', '67158']`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67158
- Mesos Reviewbot Windows
On May 16, 2018, 7:28 p.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67158/
> -----------------------------------------------------------
>
> (Updated May 16, 2018, 7:28 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
> -------
>
> This patch makes the following changes:
> 1. Replace `GRPC_RPC` with `GRPC_CLIENT_METHOD`.
> 2. Replace `process::grpc::Channel` with
> `process::grpc::client::Connection`.
> 3. Make the CSI client metods return a `Future` of a `Try`.
>
>
> Diffs
> -----
>
> src/csi/client.hpp 5d846740f72125efb3654e14d763bd24634367b9
> src/csi/client.cpp 559e8057eccce5bf758918c24e5ca8c561af6592
> src/tests/csi_client_tests.cpp f5b9eac38a4079cc2873ce2e2de24eaf315e0bc9
> src/tests/mock_csi_plugin.hpp 6f7a5ab327875c0277b4e73ea9500d0fbaea8034
> src/tests/mock_csi_plugin.cpp 6983b842aeede917599bfb711197738b95856fe1
>
>
> Diff: https://reviews.apache.org/r/67158/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>
Re: Review Request 67158: Updated the CSI client to use the new
libprocess gRPC interface.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67158/#review203296
-----------------------------------------------------------
Patch looks great!
Reviews applied: [67154, 67155, 67156, 67157, 67158]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On May 16, 2018, 7:28 p.m., Chun-Hung Hsiao wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67158/
> -----------------------------------------------------------
>
> (Updated May 16, 2018, 7:28 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
> -------
>
> This patch makes the following changes:
> 1. Replace `GRPC_RPC` with `GRPC_CLIENT_METHOD`.
> 2. Replace `process::grpc::Channel` with
> `process::grpc::client::Connection`.
> 3. Make the CSI client metods return a `Future` of a `Try`.
>
>
> Diffs
> -----
>
> src/csi/client.hpp 5d846740f72125efb3654e14d763bd24634367b9
> src/csi/client.cpp 559e8057eccce5bf758918c24e5ca8c561af6592
> src/tests/csi_client_tests.cpp f5b9eac38a4079cc2873ce2e2de24eaf315e0bc9
> src/tests/mock_csi_plugin.hpp 6f7a5ab327875c0277b4e73ea9500d0fbaea8034
> src/tests/mock_csi_plugin.cpp 6983b842aeede917599bfb711197738b95856fe1
>
>
> Diff: https://reviews.apache.org/r/67158/diff/1/
>
>
> Testing
> -------
>
> sudo make check
>
>
> Thanks,
>
> Chun-Hung Hsiao
>
>