Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-18 Thread Greg Mann
> On Dec. 16, 2015, 2:38 p.m., Alexander Rukletsov wrote: > > src/tests/persistent_volume_tests.cpp, line 790 > > > > > > Do you think it makes sense to extract "role1" into a constant? Since this role name appears

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-18 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40255/#review58 --- src/master/master.cpp (lines 3346 - 3350)

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-18 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40255/ --- (Updated Dec. 18, 2015, 10:10 a.m.) Review request for mesos, Jie Yu, Michael

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-18 Thread Greg Mann
> On Dec. 18, 2015, 9:58 a.m., Alexander Rukletsov wrote: > > src/master/master.cpp, lines 3346-3350 > > > > > > How about a TODO, that we may want to retry instead of giving up > > straight away? Yea, sounds

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-18 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40255/ --- (Updated Dec. 18, 2015, 9:33 a.m.) Review request for mesos, Jie Yu, Michael

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-18 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40255/ --- (Updated Dec. 18, 2015, 9:28 a.m.) Review request for mesos, Jie Yu, Michael

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-18 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40255/ --- (Updated Dec. 18, 2015, 9:37 a.m.) Review request for mesos, Jie Yu, Michael

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-18 Thread Alexander Rukletsov
> On Dec. 16, 2015, 2:38 p.m., Alexander Rukletsov wrote: > > src/master/master.cpp, line 3348 > > > > > > You are following the pattern here, but are we sure that the framework > > has the principal? I also do

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-18 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40255/#review63 --- src/tests/persistent_volume_tests.cpp (lines 887 - 890)

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-18 Thread Jie Yu
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40255/#review111233 --- Ship it! src/master/master.cpp (line 3425)

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-18 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40255/ --- (Updated Dec. 18, 2015, 11:12 p.m.) Review request for mesos, Jie Yu, Michael

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-18 Thread Greg Mann
> On Dec. 18, 2015, 7:59 p.m., Jie Yu wrote: > > Thanks for the reviews, Jie! > On Dec. 18, 2015, 7:59 p.m., Jie Yu wrote: > > src/tests/persistent_volume_tests.cpp, line 751 > > > > > > If you pause the clock,

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-18 Thread Greg Mann
> On Dec. 18, 2015, 10:48 a.m., Alexander Rukletsov wrote: > > src/tests/persistent_volume_tests.cpp, lines 888-891 > > > > > > It looks like this rule is not used in this test. Could you please > > explain why

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-16 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40255/#review110658 --- src/master/master.cpp (lines 3141 - 3142)

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-16 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40255/#review110664 --- And what about master endpoints (authz for operators)? I can't

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-16 Thread Greg Mann
> On Dec. 16, 2015, 3:24 p.m., Alexander Rukletsov wrote: > > And what about master endpoints (authz for operators)? I can't find that > > code in the chain of reviews. Will it be done separately? Yep, there is a separate ticket for that: https://issues.apache.org/jira/browse/MESOS-3903 -

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-12-08 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40255/ --- (Updated Dec. 8, 2015, 7:03 p.m.) Review request for mesos, Jie Yu, Michael

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-11-13 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40255/ --- (Updated Nov. 14, 2015, 12:25 a.m.) Review request for mesos, Jie Yu, Michael

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-11-12 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40255/ --- (Updated Nov. 13, 2015, 1:21 a.m.) Review request for mesos, Jie Yu, Michael

Re: Review Request 40255: [5/7] Added framework authorization for persistent volumes.

2015-11-12 Thread Greg Mann
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40255/ --- (Updated Nov. 13, 2015, 1:21 a.m.) Review request for mesos, Jie Yu, Michael