> On Feb. 9, 2017, 3:03 p.m., Jan Schlicht wrote: > > 3rdparty/libprocess/src/authenticator_manager.cpp, lines 168-204 > > <https://reviews.apache.org/r/56474/diff/1/?file=1628065#file1628065line168> > > > > How about we make this a function instead of using a lambda? > > Seems to do a lot and even contains another lambda in line 180.
The outer lambda needs to be something like a mutable lambda so that it is instantiated once when `loop` is invoked, allowing us to capture the `results` list once and push onto it as we iterate. I suppose we could accomplish this with a struct bearing this code in its `()` operator, which we instantiate here, but I'm not sure I like that more. Perhaps there's a way of doing this that I'm not thinking of? It would be possible to use `std::bind` with a static member function instead of the inner lambda, but since we use the `loop`-related constructs `Break` and `Continue` within that block, I think it actually makes the code a bit more readable to keep it all in one place. Let me know what you think. > On Feb. 9, 2017, 3:03 p.m., Jan Schlicht wrote: > > 3rdparty/libprocess/src/authenticator_manager.cpp, line 160 > > <https://reviews.apache.org/r/56474/diff/1/?file=1628065#file1628065line160> > > > > Could also be `vector<AuthenticationResult> > > results(authenticators_[realm].size());`. Of course, > > `combineFailedRequests` would need to be changed respectively. Yep good point, that is another option. I think I'm going to do some benchmarking of this patch vs. the old authentication code, so perhaps I can benchmark the list vs. vector in this case as well. - Greg ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56474/#review164924 ----------------------------------------------------------- On Feb. 11, 2017, 12:44 a.m., Greg Mann wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56474/ > ----------------------------------------------------------- > > (Updated Feb. 11, 2017, 12:44 a.m.) > > > Review request for mesos, Alexander Rojas, Jan Schlicht, Till Toenshoff, and > Vinod Kone. > > > Bugs: MESOS-7004 > https://issues.apache.org/jira/browse/MESOS-7004 > > > Repository: mesos > > > Description > ------- > > This patch updates the `AuthenticatorManager` to allow > multiple authenticators to be set for a single realm. > > > Diffs > ----- > > 3rdparty/libprocess/src/authenticator_manager.cpp > a22acd026a001788dc39b8005a56577e33c6800b > > Diff: https://reviews.apache.org/r/56474/diff/ > > > Testing > ------- > > Testing information can be found in the subsequent patch in this chain. > > > Thanks, > > Greg Mann > >
