----------------------------------------------------------- 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 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 to conditionally configure the `HttpSecurityModule`. src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java (lines 174 - 179) <https://reviews.apache.org/r/42046/#comment174789> Does this need to be here? Can this just be done in the downstream user-defined filter instead? src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java (line 123) <https://reviews.apache.org/r/42046/#comment174795> Do we need to use a `StatsProvider` for this? Can we just expose call counts from this class directly? Are there even threading concerns that require atomicity? src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java (lines 153 - 156) <https://reviews.apache.org/r/42046/#comment174792> Nit, just use `//` for multi-line comments. - Joshua Cohen On Jan. 12, 2016, 1:15 a.m., Amol Deshmukh wrote: > > ----------------------------------------------------------- > 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, Maxim Khutornenko, and Bill Farner. > > > Bugs: AURORA-1576 > https://issues.apache.org/jira/browse/AURORA-1576 > > > Repository: aurora > > > Description > ------- > > Allow for plugging in cli-configurable filters that are invoked post shiro > filters. > > > Diffs > ----- > > > src/main/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityModule.java > 4b6a872737e5934394e9511af7b6bd96c6034e72 > src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java > 19c8a1fe06a333324022da11f74d7c96b2942587 > > src/test/java/org/apache/aurora/scheduler/http/api/security/HttpSecurityIT.java > ac92117e14c105bec74e49d8e80eb56008237abf > > Diff: https://reviews.apache.org/r/42046/diff/ > > > Testing > ------- > > Unit tests: > ``` > $ ./gradlew clean test > ... > BUILD SUCCESSFUL > > Total time: 3 mins 24.616 secs > ``` > > End to end tests: > ``` > $ ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > ... > > *** OK (All tests passed) *** > > aurora-scheduler-kerberos stop/waiting > aurora-scheduler start/running, process 15362 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > ``` > > > Thanks, > > Amol Deshmukh > >
