aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Kevin Sweeney" <kevi...@apache.org>
Subject Re: Review Request 31754: Break out API servlet configuration into its own module.
Date Thu, 05 Mar 2015 19:00:56 GMT


> On March 4, 2015, 7:18 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/app/AppModule.java, line 141
> > <https://reviews.apache.org/r/31754/diff/1/?file=885268#file885268line141>
> >
> >     There seems to be implementation detail leaking here.  Can you make JettyServerModule
hide this away, and force the leak into the unit test instead?

Any thoughts on how that should look? Possibly an @VisibleForTesting constructor to JettyServerModule
with a boolean production? The test needs to replace the binding for ServletContextListener,
which needs a parent Injector handle, since binding a Module isn't allowed due to guice internals.


- Kevin


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


On March 4, 2015, 5:55 p.m., Kevin Sweeney wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31754/
> -----------------------------------------------------------
> 
> (Updated March 4, 2015, 5:55 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Break out API servlet configuration into its own module.
> 
> This is necessary to make a follow-up patch introducing HTTP Basic auth testable.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 5f6a019e4d6401e1efd075b72c049fa245cc0d0a

>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 8a59d89c07b406ce98076ca7ee51b958599a39ec

>   src/main/java/org/apache/aurora/scheduler/http/SchedulerAPIServlet.java 33ad43b3202e5e9ef5be919b6abc5cbc7f62b660

>   src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java d1ab9b18394ad37fe9dcb131816fcfb2952bf8b6

>   src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 80beb258d9f2786668d29db85b1295163a402d42

>   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 47d54e3c3bb1ba5e0fb26379792f515f25316480

>   src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java 5019094333f9807c64a49c29569ade191ee61824

>   src/test/java/org/apache/aurora/scheduler/http/api/ApiTServletTest.java PRE-CREATION

> 
> Diff: https://reviews.apache.org/r/31754/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>


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