Re: Review Request 31754: Break out API servlet configuration into its own module.

2015-03-05 Thread Kevin Sweeney


 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
 




Review Request 31754: Break out API servlet configuration into its own module.

2015-03-04 Thread Kevin Sweeney

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

Review request for Aurora.


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



Re: Review Request 31754: Break out API servlet configuration into its own module.

2015-03-04 Thread Kevin Sweeney

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


Looks like my IDE was a little overzealous applying a different styleguide - 
updated diff to follow.

- Kevin Sweeney


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