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

Ship it!



commons/src/main/java/org/apache/aurora/common/net/http/handlers/AbortHandler.java
 (lines 64 - 65)
<https://reviews.apache.org/r/38332/#comment155525>

    Will this ever appear to the client? Presumably abortListener shuts down 
the http server. quitquitquit appears to get around this by forking a new 
thread for the listener.



commons/src/main/java/org/apache/aurora/common/net/http/handlers/ContentionPrinter.java
 (line 77)
<https://reviews.apache.org/r/38332/#comment155526>

    `String.join`



commons/src/main/java/org/apache/aurora/common/net/http/handlers/QuitHandler.java
 (lines 63 - 64)
<https://reviews.apache.org/r/38332/#comment155527>

    same question - will this actually run?



commons/src/main/java/org/apache/aurora/common/net/http/handlers/TimeSeriesDataSource.java
 (line 98)
<https://reviews.apache.org/r/38332/#comment155530>

    isn't the `@Produces` annotation enough here? afaik we don't need to 
explicitly serialize and deserialize json in jax-rs due to the json provider



commons/src/main/java/org/apache/aurora/common/net/http/handlers/VarsJsonHandler.java
 (line 70)
<https://reviews.apache.org/r/38332/#comment155533>

    same question - do we stilll need to explicitly serialize to json?



commons/src/test/java/org/apache/aurora/common/net/http/handlers/VarsHandlerTest.java
 (line 74)
<https://reviews.apache.org/r/38332/#comment155534>

    String.join



src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java (lines 
199 - 208)
<https://reviews.apache.org/r/38332/#comment155542>

    This feels like a DRY violation to me - consider adding
    
    `public static final String PATH = "/api"`
    
    etc to each resource and referencing them in both the `@Path` annotation 
and here.


- Kevin Sweeney


On Sept. 13, 2015, 11:06 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38332/
> -----------------------------------------------------------
> 
> (Updated Sept. 13, 2015, 11:06 a.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> There are a few positive outcomes here:
> 
> * our HTTP serving code is much more uniform
> * endpoints have less boilerplate code
> * endpoints are easier to test
> * ServletModule setup is simpler and uniform
> 
> 
> Diffs
> -----
> 
>   buildSrc/src/main/groovy/org/apache/aurora/build/CoverageReportCheck.groovy 
> b996d4597492a76db0f2571db4e6c1ae9b4bcf33 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/AbortHandler.java
>  e97bd825752bf04cca391e2119b5e33c66a1cab5 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/AssetHandler.java
>  7a44f07e8514f32637d1832273cb4d9081a14031 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/ContentionPrinter.java
>  1f8c453c8e582da44849a8e922d974a0e054c638 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/HealthHandler.java
>  cc5ad4d5e15eaff78832c033e77b4ac0a532f6b3 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/LogConfig.java
>  5520fb6a83d37884f6cace995f3cc3313c3980c4 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/QuitHandler.java
>  4ce3c971b8e8ca696d7d5ea4e7d348237b836513 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/StringTemplateServlet.java
>  60e0abb7f7bac4c84ece5e007a4b3980fbd9a585 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/TextResponseHandler.java
>  23068eb4b8d9f1d848b4d007e14e7d32f3189ee1 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/ThreadStackPrinter.java
>  5dd88041faafc563c87f51807476aa142e4942d5 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/TimeSeriesDataSource.java
>  e87fe2c2a2d2ed609273298eb57085bcb155c3b2 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/VarsHandler.java
>  bf04525115fa4045e792f2c45116e8d10addb24f 
>   
> commons/src/main/java/org/apache/aurora/common/net/http/handlers/VarsJsonHandler.java
>  e97ec6085c52a155ff9978c5121a5f98a8f93593 
>   
> commons/src/test/java/org/apache/aurora/common/net/http/handlers/AssetHandlerTest.java
>  740c42fedf668233b80f878728620a10ecf9b86f 
>   
> commons/src/test/java/org/apache/aurora/common/net/http/handlers/VarsHandlerTest.java
>  34f62fb9db2d723f8ef171db7c7b485d8416d24f 
>   config/legacy_untested_classes.txt 07fd5f1066a4d926072d91b9de170b9035fb3ea4 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> c503dcceb417d1f73c30dc63cf5352470d4c761d 
>   src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java 
> 738c8b644287b93372f3227832a9d92b95dc498a 
> 
> Diff: https://reviews.apache.org/r/38332/diff/
> 
> 
> Testing
> -------
> 
> manually clicked through everything i could think of in ./gradlew run and in 
> vagrant
> end-to-end tests
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to