Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-14 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review114560 --- One more drive-by: in the test I don't believe you need captures.

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-14 Thread Amol Deshmukh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/ --- (Updated Jan. 14, 2016, 1:10 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-14 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review114579 --- Ship it! Master (a80260e) is green with this patch.

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-14 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review114580 --- Ship it! Ship It! - Zameer Manji On Jan. 14, 2016, 1:10 p.m.,

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-14 Thread Amol Deshmukh
> On Jan. 14, 2016, 12:26 p.m., Bill Farner wrote: > > On mobile and can't comment on the specific line, but you should qualify > > the `Filter` binding with a binding annotation. This is best practice for > > any binding that is moderately generic. Ack. - Amol

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-14 Thread Amol Deshmukh
> On Jan. 14, 2016, 12:29 p.m., Bill Farner wrote: > > One more drive-by: in the test I don't believe you need captures. EasyMock > > gives you access to call arguments (EasyMock.getCurrentArguments I believe) > > within the andAnswer. This unfortunately requires you to cast, but I find > >

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-14 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review114557 --- On mobile and can't comment on the specific line, but you should

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-14 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review114589 --- Amol, can you please rebase so I can apply this change? - Zameer

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review114159 --- Ship it! Master (952ef6d) is green with this patch.

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Amol Deshmukh
> On Jan. 13, 2016, 1:31 a.m., Bill Farner wrote: > > It sounds like you ran into an issue that necessitated this patch. Is > > there anything you can add to `docs/security.md` so others don't get stuck > > down the same dead end? > > > > Also, please add a NEWS entry summarizing the new

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Amol Deshmukh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/ --- (Updated Jan. 13, 2016, 7:20 a.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Amol Deshmukh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/ --- (Updated Jan. 12, 2016, 6:52 p.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Amol Deshmukh
> On Jan. 12, 2016, 5:41 p.m., Joshua Cohen wrote: > > src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java, > > line 126 > > > > > > Do we need to use a `StatsProvider` for this? Can we

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Amol Deshmukh
> On Jan. 12, 2016, 5:41 p.m., Joshua Cohen wrote: > > It would've been nice if, by default, we did not wire up the post-filter at > > all, and only asserted its counters when it has been wired in, but from a > > quick glance at the way the tests are set up, it doesn't seem to be > > possible

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

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

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review114060 --- Ship it! Ship It! - Joshua Cohen On Jan. 12, 2016, 6:52 p.m.,

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review114103 --- It sounds like you ran into an issue that necessitated this patch.

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-12 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review114009 --- It would've been nice if, by default, we did not wire up the

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-11 Thread Amol Deshmukh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/ --- (Updated Jan. 12, 2016, 1:15 a.m.) Review request for Aurora, Joshua Cohen,

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review113898 --- Ship it! Master (f064dc1) is green with this patch.

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-11 Thread Amol Deshmukh
> On Jan. 10, 2016, 5:21 a.m., Bill Farner wrote: > > -1, please add test coverage to mitigate likelihood of breaking this change. > > Joshua Cohen wrote: > +1 to the -1 ;). Please add tests! Please let me know if you have suggestions/guidance for adding tests for this change. Given that

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-09 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review113638 --- -1, please add test coverage to mitigate likelihood of breaking

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-09 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review113637 --- Ship it! Ship It! - Maxim Khutornenko On Jan. 9, 2016, 12:55

Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-08 Thread Amol Deshmukh
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/ --- Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.

Re: Review Request 42046: Allow for plugging in cli-configurable filters that are invoked post shiro filters.

2016-01-08 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/42046/#review113591 --- Master (b25ab87) is green with this patch.