Re: Review Request 24059: Replace HttpModule from twitter.common with our own code.

2014-07-29 Thread Kevin Sweeney

---
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.

2014-07-29 Thread Bill Farner


 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.

2014-07-29 Thread Bill Farner

---
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 (updated)
-

  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.

2014-07-29 Thread Maxim Khutornenko

---
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.

2014-07-29 Thread Bill Farner

---
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.

2014-07-29 Thread Bill Farner


 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
 




Re: Review Request 24059: Replace HttpModule from twitter.common with our own code.

2014-07-29 Thread Maxim Khutornenko

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24059/#review49042
---

Ship it!


Ship It!

- Maxim Khutornenko


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
 




Re: Review Request 24059: Replace HttpModule from twitter.common with our own code.

2014-07-29 Thread Bill Farner

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24059/
---

(Updated July 29, 2014, 11:58 p.m.)


Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


Changes
---

Rebased on master.


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 (updated)
-

  build.gradle 22ac38662a19a09374f731c2493ed64740267b79 
  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 
5c0fc2c36ba67e8701288ac5d678eea28aba6528 
  src/test/java/org/apache/aurora/scheduler/http/MnameTest.java 
b7b6a3e32f02b77da6f83ab891ff90b1992faa95 
  src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
0399ec0fe40a7f2057ac59a7fe5fb2583e93ded3 
  src/test/java/org/apache/aurora/scheduler/http/ServletModuleTest.java 
63c504ed1f863cd7294d572766ef4eebd1d5d313 
  src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 
0bcc015977ee475954172c461306844f77eb13af 

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