Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review130419 --- Ship it! Ship It! - Alexander Rojas On April 22, 2016,

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review130384 --- Ship it! Ship It! - Adam B On April 22, 2016, 3:11 a.m.,

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review130153 --- Patch looks great! Reviews applied: [45922] Passed command:

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 22, 2016, 12:11 p.m.) Review request for mesos, Adam B and

Re: Review Request 45922: Added agent authorization flags.

> On April 22, 2016, 9:35 a.m., Adam B wrote: > > src/tests/cluster.cpp, line 392 > > > > > > Double underscores for variable names are discouraged (usually only > > acceptable for continuation function names). >

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 22, 2016, 12:05 p.m.) Review request for mesos, Adam B and

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review130041 --- Fix it, then Ship it! Looks great! Just one issue with

Re: Review Request 45922: Added agent authorization flags.

> On April 20, 2016, 12:26 a.m., Adam B wrote: > > src/tests/cluster.hpp, line 151 > > > > > > Why do you even need the overload for the authorizer here? Seems like > > most tests will either provide --acls and use

Re: Review Request 45922: Added agent authorization flags.

> On April 20, 2016, 9:26 a.m., Adam B wrote: > > src/tests/cluster.cpp, lines 406-412 > > > > > > Why is it an error to start an agent with no authorizer and no ACLs? > > What if I don't want to do any

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 21, 2016, 3:45 p.m.) Review request for mesos, Adam B and

Re: Review Request 45922: Added agent authorization flags.

> On April 20, 2016, 9:26 a.m., Adam B wrote: > > src/tests/cluster.hpp, line 151 > > > > > > Why do you even need the overload for the authorizer here? Seems like > > most tests will either provide --acls and use

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review129694 --- Looks great. So very close! Just an unnecessary slave::start()

Re: Review Request 45922: Added agent authorization flags.

> On April 12, 2016, 12:30 p.m., Joerg Schad wrote: > > src/slave/flags.hpp, line 102 > > > > > > Do we have a particular order here? I don't know exactly. I kept it consistent to the one in `master/flags.hpp`. In

Re: Review Request 45922: Added agent authorization flags.

> On April 15, 2016, 1:33 p.m., Benjamin Bannier wrote: > > src/tests/mesos.cpp, lines 373-377 > > > > > > Could you add a similar helper to inject e.g., a mock authorizer? It looks like these helper functions are

Re: Review Request 45922: Added agent authorization flags.

> On April 15, 2016, 12:16 p.m., Alexander Rojas wrote: > > src/slave/slave.hpp, line 108 > > > > > > I think is a better practice to move optional parameters at the end and > > default initialize them to `None()`.

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 18, 2016, 11:49 a.m.) Review request for mesos, Adam B and

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review129097 --- src/tests/mesos.cpp (lines 373 - 377)

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review129089 --- src/local/local.cpp (line 393)

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 14, 2016, 6:51 p.m.) Review request for mesos, Adam B and

Re: Review Request 45922: Added agent authorization flags.

> On April 12, 2016, 11:31 a.m., Adam B wrote: > > docs/configuration.md, line 886 > > > > > > None of these examples apply to an agent. Maybe we need to implement an > > ACL (e.g. GET_ENDPOINT_WITH_PATH "/flags")

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 14, 2016, 6:16 p.m.) Review request for mesos, Adam B and

Re: Review Request 45922: Added agent authorization flags.

> On April 13, 2016, 3:58 p.m., Benjamin Bannier wrote: > > src/tests/cluster.cpp, lines 398-410 > > > > > > You are shadowing the outer `Option` here with a > > `Result`. All of the

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review128875 --- Patch looks great! Reviews applied: [45922] Passed command:

Re: Review Request 45922: Added agent authorization flags.

> On April 12, 2016, 11:31 a.m., Adam B wrote: > > docs/configuration.md, line 94 > > > > > > Why would we need to support multiple authorizers? Would a particular > > request check two authorizers then and/or the

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review128666 --- src/tests/cluster.cpp (lines 398 - 410)

Re: Review Request 45922: Added agent authorization flags.

> On April 12, 2016, 2:31 a.m., Adam B wrote: > > docs/configuration.md, line 94 > > > > > > Why would we need to support multiple authorizers? Would a particular > > request check two authorizers then and/or the

Re: Review Request 45922: Added agent authorization flags.

> On April 12, 2016, 11:31 a.m., Adam B wrote: > > docs/configuration.md, line 94 > > > > > > Why would we need to support multiple authorizers? Would a particular > > request check two authorizers then and/or the

Re: Review Request 45922: Added agent authorization flags.

> On April 12, 2016, 9:31 a.m., Adam B wrote: > > src/slave/flags.cpp, lines 446-449 > > > > > > I'd rather have no example than a confusing/invalid example. We can add > > an example after we have at least one ACL

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review128356 --- docs/configuration.md (line 867)

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review128350 --- Looks nice and clean. Three main concerns: 1. Why support

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review127833 --- Patch looks great! Reviews applied: [45922] Passed command:

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 8, 2016, 4:25 p.m.) Review request for mesos, Adam B and

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review127768 --- src/slave/flags.cpp (lines 459 - 491)

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review127781 --- Patch looks great! Reviews applied: [45922] Passed command:

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/ --- (Updated April 8, 2016, 11:26 a.m.) Review request for mesos, Adam B and

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review127753 --- src/slave/flags.cpp (line 446)

Re: Review Request 45922: Added agent authorization flags.

--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45922/#review127751 --- src/slave/flags.cpp (lines 459 - 491)