----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44319/#review122961 -----------------------------------------------------------
Fix it, then Ship it! Looks good. Some minor comments. include/mesos/authorizer/authorizer.hpp (line 61) <https://reviews.apache.org/r/44319/#comment185058> s/returned value/the authorizer/ src/authorizer/local/authorizer.hpp (line 50) <https://reviews.apache.org/r/44319/#comment185061> Add a comment for this? src/authorizer/local/authorizer.cpp (line 353) <https://reviews.apache.org/r/44319/#comment185063> s/stringAcls/acls/ src/authorizer/local/authorizer.cpp (line 364) <https://reviews.apache.org/r/44319/#comment185065> s/acls/acls_/ src/local/local.cpp (line 228) <https://reviews.apache.org/r/44319/#comment185066> s/create/authorizer/ src/local/local.cpp (line 243) <https://reviews.apache.org/r/44319/#comment185067> s/authorizer/authorizer_/ src/master/main.cpp (line 372) <https://reviews.apache.org/r/44319/#comment185068> So, we create non-default authorizer if --authorizers specifies a non-default name but create the default authorizer only if --acls is present (irrespective of whether --authorizers says "local)". I think those semantics are a bit weird to grok. IIRC, we did it that way for backwards compatibility with old releases that didn't have "--authorizers" flag? We should atleast mention this behavior in the flags help. src/tests/cluster.cpp (line 175) <https://reviews.apache.org/r/44319/#comment185069> s/create/authorizer/ - Vinod Kone On March 9, 2016, 2:06 p.m., Alexander Rojas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44319/ > ----------------------------------------------------------- > > (Updated March 9, 2016, 2:06 p.m.) > > > Review request for mesos, Adam B, Joerg Schad, and Vinod Kone. > > > Bugs: MESOS-2950 > https://issues.apache.org/jira/browse/MESOS-2950 > > > Repository: mesos > > > Description > ------- > > Removed initialize method from the authorizer interface. > > > Diffs > ----- > > include/mesos/authorizer/authorizer.hpp > 705482656788ac12e9d21e355b71fd2ede2be558 > src/authorizer/authorizer.cpp 54278b022118c40d3b976794fd472ce8d8b6a5e2 > src/authorizer/local/authorizer.hpp > c87a9915bae6bae7744bd57abd12e8d857181051 > src/authorizer/local/authorizer.cpp > 15c857de79cd7dd2c29b6b8cfb81204b919f1b28 > src/examples/test_authorizer_module.cpp > 95d77fbff0cdfdb360a8597fbba28404b59d0042 > src/local/local.cpp 359fc54d7c4081f536a8de8b1dfcde413d75c9a9 > src/master/main.cpp 4263110c9b889984ef74eb94fed629958f2abd79 > src/tests/authorization_tests.cpp 2b2297036550412a955ff479f6ec9d7dad8cb0e3 > src/tests/cluster.cpp 084fb1ce37a315c561c4587c4761c870f54c8625 > > Diff: https://reviews.apache.org/r/44319/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Alexander Rojas > >
