You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Meng Zhu <mz...@mesosphere.io> on 2019/03/13 06:43:15 UTC
Review Request 70202: Added call handler for `UPDATE_QUOTA`.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70202/
-----------------------------------------------------------
Review request for mesos and Benjamin Mahler.
Bugs: MESOS-9596
https://issues.apache.org/jira/browse/MESOS-9596
Repository: mesos
Description
-------
The current implementation is incomplete and only does call
validation. It returns 501 `NOT_IMPLEMENTED`.
Further work includes authorization and actual enforcement
of the quota in the allocator.
Diffs
-----
src/master/master.hpp 953cc5b8ab6a8e1920a3ad63fb2dd6382e3603ec
src/master/quota_handler.cpp 8d417a9b2a2fa0d83d98d65167adfada2994f75c
Diff: https://reviews.apache.org/r/70202/diff/1/
Testing
-------
make check
dedicated test added in subsequent patches
Thanks,
Meng Zhu
Re: Review Request 70202: Added call handler for `UPDATE_QUOTA`.
Posted by Meng Zhu <mz...@mesosphere.io>.
> On March 13, 2019, 12:54 p.m., Benjamin Mahler wrote:
> > src/master/master.hpp
> > Lines 1169 (patched)
> > <https://reviews.apache.org/r/70202/diff/1/?file=2131821#file2131821line1169>
> >
> > Hm.. what does "available" mean here? Is this expressing an intention to only use it for v1? Or something else?
Yeah, I want to point out the update quota api is v1 only. But I think the comment is not clear about that. Removed.
> On March 13, 2019, 12:54 p.m., Benjamin Mahler wrote:
> > src/master/quota_handler.cpp
> > Lines 433-434 (patched)
> > <https://reviews.apache.org/r/70202/diff/1/?file=2131822#file2131822line433>
> >
> > Maybe this should more directly say the role is not in the whitelist?
Sounds good.
> On March 13, 2019, 12:54 p.m., Benjamin Mahler wrote:
> > src/master/quota_handler.cpp
> > Lines 451 (patched)
> > <https://reviews.apache.org/r/70202/diff/1/?file=2131822#file2131822line451>
> >
> > Rather than saying "failed to validate" maybe say "Invalid `QuotaConfig`: ..." or something?
> >
> > Failed to validate seems to suggest we failed at doing something?
> >
> > Ditto above in the other BadRequest
Sounds good.
- Meng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70202/#review213683
-----------------------------------------------------------
On March 12, 2019, 11:43 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70202/
> -----------------------------------------------------------
>
> (Updated March 12, 2019, 11:43 p.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9596
> https://issues.apache.org/jira/browse/MESOS-9596
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The current implementation is incomplete and only does call
> validation. It returns 501 `NOT_IMPLEMENTED`.
>
> Further work includes authorization and actual enforcement
> of the quota in the allocator.
>
>
> Diffs
> -----
>
> src/master/master.hpp 953cc5b8ab6a8e1920a3ad63fb2dd6382e3603ec
> src/master/quota_handler.cpp 8d417a9b2a2fa0d83d98d65167adfada2994f75c
>
>
> Diff: https://reviews.apache.org/r/70202/diff/2/
>
>
> Testing
> -------
>
> make check
> dedicated test added in subsequent patches
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 70202: Added call handler for `UPDATE_QUOTA`.
Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/70202/#review213683
-----------------------------------------------------------
Fix it, then Ship it!
src/master/master.hpp
Lines 1169 (patched)
<https://reviews.apache.org/r/70202/#comment299684>
Hm.. what does "available" mean here? Is this expressing an intention to only use it for v1? Or something else?
src/master/quota_handler.cpp
Lines 429 (patched)
<https://reviews.apache.org/r/70202/#comment299685>
auto&& or const auto&?
src/master/quota_handler.cpp
Lines 433-434 (patched)
<https://reviews.apache.org/r/70202/#comment299686>
Maybe this should more directly say the role is not in the whitelist?
src/master/quota_handler.cpp
Lines 451 (patched)
<https://reviews.apache.org/r/70202/#comment299687>
Rather than saying "failed to validate" maybe say "Invalid `QuotaConfig`: ..." or something?
Failed to validate seems to suggest we failed at doing something?
Ditto above in the other BadRequest
- Benjamin Mahler
On March 13, 2019, 6:43 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/70202/
> -----------------------------------------------------------
>
> (Updated March 13, 2019, 6:43 a.m.)
>
>
> Review request for mesos and Benjamin Mahler.
>
>
> Bugs: MESOS-9596
> https://issues.apache.org/jira/browse/MESOS-9596
>
>
> Repository: mesos
>
>
> Description
> -------
>
> The current implementation is incomplete and only does call
> validation. It returns 501 `NOT_IMPLEMENTED`.
>
> Further work includes authorization and actual enforcement
> of the quota in the allocator.
>
>
> Diffs
> -----
>
> src/master/master.hpp 953cc5b8ab6a8e1920a3ad63fb2dd6382e3603ec
> src/master/quota_handler.cpp 8d417a9b2a2fa0d83d98d65167adfada2994f75c
>
>
> Diff: https://reviews.apache.org/r/70202/diff/1/
>
>
> Testing
> -------
>
> make check
> dedicated test added in subsequent patches
>
>
> Thanks,
>
> Meng Zhu
>
>