Re: Review Request 45042: Add ACL support for announcer

2016-03-30 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review126191 --- Ship it! Thanks for sticking it out through the review, nice

Re: Review Request 45042: Add ACL support for announcer

2016-03-30 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review126190 --- Ship it! Master (bc4649e) is green with this patch.

Re: Review Request 45042: Add ACL support for announcer

2016-03-30 Thread Kunal Thakar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/ --- (Updated March 30, 2016, 7:50 p.m.) Review request for Aurora, Bill Farner and

Re: Review Request 45042: Add ACL support for announcer

2016-03-30 Thread Kunal Thakar
> On March 28, 2016, 11:46 p.m., Bill Farner wrote: > > examples/vagrant/announcer-auth.json, line 11 > > > > > > I now have to backpedal on my advice to store the encrypted credentials > > here. Since our hand is

Re: Review Request 45042: Add ACL support for announcer

2016-03-30 Thread Kunal Thakar
> On March 30, 2016, 2:20 a.m., Bill Farner wrote: > > RELEASE-NOTES.md, line 14 > > > > > > s/Support/Added support/ > > s/ZK/ZooKeeper/ Done > On March 30, 2016, 2:20 a.m., Bill Farner wrote: > >

Re: Review Request 45042: Add ACL support for announcer

2016-03-29 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review126007 --- Went through docs with a fine-toothed comb. Only ship blocker

Re: Review Request 45042: Add ACL support for announcer

2016-03-29 Thread Bill Farner
> On March 28, 2016, 4:46 p.m., Bill Farner wrote: > > examples/vagrant/announcer-auth.json, line 11 > > > > > > I now have to backpedal on my advice to store the encrypted credentials > > here. Since our hand is

Re: Review Request 45042: Add ACL support for announcer

2016-03-29 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review125991 --- This patch does not apply cleanly against master (ec29ac1), do

Re: Review Request 45042: Add ACL support for announcer

2016-03-29 Thread Kunal Thakar
> On March 28, 2016, 11:46 p.m., Bill Farner wrote: > > examples/vagrant/announcer-auth.json, line 11 > > > > > > I now have to backpedal on my advice to store the encrypted credentials > > here. Since our hand is

Re: Review Request 45042: Add ACL support for announcer

2016-03-29 Thread Kunal Thakar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/ --- (Updated March 30, 2016, 12:17 a.m.) Review request for Aurora, Bill Farner

Re: Review Request 45042: Add ACL support for announcer

2016-03-28 Thread Bill Farner
> On March 22, 2016, 7:23 p.m., Bill Farner wrote: > > src/main/python/apache/aurora/executor/common/announcer.py, line 84 > > > > > > I think we should also require that at least one field is specified in > >

Re: Review Request 45042: Add ACL support for announcer

2016-03-28 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review125781 --- examples/vagrant/announcer-auth.json (line 8)

Re: Review Request 45042: Add ACL support for announcer

2016-03-28 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review125778 --- RELEASE-NOTES.md (line 15)

Re: Review Request 45042: Add ACL support for announcer

2016-03-28 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review125773 --- Great to see that you got it working in end-to-end tests! A few

Re: Review Request 45042: Add ACL support for announcer

2016-03-28 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review125775 --- Ship it! Master (f28f41a) is green with this patch.

Re: Review Request 45042: Add ACL support for announcer

2016-03-28 Thread Kunal Thakar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/ --- (Updated March 28, 2016, 11:10 p.m.) Review request for Aurora, Bill Farner

Re: Review Request 45042: Add ACL support for announcer

2016-03-28 Thread Bill Farner
> On March 22, 2016, 7:23 p.m., Bill Farner wrote: > > src/main/python/apache/aurora/executor/common/announcer.py, line 84 > > > > > > I think we should also require that at least one field is specified in > >

Re: Review Request 45042: Add ACL support for announcer

2016-03-28 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review125769 --- This patch does not apply cleanly against master (f28f41a), do

Re: Review Request 45042: Add ACL support for announcer

2016-03-28 Thread Kunal Thakar
> On March 23, 2016, 2:23 a.m., Bill Farner wrote: > > src/main/python/apache/aurora/executor/bin/thermos_executor_main.py, line > > 107 > > > > > > Sorry for not catching this earlier - how about s/auth/acl/? I

Re: Review Request 45042: Add ACL support for announcer

2016-03-28 Thread Kunal Thakar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/ --- (Updated March 28, 2016, 10:51 p.m.) Review request for Aurora, Bill Farner

Re: Review Request 45042: Add ACL support for announcer

2016-03-22 Thread Bill Farner
> On March 22, 2016, 7:23 p.m., Bill Farner wrote: > > docs/security.md, line 289 > > > > > > I'd like to propose several changes to this section, which i've made in > > the rewritten block below. > > > >

Re: Review Request 45042: Add ACL support for announcer

2016-03-22 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review124935 --- Thanks for the changes, this is looking great! My most pressing

Re: Review Request 45042: Add ACL support for announcer

2016-03-22 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review124871 --- Ship it! Master (d5d7ec0) is green with this patch.

Re: Review Request 45042: Add ACL support for announcer

2016-03-22 Thread Kunal Thakar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/ --- (Updated March 22, 2016, 6:51 p.m.) Review request for Aurora, Bill Farner and

Re: Review Request 45042: Add ACL support for announcer

2016-03-22 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review124841 --- Master (335cf88) is red with this patch.

Re: Review Request 45042: Add ACL support for announcer

2016-03-22 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review124839 --- Ship it! This change LGTM. I was a bit confused if

Re: Review Request 45042: Add ACL support for announcer

2016-03-22 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review124833 --- Drive-by review: Please also add an entry to the

Re: Review Request 45042: Add ACL support for announcer

2016-03-21 Thread Zameer Manji
> On March 19, 2016, 6:46 p.m., Bill Farner wrote: > > src/main/python/apache/aurora/executor/common/announcer.py, line 153 > > > > > > -1 on the approach of logging and moving forward without auth when > > errors

Re: Review Request 45042: Add ACL support for announcer

2016-03-21 Thread Zameer Manji
> On March 19, 2016, 6:46 p.m., Bill Farner wrote: > > src/main/python/apache/aurora/executor/common/announcer.py, line 128 > > > > > > please refactor to make the constructor accept the ACL list, making the > >

Re: Review Request 45042: Add ACL support for announcer

2016-03-20 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review124255 --- Ship it! Master (12be6fb) is green with this patch.

Re: Review Request 45042: Add ACL support for announcer

2016-03-19 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review124431 --- Overall looks in decent shape, made some suggestions. Before

Re: Review Request 45042: Add ACL support for announcer

2016-03-19 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/#review124334 --- Master (b24619b) is red with this patch.

Re: Review Request 45042: Add ACL support for announcer

2016-03-18 Thread Kunal Thakar
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45042/ --- (Updated March 18, 2016, 9:57 p.m.) Review request for Aurora, Bill Farner and