----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/36049/#review90317 -----------------------------------------------------------
src/authorizer/authorizer.cpp (lines 17 - 28) <https://reviews.apache.org/r/36049/#comment143338> Great update! But it seems the comment is not applying, no? The built-in authorizer factory `Try<Authorizer*> LocalAuthorizer::create()` does not seem to test for `nullptr`. You seem to do those checks in master.cpp and local.cpp instead. Or am I missing the point here? src/local/local.cpp (lines 227 - 228) <https://reviews.apache.org/r/36049/#comment143336> You may reach this without an error-message, no? How about: ``` if (create.isError() || create.get() == nullptr) { EXIT(EXIT_FAILURE) << "Could not create authorizer module '" << flags.authorizer << "'" << (create.isError() ? ": " + create.error() : ""); ; } ``` Or actually, that factory can not return a `nullptr` anymore, can it? src/master/flags.cpp (lines 415 - 424) <https://reviews.apache.org/r/36049/#comment143339> We may want to follow the pattern used for the authenticator - as in, call the flag "authorizers" and prepare the CLI interface for allowing the activation of multiple, concurrently active authorizers. What do you think? src/master/main.cpp (line 311) <https://reviews.apache.org/r/36049/#comment143337> See local.cpp - Till Toenshoff On July 2, 2015, 11:34 p.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/36049/ > ----------------------------------------------------------- > > (Updated July 2, 2015, 11:34 p.m.) > > > Review request for mesos and Till Toenshoff. > > > Bugs: MESOS-2945 > https://issues.apache.org/jira/browse/MESOS-2945 > > > Repository: mesos > > > Description > ------- > > Adds all the helper classes needed to support an Authorizer module. > > It also adds a flag which allows to select the module name. > > > Diffs > ----- > > include/mesos/authorizer/authorizer.hpp PRE-CREATION > include/mesos/module/authorizer.hpp PRE-CREATION > src/Makefile.am addb63f615f16ae6b25f745b2e79fd9fc0e27851 > src/authorizer/authorizer.cpp PRE-CREATION > src/local/local.cpp 1953d84c75a83f4ace944d6243456235d8a193ff > src/master/constants.hpp 7cec18b7fdfd3b96cde42a30d217c026b2695dce > src/master/constants.cpp fbcae60c43e835f96ec061bd0e9f7961e31fc341 > src/master/flags.hpp f2cd19a6edfaa4e5bb31f024ef8d5beda32fbc2f > src/master/flags.cpp 60ac64d98d53f74f904846b27a3833a7c44a9756 > src/master/main.cpp 2624b7ea4920a534c98f5dfbf9286c54c50f11a9 > src/module/manager.cpp 909ca56eea85d365cb9ebe1b3cce43051cabb670 > > Diff: https://reviews.apache.org/r/36049/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Alexander Rojas > >
