----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/#review69371 -----------------------------------------------------------
Ship it! LGTM. All I could find is minor nits. The recent changes match very closely with Vinod and BenM's suggestions. Putting the Once (back) in solves the problem of multiple authenticators in the same linux process trying to add/initialize the plugin, and the Locks prevent simultaneous load/lookup. The only remaining concern, as mentioned in your TODO, is multiple CRAM-MD5 authenticators potentially calling a common secrets::load(), but the Lock in InMemoryAuxiliaryPropertyPlugin::load should at least synchronize those calls. Maybe in the long run we'll want one auxprop plugin per authenticator, but we can figure that out when we start supporting multiple authenticators. src/authentication/cram_md5/authenticator.hpp <https://reviews.apache.org/r/27760/#comment114071> s/CRAMMD5AuthenticatorProcess/CRAMMD5AuthenticatorSessionProcess/ src/authentication/cram_md5/auxprop.hpp <https://reviews.apache.org/r/27760/#comment114072> s/> >/>>/ ? src/authentication/cram_md5/auxprop.hpp <https://reviews.apache.org/r/27760/#comment114068> s/persistance/persistence/ s/non static/non-static/ src/master/master.cpp <https://reviews.apache.org/r/27760/#comment114074> Could probably create this promise later, after checking the authenticator and creating the session, just before calling session->authenticate. - Adam B On Jan. 21, 2015, 6:08 a.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27760/ > ----------------------------------------------------------- > > (Updated Jan. 21, 2015, 6:08 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 a8ce4d0 > src/master/master.cpp 78d5f47 > src/tests/cram_md5_authentication_tests.cpp a356aa1 > > Diff: https://reviews.apache.org/r/27760/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Till Toenshoff > >
