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


Drive-by


src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java (line 148)
<https://reviews.apache.org/r/41453/#comment170750>

    It looks like this can be an NPE without advanced knowledge of how the base 
class is used in practice, complicated by the fact the field is set inside a 
try - seems the setting could be done outside the try, but ...



src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java (line 156)
<https://reviews.apache.org/r/41453/#comment170749>

    The Optional indirection (+ volatile bit) seems a bit odd - the 
startupService is never absent.  How about just keep the variable local and 
non-Optional and on the next line add the stopAsync as a tear down?


- John Sirois


On Dec. 16, 2015, 11:19 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41453/
> -----------------------------------------------------------
> 
> (Updated Dec. 16, 2015, 11:19 a.m.)
> 
> 
> Review request for Aurora and Joshua Cohen.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> HTTP server cleanup - shut down the server after unit tests, remove jetty bug 
> workaround.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 838bfc9ed6242e777d81a17b337f342b7bea72ec 
>   src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java 
> 5768481005ef505d6c397101d8cc005af9d6815f 
> 
> Diff: https://reviews.apache.org/r/41453/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to