----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22316/#review45320 -----------------------------------------------------------
src/master/master.hpp <https://reviews.apache.org/r/22316/#comment80131> we allow non-authenticated but registered with principal right? src/master/master.hpp <https://reviews.apache.org/r/22316/#comment80132> s/an// src/master/master.cpp <https://reviews.apache.org/r/22316/#comment80137> pull this outside to the top of the if block? src/master/master.cpp <https://reviews.apache.org/r/22316/#comment80207> Why is this Owned and on a heap? src/master/master.cpp <https://reviews.apache.org/r/22316/#comment80138> why the change? src/master/master.cpp <https://reviews.apache.org/r/22316/#comment80139> const&? src/master/master.cpp <https://reviews.apache.org/r/22316/#comment80140> s/must/must be/ src/master/master.cpp <https://reviews.apache.org/r/22316/#comment80221> To simplify this can we just do if (frameworks.principals.contains(framework->pid)) { } src/tests/rate_limiting_tests.cpp <https://reviews.apache.org/r/22316/#comment80225> s/is/are/ src/tests/rate_limiting_tests.cpp <https://reviews.apache.org/r/22316/#comment80226> I've seen this block in various tests. We should probably have a helper for this. Maybe a TODO for now. src/tests/rate_limiting_tests.cpp <https://reviews.apache.org/r/22316/#comment80227> s/multiple/different/ src/tests/rate_limiting_tests.cpp <https://reviews.apache.org/r/22316/#comment80229> s/the framework/the first framework/ src/tests/rate_limiting_tests.cpp <https://reviews.apache.org/r/22316/#comment80230> why will there be subsequent registered callbacks? src/tests/rate_limiting_tests.cpp <https://reviews.apache.org/r/22316/#comment80232> kill "and to make sure..." because you AWAIT on frameworkId for that. src/tests/rate_limiting_tests.cpp <https://reviews.apache.org/r/22316/#comment80234> const& src/tests/rate_limiting_tests.cpp <https://reviews.apache.org/r/22316/#comment80235> const & src/tests/rate_limiting_tests.cpp <https://reviews.apache.org/r/22316/#comment80237> This comment should be pulled down. Here you are just sending a dup. - Vinod Kone On June 11, 2014, 5:39 p.m., Jiang Yan Xu wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22316/ > ----------------------------------------------------------- > > (Updated June 11, 2014, 5:39 p.m.) > > > Review request for mesos, Ben Mahler, Dominic Hamon, and Vinod Kone. > > > Bugs: MESOS-1339 > https://issues.apache.org/jira/browse/MESOS-1339 > > > Repository: mesos-git > > > Description > ------- > > - Multiple frameworks use the same principal use the same counter. > > > Diffs > ----- > > src/Makefile.am c91b438c3401ff815116d6faab4f0a34bc0fe3fd > src/master/master.hpp 7a121856806ff2d79661fa31e491ddd6e4512c59 > src/master/master.cpp 4a01b1aacfff83c62aa2ec3b46ad64e631488d09 > src/tests/rate_limiting_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/22316/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Jiang Yan Xu > >