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