----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/60107/#review178049 -----------------------------------------------------------
src/common/http.hpp Lines 178-180 (patched) <https://reviews.apache.org/r/60107/#comment251832> Maybe something simpler like: "Parent class for authorization-based filters." src/common/http.hpp Lines 184-185 (patched) <https://reviews.apache.org/r/60107/#comment251833> Let's make this private or protected. src/common/http.hpp Lines 185 (patched) <https://reviews.apache.org/r/60107/#comment251841> Indent two more spaces. src/common/http.hpp Lines 195-196 (patched) <https://reviews.apache.org/r/60107/#comment251835> Let's make this private or protected. src/common/http.hpp Lines 196 (patched) <https://reviews.apache.org/r/60107/#comment251842> Indent two more spaces. src/common/http.hpp Lines 198-200 (patched) <https://reviews.apache.org/r/60107/#comment251836> Let's try to move this declaration into the parent class. src/common/http.cpp Lines 1133-1134 (patched) <https://reviews.apache.org/r/60107/#comment251839> Would prefer: ``` const Option<authorization::Subject> subject = authorization::createSubject(principal); ``` src/common/http.cpp Lines 1136-1138 (patched) <https://reviews.apache.org/r/60107/#comment251840> Would prefer: ``` return authorizer.get()->getObjectApprover( subject, authorization::VIEW_TASK) .then([=](const Owned<ObjectApprover>& approver) { ``` src/common/http.cpp Lines 1139 (patched) <https://reviews.apache.org/r/60107/#comment251838> Actually, we should avoid using `new` here: ``` return AuthorizeTaskInfoFilter(approver); ``` - Greg Mann On June 15, 2017, 5:01 p.m., Quinn Leng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/60107/ > ----------------------------------------------------------- > > (Updated June 15, 2017, 5:01 p.m.) > > > Review request for mesos, Anand Mazumdar, Greg Mann, and Vinod Kone. > > > Bugs: MESOS-7630 > https://issues.apache.org/jira/browse/MESOS-7630 > > > Repository: mesos > > > Description > ------- > > Add class definition for ObjectFilter. > > Write class definition for ObjectFilter, AuthorizeFilter, > which is the superclass for all Filters related with > authorization. > > Add asynchronize construction method for AuthorizeFilter, > since it will incorporate the construction of ObjectApprover, > which is constructed asynchronously > > > Diffs > ----- > > src/common/http.hpp 93d6088e97c2384f9f6d26e010a501abf2deb43e > src/common/http.cpp 167dce2b9a2d3b68a1df5b4079f701482d34db28 > > > Diff: https://reviews.apache.org/r/60107/diff/1/ > > > Testing > ------- > > Passed "make check" > No specific test case related > > > Thanks, > > Quinn Leng > >
