> On Jan. 18, 2019, 11:03 a.m., Benjamin Bannier wrote: > > src/examples/test_csi_plugin.cpp > > Lines 970 (patched) > > <https://reviews.apache.org/r/69787/diff/1/?file=2120294#file2120294line989> > > > > Could we give this a better name, e.g., `completionQueue`? That would > > be less gRPC'y, but more Mesos'y.
Done. > On Jan. 18, 2019, 11:03 a.m., Benjamin Bannier wrote: > > src/examples/test_csi_plugin.cpp > > Lines 990 (patched) > > <https://reviews.apache.org/r/69787/diff/1/?file=2120294#file2120294line1009> > > > > We should log something here as this could fail in general. Properly handled `ok == false` in each scenario. > On Jan. 18, 2019, 11:03 a.m., Benjamin Bannier wrote: > > src/examples/test_csi_plugin.cpp > > Line 1020 (original), 1146 (patched) > > <https://reviews.apache.org/r/69787/diff/1/?file=2120294#file2120294line1165> > > > > I was wondering whether one would need to explicitly shut down the > > server and completion queue, or does that happen in their destructors? > > I.e., are we missing code here or do we just have an undocumented lifetime > > ordering? Added the proper shutdown logic. Will add more comments. > On Jan. 18, 2019, 11:03 a.m., Benjamin Bannier wrote: > > src/examples/test_csi_plugin.cpp > > Line 1025 (original), 1168 (patched) > > <https://reviews.apache.org/r/69787/diff/1/?file=2120294#file2120294line1187> > > > > nit: Not this patch, but this is the default and not required in C++. Could you explain more? Do you mean I should do `return 0` instead? - Chun-Hung ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/69787/#review212136 ----------------------------------------------------------- On Jan. 18, 2019, 5:46 a.m., Chun-Hung Hsiao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/69787/ > ----------------------------------------------------------- > > (Updated Jan. 18, 2019, 5:46 a.m.) > > > Review request for mesos, Benjamin Bannier and Jie Yu. > > > Bugs: MESOS-9517 > https://issues.apache.org/jira/browse/MESOS-9517 > > > Repository: mesos > > > Description > ------- > > If the `--proxy` flag is set, the test CSI plugin would forward all gRPC > requests to the specified gRPC server URI, and return the response to > the caller. This can be used to forward all CSI calls to a mock CSI > plugin object in the test process. > > > Diffs > ----- > > src/examples/test_csi_plugin.cpp af183037b280bab65578a4c447196a9ccf261e32 > > > Diff: https://reviews.apache.org/r/69787/diff/1/ > > > Testing > ------- > > Manually tweaked the plugin to forward requests to itself and all tests pass. > > > Thanks, > > Chun-Hung Hsiao > >
