----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/#review70051 -----------------------------------------------------------
Can you expand description on why you designed the authenticator inteface this way? Particularly, expand on "The initial design and implementation of the authenticator module interface caused issues and was not optimal for heavy lifting setup of external dependencies" and "The new design also gets us back on track to the goal of makeing SASL a soft dependency of mesos". An issue with the new design is that now users have to implement an Authenticator and an AuthenticatorSession to implement a new type of authenticator. Also, we lost the symmetry between authenticatee and authenticator. An Authenticatee now communicates with AuthenticatorSession instead of Authenticator, which is confusing. Is the plan to refactor the Authenticatee similarly? src/authentication/cram_md5/authenticator.hpp <https://reviews.apache.org/r/27760/#comment114999> lets keep the definitions for all methods of this class all in one place (.cpp). src/authentication/cram_md5/auxprop.hpp <https://reviews.apache.org/r/27760/#comment115003> s/persistence/properties/ ? src/master/master.cpp <https://reviews.apache.org/r/27760/#comment115034> Hmm. I think this is a serious enough error that the master should fail during initilization rather than sending an error message here. Do you have use cases in mind that warranted this? src/master/master.cpp <https://reviews.apache.org/r/27760/#comment115036> IIRC, sending an AuthenticationErrorMessage here causes the driver to simply retry the authentication and thus fall into a retry loop? - Vinod Kone On Jan. 26, 2015, 8:49 a.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27760/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2015, 8:49 a.m.) > > > Review request for mesos, Adam B, Kapil Arya, Niklas Nielsen, and Vinod Kone. > > > Bugs: MESOS-2050 > https://issues.apache.org/jira/browse/MESOS-2050 > > > Repository: mesos-git > > > Description > ------- > > The initial design and implementation of the authenticator module interface > caused issues and was not optimal for heavy lifting setup of external > dependencies. By introducing a two fold design, this has been decoupled from > the authentication message processing. The new design also gets us back on > track to the goal of makeing SASL a soft dependency of mesos. > > > Diffs > ----- > > src/authentication/authenticator.hpp 460494a > src/authentication/cram_md5/authenticator.hpp d739a02 > src/authentication/cram_md5/auxprop.hpp b894386 > src/authentication/cram_md5/auxprop.cpp cf503a2 > src/master/master.hpp 1d342e5 > src/master/master.cpp bda8fda > src/tests/cram_md5_authentication_tests.cpp a356aa1 > > Diff: https://reviews.apache.org/r/27760/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Till Toenshoff > >
