> On April 18, 2016, 3:28 p.m., Jan Schlicht wrote: > > src/slave/http.cpp, lines 613-616 > > <https://reviews.apache.org/r/46319/diff/1/?file=1348102#file1348102line613> > > > > You don't need to do that. If authorization is disabled, > > `authorizedEndpoint` will always return `true`.
The original intention here was to avoid needlessly copying of especially `request` when not needed. I now removed this and we always copy, like also in your patch. > On April 18, 2016, 3:28 p.m., Jan Schlicht wrote: > > src/slave/http.cpp, lines 625-626 > > <https://reviews.apache.org/r/46319/diff/1/?file=1348102#file1348102line625> > > > > Break before the '?', like > > ``` > > return !authorized.get() > > ? Forbidden() > > : _statistics(context, *limiter, request); Done. > On April 18, 2016, 3:28 p.m., Jan Schlicht wrote: > > src/slave/http.cpp, line 624 > > <https://reviews.apache.org/r/46319/diff/1/?file=1348102#file1348102line624> > > > > Use the resulting `bool` directly, instead of the future. `then` only > > gets called when the future is ready, no need to test if the future failed. Good point. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/46319/#review129327 ----------------------------------------------------------- On April 18, 2016, 3:42 p.m., Benjamin Bannier wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/46319/ > ----------------------------------------------------------- > > (Updated April 18, 2016, 3:42 p.m.) > > > Review request for mesos, Adam B, Alexander Rojas, and Jan Schlicht. > > > Bugs: MESOS-5164 > https://issues.apache.org/jira/browse/MESOS-5164 > > > Repository: mesos > > > Description > ------- > > Added authorization to agents' `/statistics` endpoints. > > > Diffs > ----- > > src/slave/http.cpp 3908e33ed5b233387790276f6f5d884452087d2c > src/slave/slave.hpp f78c1b4e4d5378ef7223c6eb3ea45491c30fb4c1 > src/tests/slave_tests.cpp ee58488b0b927c7c5833add4718941539663e6d2 > > Diff: https://reviews.apache.org/r/46319/diff/ > > > Testing > ------- > > make check (OS X, clang w/o optimization) > > > Thanks, > > Benjamin Bannier > >
