----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/63021/#review192513 -----------------------------------------------------------
Fix it, then Ship it! src/common/type_utils.cpp Lines 746 (patched) <https://reviews.apache.org/r/63021/#comment270703> Please udpate v1 src/resource_provider/storage/provider.cpp Lines 154 (patched) <https://reviews.apache.org/r/63021/#comment270710> Can we change `getContainerIdPrefix` to not have the tailing `-`? So here, you basically construct three things: 1) prefix 2) csi plugin type/name 3) services and join them by `--` src/resource_provider/storage/provider.cpp Lines 508-511 (patched) <https://reviews.apache.org/r/63021/#comment270726> Instead of nesting, i'd prefer flat them out: ``` return client .then(defer(..., [=](const csi::Client& client) { return client.GetSupportedVersion(...); }) .then(defer(..., [=](const ...) { ... }); ``` Any reason not using `const ref` for the parameter? src/resource_provider/storage/provider.cpp Lines 591 (patched) <https://reviews.apache.org/r/63021/#comment270735> You need to return after calling `fatal` - Jie Yu On Dec. 1, 2017, 1:36 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/63021/ > ----------------------------------------------------------- > > (Updated Dec. 1, 2017, 1:36 a.m.) > > > Review request for mesos, James DeFelice, Jie Yu, Joseph Wu, and Jan Schlicht. > > > Bugs: MESOS-8032 > https://issues.apache.org/jira/browse/MESOS-8032 > > > Repository: mesos > > > Description > ------- > > The `getService()` method first checks if there is already a container > daemon for the specified plugin component, and creates a new one if not. > The post-start hook for the container daemon will call `connect()` to > wait for the endpoint socket file to appear and connect to it, then > set up the corresponding promise of CSI client. The post-stop hook will > remove the socket file and create a new promise for the next call to the > post-start hook to set it up. > > > Diffs > ----- > > include/mesos/type_utils.hpp a348c7df083324602d25f40069ad49e29f7918a5 > src/common/type_utils.cpp 3657d55ab2a052b5c659022e3fc264e748429d54 > src/resource_provider/storage/provider.cpp > f586afc256fbcb2f2bf2451ffb7e7ba1d59cfa78 > > > Diff: https://reviews.apache.org/r/63021/diff/14/ > > > Testing > ------- > > > Thanks, > > Chun-Hung Hsiao > >
