----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/22222/#review44968 -----------------------------------------------------------
Let's definitely reuse the --credentials flag (and thus s/Authentications/Credentials). We can try parsing as a JSON object first and if that fails then fall back to parsing as before (and if that fails then exit and report the error). src/authentications/authentications.hpp <https://reviews.apache.org/r/22222/#comment79583> You'll want to first try and parse to JSON (and if that fails fall back and parse as we've done in the past with --credentials since we want to override that flag). src/messages/messages.proto <https://reviews.apache.org/r/22222/#comment79584> s/Authentications/Credentials/ here and everywhere else! See top-level comment. src/messages/messages.proto <https://reviews.apache.org/r/22222/#comment79579> Plus +2 indents here please. src/messages/messages.proto <https://reviews.apache.org/r/22222/#comment79585> So, I guess Credentials::HTTP ends up being a bit weird after all. :-( How do you feel about having people use 'principal' and 'secret' instead of 'username' and 'password'? I think we should also use the public Credential protobuf too, no need to duplicate that here. - Benjamin Hindman On June 3, 2014, 11:15 p.m., Isabel Jimenez wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/22222/ > ----------------------------------------------------------- > > (Updated June 3, 2014, 11:15 p.m.) > > > Review request for mesos, Benjamin Hindman, Dominic Hamon, and Till Toenshoff. > > > Bugs: MESOS-1391 > https://issues.apache.org/jira/browse/MESOS-1391 > > > Repository: mesos-git > > > Description > ------- > > After Ben's comments, regroup of all types of authentication in one common > definition, this is a draft for this Issue, here are some questions: > > - Do I replace credentials flag completely or if this slight duplication will > suffice for now ? > - If I don't replace it, for now as it is in this patch, the flag is useless, > I should give it to the sasl/authenticator but I would really appreciate some > comments for that part. > > > Diffs > ----- > > src/authentications/authentications.hpp PRE-CREATION > src/master/flags.hpp 4863359 > src/master/master.cpp 766a0e3 > src/messages/messages.proto 6f6e570 > > Diff: https://reviews.apache.org/r/22222/diff/ > > > Testing > ------- > > > Thanks, > > Isabel Jimenez > >
