Re: Review Request 39520: Quota: Added authentication, authorization tests.

2015-12-18 Thread Jan Schlicht
> On Dec. 14, 2015, 5:18 p.m., Alexander Rukletsov wrote: > > I would suggest to add some more test cases: > > - Request does not contain a principal (in absence of authz quota can be > > set); > > - Request does not contain a principal, ACLs allow `ANY` to set quota for > > role "prod"

Re: Review Request 39520: Quota: Added authentication, authorization tests.

2015-12-18 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39520/ --- (Updated Dec. 18, 2015, 10:12 a.m.) Review request for mesos, Alexander

Re: Review Request 39520: Quota: Added authentication, authorization tests.

2015-12-18 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39520/ --- (Updated Dec. 18, 2015, 1:18 p.m.) Review request for mesos, Alexander

Re: Review Request 39520: Quota: Added authentication, authorization tests.

2015-12-18 Thread Jan Schlicht
> On Dec. 18, 2015, 12:32 p.m., Alexander Rukletsov wrote: > > I would like to see one more test, where both authn and authz are disabled > > and quota set request succeeds. While having this test makes sense, it would test neither authentication nor authorization, but the actual quota

Re: Review Request 39520: Quota: Added authentication, authorization tests.

2015-12-18 Thread Alexander Rukletsov
> On Dec. 14, 2015, 4:18 p.m., Alexander Rukletsov wrote: > > I would suggest to add some more test cases: > > - Request does not contain a principal (in absence of authz quota can be > > set); > > - Request does not contain a principal, ACLs allow `ANY` to set quota for > > role "prod"

Re: Review Request 39520: Quota: Added authentication, authorization tests.

2015-12-18 Thread Alexander Rukletsov
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39520/#review66 --- I would like to see one more test, where both authn and authz are

Re: Review Request 39520: Quota: Added authentication, authorization tests.

2015-12-18 Thread Jan Schlicht
> On Dec. 18, 2015, 12:32 p.m., Alexander Rukletsov wrote: > > src/tests/master_quota_tests.cpp, line 1070 > > > > > > quota set request; > > backtick `ROLE1` > > s/ that can be authorized././ > > >

Re: Review Request 39520: Quota: Added authentication, authorization tests.

2015-12-18 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39520/ --- (Updated Dec. 18, 2015, 1:27 p.m.) Review request for mesos, Alexander

Re: Review Request 39520: Quota: Added authentication, authorization tests.

2015-12-15 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39520/ --- (Updated Dec. 15, 2015, 11:13 a.m.) Review request for mesos, Alexander

Re: Review Request 39520: Quota: Added authentication, authorization tests.

2015-12-15 Thread Jan Schlicht
> On Dec. 14, 2015, 4:52 p.m., Till Toenshoff wrote: > > src/tests/master_quota_tests.cpp, lines 78-79 > > > > > > Comma, really? 8] Hm, even an Oxford comma doesn't count here :D - Jan

Re: Review Request 39520: Quota: Added authentication, authorization tests.

2015-12-15 Thread Jan Schlicht
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39520/ --- (Updated Dec. 15, 2015, 12:09 p.m.) Review request for mesos, Alexander

Re: Review Request 39520: Quota: Added authentication, authorization tests.

2015-12-15 Thread Alexander Rukletsov
> On Dec. 14, 2015, 4:18 p.m., Alexander Rukletsov wrote: > > src/tests/master_quota_tests.cpp, line 1002 > > > > > > This could fail if we ever enforce `MIN_MEM` check. How about > > "cpus:1;mem:512", which we

Re: Review Request 39520: Quota: Added authentication, authorization tests.

2015-12-15 Thread Jan Schlicht
> On Dec. 14, 2015, 11:16 p.m., Greg Mann wrote: > > src/tests/master_quota_tests.cpp, line 1061 > > > > > > In addition to verifying the OK response, is it possible to verify by > > another means that the quota

Re: Review Request 39520: Quota: Added authentication, authorization tests.

2015-12-15 Thread Mesos ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/39520/#review110495 --- Patch looks great! Reviews applied: [39211, 39018, 39102, 36913,