aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Farner" <wfar...@apache.org>
Subject Re: Review Request 24059: Replace HttpModule from twitter.common with our own code.
Date Tue, 29 Jul 2014 19:04:12 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message