Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-02 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/ --- (Updated Dec. 2, 2015, 10:38 p.m.) Review request for mesos, Adam B, Jie Yu, Mi

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-02 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/ --- (Updated Dec. 2, 2015, 7:52 p.m.) Review request for mesos, Adam B, Jie Yu, Mic

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-02 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/#review108697 --- src/master/master.cpp (lines 2804 - 2805)

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-02 Thread Jie Yu
> On Dec. 2, 2015, 4:02 p.m., Alexander Rukletsov wrote: > > src/master/master.cpp, lines 2769-2770 > > > > > > Is my understanding is correct, we do not allow reserving resources > > neither for operators nor for

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-02 Thread Greg Mann
> On Dec. 2, 2015, 4:02 p.m., Alexander Rukletsov wrote: > > src/master/master.cpp, lines 2769-2770 > > > > > > Is my understanding is correct, we do not allow reserving resources > > neither for operators nor for

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-02 Thread Greg Mann
> On Dec. 2, 2015, 2:17 p.m., Alexander Rukletsov wrote: > > src/master/master.cpp, line 2775 > > > > > > I believe you inherited the TODO from MPark. Do you want to preserve > > his name or change to yours? I wa

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-02 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/ --- (Updated Dec. 2, 2015, 6 p.m.) Review request for mesos, Adam B, Jie Yu, Michae

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-02 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/#review108655 --- src/master/master.cpp (lines 2769 - 2770)

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-02 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/#review108638 --- A drive-by review, I'm looking into authz and ACLs and learning fro

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-02 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/ --- (Updated Dec. 2, 2015, 9 a.m.) Review request for mesos, Adam B, Jie Yu, Michae

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-01 Thread Jie Yu
> On Dec. 1, 2015, 2:24 p.m., Michael Park wrote: > > src/master/master.cpp, line 2771 > > > > > > Why is it that we need to perform validation within authorization? We > > perform `authorizeTask` with a `TaskInfo`

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-01 Thread Greg Mann
> On Dec. 1, 2015, 2:24 p.m., Michael Park wrote: > > src/master/master.cpp, line 2771 > > > > > > Why is it that we need to perform validation within authorization? We > > perform `authorizeTask` with a `TaskInfo`

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-01 Thread Jie Yu
> On Dec. 1, 2015, 2:24 p.m., Michael Park wrote: > > src/master/master.cpp, line 2771 > > > > > > Why is it that we need to perform validation within authorization? We > > perform `authorizeTask` with a `TaskInfo`

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-01 Thread Greg Mann
> On Dec. 1, 2015, 2:24 p.m., Michael Park wrote: > > src/master/master.cpp, line 2771 > > > > > > Why is it that we need to perform validation within authorization? We > > perform `authorizeTask` with a `TaskInfo`

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-01 Thread Greg Mann
> On Dec. 1, 2015, 2:24 p.m., Michael Park wrote: > > src/master/master.cpp, line 2771 > > > > > > Why is it that we need to perform validation within authorization? We > > perform `authorizeTask` with a `TaskInfo`

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-01 Thread Jie Yu
> On Dec. 1, 2015, 2:24 p.m., Michael Park wrote: > > src/master/master.cpp, line 2771 > > > > > > Why is it that we need to perform validation within authorization? We > > perform `authorizeTask` with a `TaskInfo`

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-01 Thread Greg Mann
> On Dec. 1, 2015, 2:24 p.m., Michael Park wrote: > > src/master/master.cpp, line 2771 > > > > > > Why is it that we need to perform validation within authorization? We > > perform `authorizeTask` with a `TaskInfo`

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-12-01 Thread Michael Park
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/#review108496 --- src/master/master.hpp (line 706)

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-11-30 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/#review108395 --- src/master/master.cpp (lines 2776 - 2778)

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-11-30 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/ --- (Updated Nov. 30, 2015, 8:35 p.m.) Review request for mesos, Adam B, Jie Yu, Mi

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-11-19 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/#review107278 --- Ship it! src/master/master.cpp (line 2792)

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-11-16 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/ --- (Updated Nov. 16, 2015, 11:48 p.m.) Review request for mesos, Adam B, Jie Yu, M

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-11-13 Thread Greg Mann
> On Nov. 13, 2015, 11:11 p.m., Jie Yu wrote: > > src/master/master.cpp, lines 2737-2738 > > > > > > This temp variable does not seem to be necessary. Can you just do: > > > > ``` > > LOG(INFO) << "Auth

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-11-13 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/ --- (Updated Nov. 13, 2015, 11:32 p.m.) Review request for mesos, Adam B, Jie Yu, M

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-11-13 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/ --- (Updated Nov. 13, 2015, 11:15 p.m.) Review request for mesos, Adam B, Jie Yu, M

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-11-13 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/#review106518 --- src/master/master.cpp (lines 2737 - 2738)

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-11-12 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/ --- (Updated Nov. 13, 2015, 4:30 a.m.) Review request for mesos, Adam B, Jie Yu, Mi

Re: Review Request 39987: [3/5] Added 'Master::authorize(Un)reserveResources()' for Reserve/Unreserve.

2015-11-09 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39987/ --- (Updated Nov. 9, 2015, 11:42 p.m.) Review request for mesos, Adam B, Jie Yu, Mi