Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-22 Thread Adam B
> On July 21, 2016, 7:38 p.m., Adam B wrote: > > src/slave/slave.cpp, lines 443-451 > > > > > > This logic seems to be missing from your new patch. > > Zhitao Li wrote: > This is because I aligned this logic

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-22 Thread Adam B
> On July 21, 2016, 7:38 p.m., Adam B wrote: > > src/slave/slave.cpp, lines 410-415 > > > > > > This error is gone? > > Zhitao Li wrote: > Yes, because I refactored the code to be aligned with master side. This

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-22 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50223/ --- (Updated July 22, 2016, 6:21 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-22 Thread Greg Mann
> On July 22, 2016, 5:07 a.m., Greg Mann wrote: > > src/slave/flags.cpp, lines 791-793 > > > > > > I think the word "authenticatable" throws me off a bit. > > > > Maybe something like: > > "If `true`,

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-22 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50223/#review143184 --- src/master/flags.cpp (line 237)

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-21 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50223/#review143179 --- src/slave/flags.cpp (lines 791 - 793)

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-21 Thread Zhitao Li
> On July 22, 2016, 2:38 a.m., Adam B wrote: > > src/master/master.cpp, line 380 > > > > > > Rather than EXIT() inside this function, how about having it return a > > `Try` and have those EXIT lines return `Error("

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-21 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50223/ --- (Updated July 22, 2016, 4:38 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-21 Thread Greg Mann
> On July 22, 2016, 2:38 a.m., Adam B wrote: > > src/master/master.cpp, line 386 > > > > > > This now applies to `http_authenticators` and > > `http_framework_authenticators` > > Zhitao Li wrote: > In the

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-21 Thread Zhitao Li
> On July 22, 2016, 2:38 a.m., Adam B wrote: > > src/master/master.cpp, line 386 > > > > > > This now applies to `http_authenticators` and > > `http_framework_authenticators` In the workgroup today, we decided to

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-21 Thread Adam B
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50223/#review143153 --- Looking good. Thanks for the patch! Mostly just nits and some

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-21 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50223/#review143163 --- src/master/main.cpp (lines 278 - 281)

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-21 Thread Zhitao Li
> On July 21, 2016, 8:05 p.m., Greg Mann wrote: > > src/tests/master_quota_tests.cpp, lines 1052-1053 > > > > > > Perhaps we can get away with just setting > > `masterFlags.authenticate_http_readwrite = false`,

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-21 Thread Zhitao Li
> On July 21, 2016, 8:05 p.m., Greg Mann wrote: > > src/slave/slave.cpp, line 841 > > > > > > From the description in this RR it sound like you may have plans along > > these lines already: perhaps we could factor

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-21 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50223/#review143110 --- src/master/main.cpp (lines 278 - 280)

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-21 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50223/#review143099 --- Patch looks great! Reviews applied: [50277, 50223] Passed

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-20 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50223/ --- (Updated July 21, 2016, 4:33 a.m.) Review request for mesos, Adam B, Benjamin

Re: Review Request 50223: Separate AuthN for readonly and readwrite endpoints in Mesos.

2016-07-20 Thread Zhitao Li
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50223/ --- (Updated July 21, 2016, 1:25 a.m.) Review request for mesos, Adam B, Benjamin