> On April 21, 2017, 6:01 a.m., Jay Guo wrote: > > src/master/quota_handler.cpp > > Lines 518-521 (patched) > > <https://reviews.apache.org/r/58584/diff/2/?file=1695640#file1695640line518> > > > > 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.
Personally, I actually like validating quota for nested roles before returning the "not supported yet" error message: the quota validation code works fine and is not expensive, so I don't think we need to disable it. - Neil ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58584/#review172587 ----------------------------------------------------------- On April 20, 2017, 6:18 p.m., Neil Conway wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58584/ > ----------------------------------------------------------- > > (Updated April 20, 2017, 6:18 p.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 > >