> On Sept. 25, 2013, 5:32 p.m., Vinod Kone wrote: > > configure.ac, line 516 > > <https://reviews.apache.org/r/14310/diff/3/?file=356654#file356654line516> > > > > I thought we needed couple other packages too. Can we check for them > > here?
I've added a TODO because properly checking is considerably more involved. > On Sept. 25, 2013, 5:32 p.m., Vinod Kone wrote: > > src/messages/messages.proto, line 344 > > <https://reviews.apache.org/r/14310/diff/3/?file=356657#file356657line344> > > > > Should the mechanisms be enums? I think strings are okay. We have enough string manipulation techniques that makes this easy. > On Sept. 25, 2013, 5:32 p.m., Vinod Kone wrote: > > src/sasl/authenticatee.hpp, line 81 > > <https://reviews.apache.org/r/14310/diff/3/?file=356658#file356658line81> > > > > Why virtual? http://stackoverflow.com/questions/461203/when-to-use-virtual-destructors > On Sept. 25, 2013, 5:32 p.m., Vinod Kone wrote: > > src/sasl/authenticatee.hpp, line 284 > > <https://reviews.apache.org/r/14310/diff/3/?file=356658#file356658line284> > > > > Can you add a comment here on why you set it to false instead of an > > error? I put a comment on Authenticatee::authenticate instead! > On Sept. 25, 2013, 5:32 p.m., Vinod Kone wrote: > > src/sasl/authenticatee.hpp, line 52 > > <https://reviews.apache.org/r/14310/diff/3/?file=356658#file356658line52> > > > > Since want users to not re-use Authenticatee for multiple attempts, how > > about taking the 'pid' as part of constructor argument? That probably > > drives the point home better? I thought about this a bit but I don't really like the idea of keeping the pid around after the first authenticate call because subsequent calls should not "by accident" use it but use 'from' instead. - Benjamin ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/14310/#review26379 ----------------------------------------------------------- On Sept. 24, 2013, 8:26 p.m., Benjamin Hindman wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/14310/ > ----------------------------------------------------------- > > (Updated Sept. 24, 2013, 8:26 p.m.) > > > Review request for mesos, Ben Mahler and Vinod Kone. > > > Repository: mesos-git > > > Description > ------- > > Initial SASL based authentication support. (I'll likely also be cleaning some > stuff up as I deal with compilation issues with different libsasl2* versions.) > > > Diffs > ----- > > configure.ac 96ac7a81311e9f3765bd54e078368566a3cc33ca > include/mesos/mesos.proto 8f845cc9b7cd491622cb29a69f6909170510a664 > src/Makefile.am 3eae9642b639e78f101e7fe580aa803beadbebda > src/messages/messages.proto 4d400c2d8f6ecb9f7cf8635b2a98341206bbb6c5 > src/sasl/authenticatee.hpp PRE-CREATION > src/sasl/authenticator.hpp PRE-CREATION > src/sasl/auxprop.hpp PRE-CREATION > src/sasl/auxprop.cpp PRE-CREATION > src/tests/sasl_tests.cpp PRE-CREATION > > Diff: https://reviews.apache.org/r/14310/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Benjamin Hindman > >