mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jie Yu" <yujie....@gmail.com>
Subject Re: Review Request 39989: [5/5] Added framework authorization for dynamic reservation.
Date Fri, 20 Nov 2015 01:06:53 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39989/#review107281
-----------------------------------------------------------



src/master/master.cpp (lines 3016 - 3028)
<https://reviews.apache.org/r/39989/#comment166313>

    Let's move the validation to aurhtorizeReserveResources.
    
    You also need to add a NOTE in `_accept` saying that no need to validate the operation
again since it's already validated during authorization.



src/master/master.cpp (lines 3053 - 3054)
<https://reviews.apache.org/r/39989/#comment166312>

    I don't think the comments here is necessary. We need to avoid over-commenting. If the
code itself is quite clear about what it's about to do, no need for the extra comments here.



src/master/master.cpp (line 3160)
<https://reviews.apache.org/r/39989/#comment166314>

    No need for this comment.



src/master/master.cpp (line 3195)
<https://reviews.apache.org/r/39989/#comment166311>

    I don't think this comment add more value. The code itself is quite clear what it is about
to do.



src/master/master.cpp (line 3202)
<https://reviews.apache.org/r/39989/#comment166315>

    Ditto.



src/master/master.cpp (line 3206)
<https://reviews.apache.org/r/39989/#comment166316>

    Remove this comment.



src/master/master.cpp (line 3237)
<https://reviews.apache.org/r/39989/#comment166317>

    Ditto.



src/master/master.cpp (line 3310)
<https://reviews.apache.org/r/39989/#comment166318>

    Ditto.



src/tests/reservation_tests.cpp (lines 1657 - 1668)
<https://reviews.apache.org/r/39989/#comment166320>

    This might be flaky because master might send out an offer before it processes the terminal
status update.


- Jie Yu


On Nov. 20, 2015, 12:20 a.m., Greg Mann wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39989/
> -----------------------------------------------------------
> 
> (Updated Nov. 20, 2015, 12:20 a.m.)
> 
> 
> Review request for mesos, Adam B, Jie Yu, Michael Park, and Till Toenshoff.
> 
> 
> Bugs: MESOS-3062
>     https://issues.apache.org/jira/browse/MESOS-3062
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Added framework authorization for dynamic reservation.
> Note: this review is continued from https://reviews.apache.org/r/37127/
> 
> 
> Diffs
> -----
> 
>   src/master/master.cpp 39a0e730b230cee73e30d831ee67d9207359ae28 
>   src/tests/reservation_tests.cpp ac664ebb49e74aa28551f427ea8f39ac9ce0cfb3 
> 
> Diff: https://reviews.apache.org/r/39989/diff/
> 
> 
> Testing
> -------
> 
> This is the fifth in a chain of 5 patches. New reservation tests were added to `reservation_tests.cpp`
to validate the authentication of framework reserve and unreserve operations using ACLs. `make
check` was run to test after all patches were applied.
> 
> 
> Thanks,
> 
> Greg Mann
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message