> 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
> 
>

Reply via email to