----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58584/#review172587 -----------------------------------------------------------
LGTM src/master/quota_handler.cpp Lines 518-521 (patched) <https://reviews.apache.org/r/58584/#comment245683> How about putting this before building `QuotaTree` so we don't bother to validate nested quota for now. Also, how about changing error message to something like: `"Setting quota on nested role '" + quotaInfo.role() + "' is not supported yet"`, so we don't mislead users to believe that we don't plan to. src/tests/hierarchical_allocator_tests.cpp Lines 4610 (patched) <https://reviews.apache.org/r/58584/#comment245686> How about `NON_QUOTA_ROLE` to be explicit. src/tests/hierarchical_allocator_tests.cpp Lines 4613 (patched) <https://reviews.apache.org/r/58584/#comment245684> I like indexing from zero too, but I think we start from one in most of the tests. Also the agent starts from one in this test case :P src/tests/hierarchical_allocator_tests.cpp Lines 4617 (patched) <https://reviews.apache.org/r/58584/#comment245685> Let's make value of mem greater than `MIN_MEM` to be consistent with other tests. - Jay Guo On April 21, 2017, 2:18 a.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58584/ > ----------------------------------------------------------- > > (Updated April 21, 2017, 2:18 a.m.) > > > Review request for mesos, Benjamin Mahler, Jay Guo, and Michael Park. > > > Repository: mesos > > > Description > ------- > > Correct support for quota on nested roles will require further work in > the allocator (see MESOS-7402 for details). For now, setting quota on > nested roles is disabled until MESOS-7402 can be fixed. This commit > disables any tests that rely on setting quota on nested roles; it also > adds a (disabled) test to cover the behavior that will be fixed as part > of MESOS-7402. > > > Diffs > ----- > > src/master/quota_handler.cpp 7ff43a048e17b9e9ac0ceed248f7b3fd56b007d6 > src/tests/hierarchical_allocator_tests.cpp > 33e7b455f8664858eb4f03727b076a10c80cd6e0 > src/tests/master_quota_tests.cpp 1714ba13ea63bae05448d0898bf722ef472c672b > > > Diff: https://reviews.apache.org/r/58584/diff/2/ > > > Testing > ------- > > `make check` > > > Thanks, > > Neil Conway > >
