----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66464/#review200680 -----------------------------------------------------------
src/master/http.cpp Lines 959-973 (original), 959-969 (patched) <https://reviews.apache.org/r/66464/#comment281526> Hmm I think we only want to execute this block for calls which will get a response body (i.e., for SUBSCRIBE and RECONCILE_OPERATIONS calls). The other calls do not require that the 'Accept' header be set. src/master/http.cpp Lines 5097 (patched) <https://reviews.apache.org/r/66464/#comment281527> s/reconcileOperations/std::move(reconcileOperations)/ src/master/http.cpp Lines 5099-5100 (patched) <https://reviews.apache.org/r/66464/#comment281594> Fits on one line. src/master/master.hpp Lines 2556 (patched) <https://reviews.apache.org/r/66464/#comment281529> A return type of `Try<Operation*>` seems semanticallly more appropriate here? Also, let's pass the id as a const ref. src/master/master.cpp Lines 8923-8929 (patched) <https://reviews.apache.org/r/66464/#comment281592> We would also hit this block when `slaveId.isNone()` - should we handle that case separately so that we can provide a more accurate message? - Greg Mann On April 5, 2018, 12:47 a.m., Gaston Kleiman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66464/ > ----------------------------------------------------------- > > (Updated April 5, 2018, 12:47 a.m.) > > > Review request for mesos and Greg Mann. > > > Repository: mesos > > > Description > ------- > > Implemented operation status reconciliation. > > > Diffs > ----- > > src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 > src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 > src/master/master.cpp 18382fa93fd0c59e641e00f2028ac1ae2e67c01c > > > Diff: https://reviews.apache.org/r/66464/diff/1/ > > > Testing > ------- > > `sudo bin/mesos-tests` on GNU/Linux > > https://reviews.apache.org/r/66468/ adds new tests. > > > Thanks, > > Gaston Kleiman > >
