----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/59100/#review176702 -----------------------------------------------------------
src/master/http.cpp Lines 4148-4151 (patched) <https://reviews.apache.org/r/59100/#comment250134> See Till's comment on previous RR regarding spacing here, to make it consistent with the rest of this file. src/master/http.cpp Lines 4172-4173 (original), 4186-4194 (patched) <https://reviews.apache.org/r/59100/#comment250136> Why not use `defer` here, instead of the double lambda? Something like: ``` return approver.then(defer( master->self(), [=](Owned<ObjectApprover> approver) -> Future<Response> { const mesos::maintenance::Schedule schedule = _getMaintenanceSchedule(approver); return OK(JSON::protobuf(schedule), request.url.query.get("jsonp")); }, lambda::_1)); ``` src/master/http.cpp Lines 4222-4240 (patched) <https://reviews.apache.org/r/59100/#comment250143> Simply copying into a new `Schedule` message is inefficient, but probably OK for now, as it's more readable and the output of this endpoint may not become too large. I'm mostly just leaving this comment for posterity, to note that we may want to use the more sophisiticated JSON writer-based filtering in the future if performance becomes an issue, as we do in the `/state` handler. src/master/http.cpp Lines 4331-4337 (original), 4383-4394 (patched) <https://reviews.apache.org/r/59100/#comment250152> Ditto here regarding `defer`. src/tests/api_tests.cpp Lines 1263-1268 (patched) <https://reviews.apache.org/r/59100/#comment250160> Ditto here, regarding changing from this callback-based pattern to the standard `AWAIT_EXPECT_RESPONSE_STATUS_EQ` pattern. - Greg Mann On June 1, 2017, 2:57 p.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/59100/ > ----------------------------------------------------------- > > (Updated June 1, 2017, 2:57 p.m.) > > > Review request for mesos, Adam B, Greg Mann, and Till Toenshoff. > > > Bugs: MESOS-7415 > https://issues.apache.org/jira/browse/MESOS-7415 > > > Repository: mesos > > > Description > ------- > > Enables the use of authorization for the `GET_MAINTENANCE_SCHEDULE` > v1 API call, using the ACL `GetMaintenanceSchedule` and the action > of the same name as the API call. > > It also updates the ApiTests to take into account the authorization > case. > > > Diffs > ----- > > src/master/http.cpp 7060b8fa53e0682681c50e051908ffbbf50fb7da > src/master/master.hpp 89d0790fd5fea59e74276f462581fe0073594732 > src/tests/api_tests.cpp faf3242f9c86d866c4bb5e457fcfe47c1063cc09 > > > Diff: https://reviews.apache.org/r/59100/diff/4/ > > > Testing > ------- > > make check > > > Thanks, > > Alexander Rojas > >
