mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Benjamin Mahler <bmah...@apache.org>
Subject Re: Review Request 71863: Moved creating authorization Object out of `Master::authorize*Volume`.
Date Wed, 04 Dec 2019 20:07:18 GMT

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



Hm.. the Try cases bring up a broader question about validation, see below!


src/master/authorization.cpp
Lines 132 (patched)
<https://reviews.apache.org/r/71863/#comment306846>

    Ahhh.. here is where we need the action-only constructor! Perhaps have this review introduce
it? (see the initial review's comments about avoiding construction then mutation)



src/master/authorization.cpp
Lines 145-147 (patched)
<https://reviews.apache.org/r/71863/#comment306848>

    A broader question that relates to all of these reviews with Try values.. are we essentially
changing the way validation works? Will validation no longer be catching cases (and creating
error message for them) because we now catch them during authorization, and potentially send
worse error messages than the validation code generates?



src/master/authorization.cpp
Lines 146 (patched)
<https://reviews.apache.org/r/71863/#comment306847>

    must be



src/master/http.cpp
Lines 1067 (patched)
<https://reviews.apache.org/r/71863/#comment306845>

    `std::move(*actionObjects)` ?



src/master/master.cpp
Lines 4707-4713 (original), 4594-4601 (patched)
<https://reviews.apache.org/r/71863/#comment306844>

    This is inconsistent with the last review?
    
    i.e. `s/authz/actionObjects/`
         `std::move(*actionObjects);`


- Benjamin Mahler


On Dec. 3, 2019, 5:54 p.m., Andrei Sekretenko wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71863/
> -----------------------------------------------------------
> 
> (Updated Dec. 3, 2019, 5:54 p.m.)
> 
> 
> Review request for mesos, Benjamin Bannier and Benjamin Mahler.
> 
> 
> Bugs: MESOS-10023 and MESOS-10056
>     https://issues.apache.org/jira/browse/MESOS-10023
>     https://issues.apache.org/jira/browse/MESOS-10056
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Moved creating authorization Object out of `Master::authorize*Volume`.
> 
> 
> Diffs
> -----
> 
>   src/master/authorization.hpp PRE-CREATION 
>   src/master/authorization.cpp PRE-CREATION 
>   src/master/http.cpp e03655863ea2d4e4464b3d14b359de3d7f059778 
>   src/master/master.hpp 93630421d58e6fd26566e81a23cd910957795665 
>   src/master/master.cpp 14b90a5e276df055bb8a570331f27cab200c9869 
> 
> 
> Diff: https://reviews.apache.org/r/71863/diff/1/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Andrei Sekretenko
> 
>


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