> On Feb. 20, 2015, 2:19 p.m., Vinod Kone wrote: > > src/master/master.cpp, lines 3897-3898 > > <https://reviews.apache.org/r/27760/diff/17/?file=869282#file869282line3897> > > > > Why do we need this 'onAny'? Why can't 'authenticated' be set in > > __authenticate()? > > Till Toenshoff wrote: > `__authenticated` is needed to capture (at least) a discard on the outer > future. I have rearranged the code of both continuations to be more explicit > about that. > > Vinod Kone wrote: > A Future::discard() *does not* result in a future being DISCARDED (and > onAny callbacks being called). You want an onDiscard() handler (instead of > onAny) to capture that case.
> "A Future::discard() does not result in a future being DISCARDED" That sounds strange. What does it do then? Is an onDiscard() handler going to be sufficient here? > On Feb. 20, 2015, 2:19 p.m., Vinod Kone wrote: > > src/master/master.cpp, lines 3952-3954 > > <https://reviews.apache.org/r/27760/diff/17/?file=869282#file869282line3952> > > > > Can you rephrase these log messages to something meaninguful when one > > looks at master log? > > Till Toenshoff wrote: > How about this? > ``` > if (authenticate.isDiscarded()) { > LOG(WARNING) << "Authentication for " << pid > << " has pending cancel request"; > } else if (authenticate.discard()) { > LOG(WARNING) << "Requested to cancel authentication for " << pid; > } > ``` > > Adam B wrote: > How about: "Tried to cancel authentication session for [pid], but it was > already cancelled" and "Cancelling authentication session for PID"? > > Vinod Kone wrote: > How are you cancelling the authentication here? The intention was that discarding the authenticate future would trigger the (now onDiscard) handler, which would end the authentication session. - Adam ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27760/#review73310 ----------------------------------------------------------- On March 5, 2015, 11:22 a.m., Till Toenshoff wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27760/ > ----------------------------------------------------------- > > (Updated March 5, 2015, 11:22 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 > > > 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 > ----- > > include/mesos/authentication/authenticator.hpp f66217a > src/Makefile.am d299f07 > src/authentication/cram_md5/authenticator.hpp c6f465f > src/authentication/cram_md5/authenticator.cpp PRE-CREATION > src/authentication/cram_md5/auxprop.hpp b894386 > src/authentication/cram_md5/auxprop.cpp cf503a2 > src/master/master.hpp 3c957ab > src/master/master.cpp 68ca19a > src/tests/cram_md5_authentication_tests.cpp 92a89c5 > > Diff: https://reviews.apache.org/r/27760/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Till Toenshoff > >
