> On Dec. 4, 2019, 6:43 p.m., Benjamin Mahler wrote: > > src/master/authorization.cpp > > Lines 38 (patched) > > <https://reviews.apache.org/r/71859/diff/2/?file=2181799#file2181799line38> > > > > Actually, come to think of it, this construction style seems quite odd? > > > > We're mixing construction and mutation, which means we have a > > intermediate state where the action object is not valid. > > > > I guess the struct-like approach doesn't work because we don't want to > > default initialize the action field: > > > > ``` > > ActionObject actionObject; > > > > actionObject.action = authorization::RUN_TASK; > > actionObject.object = Object(); > > *actionObject.object->mutable_task_info() = task; > > ``` > > > > So it seems we should go with something like: > > > > ``` > > Object object = ...; > > ... > > > > ActionObject actionObject( > > actionObject.action = authorization::RUN_TASK, > > std::move(object)); > > ``` > > > > Also, might want to make this constructor private for only the helpers > > to use? Or do we want callers using it?
Good point, thanks! Converted ActionObject into a class with private constructor. - Andrei ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/71859/#review218916 ----------------------------------------------------------- On Jan. 2, 2020, 8 p.m., Andrei Sekretenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/71859/ > ----------------------------------------------------------- > > (Updated Jan. 2, 2020, 8 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 > ------- > > This is the first patch in the chain that extract Master code > generating Action-Object pairs into a dedicated ActionObject class, > thus seperating authz Object creation from feeding them into authorizer. > > This is a prerequisite to using ObjectApprover interface for > synchronous authorization of Scheduler API calls. > > > Diffs > ----- > > src/CMakeLists.txt ef9382dc77d37fed344b7267119f3251acd3088a > src/Makefile.am 111c156c8a7abc5ece04779e8ac8879a30c22dbf > src/master/authorization.hpp PRE-CREATION > src/master/authorization.cpp PRE-CREATION > src/master/master.hpp f97b085ae908278731acd326df68f9f381f09483 > src/master/master.cpp 14b90a5e276df055bb8a570331f27cab200c9869 > > > Diff: https://reviews.apache.org/r/71859/diff/3/ > > > Testing > ------- > > > Thanks, > > Andrei Sekretenko > >
