Re: Review Request 36927: Pulled apart authorization and authentication validation for frameworks in the master.

2015-07-30 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36927/#review93657 --- Ship it! Ship It! - Vinod Kone On July 30, 2015, 10:16 p.m.,

Re: Review Request 36927: Pulled apart authorization and authentication validation for frameworks in the master.

2015-07-30 Thread Ben Mahler
On July 30, 2015, 12:49 a.m., Vinod Kone wrote: Kept the validation error composition per our offline discussion, returning for each case individually led to really verbose code, and we looked at using a lambda to leverage 'return', but this seemed to be the simplest route for now. On

Re: Review Request 36927: Pulled apart authorization and authentication validation for frameworks in the master.

2015-07-30 Thread Ben Mahler
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36927/ --- (Updated July 30, 2015, 10:16 p.m.) Review request for mesos and Vinod Kone.

Re: Review Request 36927: Pulled apart authorization and authentication validation for frameworks in the master.

2015-07-29 Thread Vinod Kone
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36927/#review93532 --- src/master/master.cpp (lines 1574 - 1576)

Re: Review Request 36927: Pulled apart authorization and authentication validation for frameworks in the master.

2015-07-29 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36927/#review93549 --- Patch looks great! Reviews applied: [36927] All tests passed. -

Re: Review Request 36927: Pulled apart authorization and authentication validation for frameworks in the master.

2015-07-29 Thread Ben Mahler
On July 30, 2015, 12:49 a.m., Vinod Kone wrote: src/master/master.cpp, lines 1757-1779 https://reviews.apache.org/r/36927/diff/1/?file=1024921#file1024921line1757 checking validationError.isNone() in each if statement looks a bit weird. how about doing these in an else if