----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26817/#review57190 -----------------------------------------------------------
src/master/master.hpp <https://reviews.apache.org/r/26817/#comment97724> // Returns true if task is authorized. // Returns false if task is not authorized. // Returns failure for transient authorization failures. src/master/master.cpp <https://reviews.apache.org/r/26817/#comment97729> Hmm. This should be rethought now because there is no reason validateTask should return a future anymore. The only async part of validation was authentication, which is now split out of it. src/master/master.cpp <https://reviews.apache.org/r/26817/#comment97731> We should only proceed with authorization if validations succeeded. Otherwise there is no point. Also, it is a bit weird that we do all validations for all tasks and then do all authorizations for all tasks. I wonder if we can solve the original problem (not able to distinguish between validation failure vs authorization failure) differently, e.g., by returning ValidationError instead of Error, where the former contains more context. struct ValidationError : Error { enum Reason; } Ofcourse this assumes, the "Reason" patch lands first. Thoughts? src/master/master.cpp <https://reviews.apache.org/r/26817/#comment97735> The caller shouldn't call this method if authorization is disabled. Move this logic to the caller. - Vinod Kone On Oct. 16, 2014, 7:06 p.m., Dominic Hamon wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/26817/ > ----------------------------------------------------------- > > (Updated Oct. 16, 2014, 7:06 p.m.) > > > Review request for mesos and Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > split authorization from task validation. allows > https://reviews.apache.org/r/26382/ to differentiate between validation > issues and task authorization issues. > > also replaced some > > with >> and changed the order of methods in the cpp to > match the continuation order. > > > Diffs > ----- > > src/master/master.hpp 14f1d0fd240c9cd0718d0238e1fbb9c733190205 > src/master/master.cpp 1b1ce0de3065ced45890777d42c69ef1091f5699 > > Diff: https://reviews.apache.org/r/26817/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Dominic Hamon > >
