Re: Review Request 24059: Replace HttpModule from twitter.common with our own code.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24059/#review49002 --- Ship it! src/main/java/org/apache/aurora/scheduler/http/AbortCallback.java https://reviews.apache.org/r/24059/#comment85833 Since this isn't in a library anymore you should be fine replacing this with Runtime.getRuntime().halt(0); (better matches the documented semantics of /abortabortabort). src/main/java/org/apache/aurora/scheduler/http/ServletModule.java https://reviews.apache.org/r/24059/#comment85832 TODO to reevaluate the use of Named here. src/main/java/org/apache/aurora/scheduler/http/ServletModule.java https://reviews.apache.org/r/24059/#comment85831 Drop this comment too? src/main/java/org/apache/aurora/scheduler/http/ServletModule.java https://reviews.apache.org/r/24059/#comment85830 TODO to switch this to guava Service? - Kevin Sweeney On July 29, 2014, 11:05 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24059/ --- (Updated July 29, 2014, 11:05 a.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Bugs: AURORA-606 https://issues.apache.org/jira/browse/AURORA-606 Repository: aurora Description --- This is somewhat a copy-paste of the original HttpModule [1], as the first step in a general improvement in how we interact with jetty. I've left a TODO for the next step (simplify addition of static assets). The only expected functional changes are the removal of /healthz (long-since deprecated in favor of /health) and /pprof/* which i have not found to produce useful output in practice (at least, not as useful as other standard JVM tools). I've also removed our dependencies on twitter's jar-packaged jquery and bootstrap, which we no longer appear to use anywhere. [1] https://github.com/twitter/commons/blob/master/src/java/com/twitter/common/application/modules/HttpModule.java Diffs - build.gradle 5919a984ae8d5067f72e6efe50ad590405e779eb config/checkstyle/checkstyle.xml 20709896213b56c0dbdeaf790c826839383df20b config/findbugs/excludeFilter.xml d6c1b1681c2d8505a088f9fb082ce11ac400126f src/main/java/org/apache/aurora/scheduler/app/AppModule.java 30b1ba623daa69a1e184cb91a92e58720648caa2 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java f429eda3bc2c9ae80a67dca8da8574eb7f92976b src/main/java/org/apache/aurora/scheduler/http/AbortCallback.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 729e0ab035b29dc570a2128266112db5312138ed src/test/java/org/apache/aurora/scheduler/http/ServletModuleTest.java 90a001b38ce35fe4da666febde328c1af30f9663 Diff: https://reviews.apache.org/r/24059/diff/ Testing --- ./gradlew clean build -Pq bash examples/vagrant/test_tutorial.sh Manually clicked around in a local scheduler. Thanks, Bill Farner
Re: Review Request 24059: Replace HttpModule from twitter.common with our own code.
On July 29, 2014, 6:18 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/http/AbortCallback.java, line 27 https://reviews.apache.org/r/24059/diff/1/?file=644660#file644660line27 Since this isn't in a library anymore you should be fine replacing this with Runtime.getRuntime().halt(0); (better matches the documented semantics of /abortabortabort). Done. On July 29, 2014, 6:18 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/http/ServletModule.java, line 106 https://reviews.apache.org/r/24059/diff/1/?file=644661#file644661line106 TODO to reevaluate the use of Named here. AbortHandler and QuitHandler inject with @Named, so we're stuck if we want to use them. We could replace them with minimal effort, but that won't buy us anything in terms of dependencies, unfortunately. On July 29, 2014, 6:18 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/http/ServletModule.java, line 148 https://reviews.apache.org/r/24059/diff/1/?file=644661#file644661line148 Drop this comment too? Done. On July 29, 2014, 6:18 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/http/ServletModule.java, line 333 https://reviews.apache.org/r/24059/diff/1/?file=644661#file644661line333 TODO to switch this to guava Service? Done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24059/#review49002 --- On July 29, 2014, 6:05 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24059/ --- (Updated July 29, 2014, 6:05 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Bugs: AURORA-606 https://issues.apache.org/jira/browse/AURORA-606 Repository: aurora Description --- This is somewhat a copy-paste of the original HttpModule [1], as the first step in a general improvement in how we interact with jetty. I've left a TODO for the next step (simplify addition of static assets). The only expected functional changes are the removal of /healthz (long-since deprecated in favor of /health) and /pprof/* which i have not found to produce useful output in practice (at least, not as useful as other standard JVM tools). I've also removed our dependencies on twitter's jar-packaged jquery and bootstrap, which we no longer appear to use anywhere. [1] https://github.com/twitter/commons/blob/master/src/java/com/twitter/common/application/modules/HttpModule.java Diffs - build.gradle 5919a984ae8d5067f72e6efe50ad590405e779eb config/checkstyle/checkstyle.xml 20709896213b56c0dbdeaf790c826839383df20b config/findbugs/excludeFilter.xml d6c1b1681c2d8505a088f9fb082ce11ac400126f src/main/java/org/apache/aurora/scheduler/app/AppModule.java 30b1ba623daa69a1e184cb91a92e58720648caa2 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java f429eda3bc2c9ae80a67dca8da8574eb7f92976b src/main/java/org/apache/aurora/scheduler/http/AbortCallback.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 729e0ab035b29dc570a2128266112db5312138ed src/test/java/org/apache/aurora/scheduler/http/ServletModuleTest.java 90a001b38ce35fe4da666febde328c1af30f9663 Diff: https://reviews.apache.org/r/24059/diff/ Testing --- ./gradlew clean build -Pq bash examples/vagrant/test_tutorial.sh Manually clicked around in a local scheduler. Thanks, Bill Farner
Re: Review Request 24059: Replace HttpModule from twitter.common with our own code.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24059/#review49022 --- src/main/java/org/apache/aurora/scheduler/http/ServletModule.java https://reviews.apache.org/r/24059/#comment85856 I thought you wanted to drop it? - Maxim Khutornenko On July 29, 2014, 7:06 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24059/ --- (Updated July 29, 2014, 7:06 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Bugs: AURORA-606 https://issues.apache.org/jira/browse/AURORA-606 Repository: aurora Description --- This is somewhat a copy-paste of the original HttpModule [1], as the first step in a general improvement in how we interact with jetty. I've left a TODO for the next step (simplify addition of static assets). The only expected functional changes are the removal of /healthz (long-since deprecated in favor of /health) and /pprof/* which i have not found to produce useful output in practice (at least, not as useful as other standard JVM tools). I've also removed our dependencies on twitter's jar-packaged jquery and bootstrap, which we no longer appear to use anywhere. [1] https://github.com/twitter/commons/blob/master/src/java/com/twitter/common/application/modules/HttpModule.java Diffs - build.gradle 5919a984ae8d5067f72e6efe50ad590405e779eb src/main/java/org/apache/aurora/scheduler/app/AppModule.java 30b1ba623daa69a1e184cb91a92e58720648caa2 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java f429eda3bc2c9ae80a67dca8da8574eb7f92976b src/main/java/org/apache/aurora/scheduler/http/AbortCallback.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 729e0ab035b29dc570a2128266112db5312138ed src/test/java/org/apache/aurora/scheduler/http/ServletModuleTest.java 90a001b38ce35fe4da666febde328c1af30f9663 Diff: https://reviews.apache.org/r/24059/diff/ Testing --- ./gradlew clean build -Pq bash examples/vagrant/test_tutorial.sh Manually clicked around in a local scheduler. Thanks, Bill Farner
Re: Review Request 24059: Replace HttpModule from twitter.common with our own code.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24059/ --- (Updated July 29, 2014, 9:44 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Bugs: AURORA-606 https://issues.apache.org/jira/browse/AURORA-606 Repository: aurora Description (updated) --- This is somewhat a copy-paste of the original HttpModule [1], as the first step in a general improvement in how we interact with jetty. I've left a TODO for the next step (simplify addition of static assets). The only expected functional change is the removal of /pprof/*, which i have not found to produce useful output in practice (at least, not as useful as other standard JVM tools). I've also removed our dependencies on twitter's jar-packaged jquery and bootstrap, which we no longer appear to use anywhere. [1] https://github.com/twitter/commons/blob/master/src/java/com/twitter/common/application/modules/HttpModule.java Diffs - build.gradle 5919a984ae8d5067f72e6efe50ad590405e779eb src/main/java/org/apache/aurora/scheduler/app/AppModule.java 30b1ba623daa69a1e184cb91a92e58720648caa2 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java f429eda3bc2c9ae80a67dca8da8574eb7f92976b src/main/java/org/apache/aurora/scheduler/http/AbortCallback.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 729e0ab035b29dc570a2128266112db5312138ed src/test/java/org/apache/aurora/scheduler/http/ServletModuleTest.java 90a001b38ce35fe4da666febde328c1af30f9663 Diff: https://reviews.apache.org/r/24059/diff/ Testing --- ./gradlew clean build -Pq bash examples/vagrant/test_tutorial.sh Manually clicked around in a local scheduler. Thanks, Bill Farner
Re: Review Request 24059: Replace HttpModule from twitter.common with our own code.
On July 29, 2014, 8:58 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/http/ServletModule.java, line 124 https://reviews.apache.org/r/24059/diff/2/?file=644918#file644918line124 I thought you wanted to drop it? Hah whoops, that happened in a different branch. Updated the review description to match the diff. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24059/#review49022 --- On July 29, 2014, 9:44 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24059/ --- (Updated July 29, 2014, 9:44 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Bugs: AURORA-606 https://issues.apache.org/jira/browse/AURORA-606 Repository: aurora Description --- This is somewhat a copy-paste of the original HttpModule [1], as the first step in a general improvement in how we interact with jetty. I've left a TODO for the next step (simplify addition of static assets). The only expected functional change is the removal of /pprof/*, which i have not found to produce useful output in practice (at least, not as useful as other standard JVM tools). I've also removed our dependencies on twitter's jar-packaged jquery and bootstrap, which we no longer appear to use anywhere. [1] https://github.com/twitter/commons/blob/master/src/java/com/twitter/common/application/modules/HttpModule.java Diffs - build.gradle 5919a984ae8d5067f72e6efe50ad590405e779eb src/main/java/org/apache/aurora/scheduler/app/AppModule.java 30b1ba623daa69a1e184cb91a92e58720648caa2 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java f429eda3bc2c9ae80a67dca8da8574eb7f92976b src/main/java/org/apache/aurora/scheduler/http/AbortCallback.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/ServletModule.java 729e0ab035b29dc570a2128266112db5312138ed src/test/java/org/apache/aurora/scheduler/http/ServletModuleTest.java 90a001b38ce35fe4da666febde328c1af30f9663 Diff: https://reviews.apache.org/r/24059/diff/ Testing --- ./gradlew clean build -Pq bash examples/vagrant/test_tutorial.sh Manually clicked around in a local scheduler. Thanks, Bill Farner
