> On April 3, 2015, 5:15 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/http/HttpService.java, line 21
> > <https://reviews.apache.org/r/32806/diff/2/?file=914532#file914532line21>
> >
> >     This seems like it's a Qualifier for HostAndPort rather than an 
> > interface. As an interface this appears to fall into the pseudo-typedef 
> > antipattern: http://www.ibm.com/developerworks/library/j-jtp02216/.
> 
> Bill Farner wrote:
>     I could drop this for `Provider<HostAndPort>` to a similar end, but i 
> find back-tracking to concrete implementations significantly harder when 
> bindings are made against vague interfaces.  Can you see a path forward that 
> doesn't make it difficult to map from consumer to implementer?
> 
> Kevin Sweeney wrote:
>     I don't find the path for Qualifier implementaitons hard to find
>     
>     Injection sites will typically have
>     
>     ```java
>     @Inject
>     InjectionSite(@Http HostAndPort httpHostAndPort) {
>     }
>     ```
>     
>     Find Usages on Http will lead to a section "usages in .class" which will 
> lead to something like
>     
>     ```java
>     class ConfigModule extends AbstractModule {
>        @Override
>        protected void configure() {
>          
> bind(HostAndPort.class).annotatedWith(Http.class).toInstance(HostAndPort.createFrom("127.0.0.1:80"));
>        }
>     }
>     ```
>     
>     or a section Annotation that will lead to
>     
>     ```java
>     class ConfigModule extends AbstractModule {
>       @Override protected void configure() {}
>     
>       @Provides
>       @Http
>       HostAndPort provideHttpHostAndPort() {
>         //...
>       }
>     }
>     ```
>     
>     and the declaration of the qualifier itself has information about what 
> makes the thing special in its javadoc
>     
>     ```java
>     /**
>      * The main HTTP port.
>      */
>     @Qualifier
>     @Retention(RUNTIME)
>     @Target({FIELD, METHOD, PARAMETER})
>     @Documented
>     public @interface Http {}
>     ```

Right, i'm familiar with the moving parts involved.  One detail not addressed 
in your comment above is that this must be a `Provider`, since the port number 
is not known during binding (it could be an ephemeral port, and `== 0` at 
binding time).  IMHO this is the point where implementing `Provider` in 
`HttpServerLauncher` and introducing a binding annotation seemed worse than the 
custom interface.


- Bill


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


On April 3, 2015, 3:06 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32806/
> -----------------------------------------------------------
> 
> (Updated April 3, 2015, 3:06 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Kevin Sweeney.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Remove use of LocalServiceRegistry, simplify plumbing of HTTP address.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 5f6a019e4d6401e1efd075b72c049fa245cc0d0a 
>   
> src/main/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverrides.java
>  6d92ae3c8ec46e7964e81e507a2f2a7f2db68cfd 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 2af009d3d9ec44a70659225d0c18de9fda3a6f7a 
>   src/main/java/org/apache/aurora/scheduler/http/HttpService.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 50f377587ac05dbb72063ea02502e6d980148d3e 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java 
> e03009c12de5a09761c1f444c6439ef3144b0b17 
>   
> src/test/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverridesTest.java
>  21fd027976f75acc427c6d9265a7c7a91895d53d 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
> c3e40d88fe7ee1a447d1d61980b69bd1b46881e7 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java 
> 7f80757cb40af7dde042f1d39355eadf2b3b1aee 
>   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
> c5c5f789de6bf7693520081d0c1acc5165603242 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
>  76cb691e6d7d4fada3a18fde73aceed7039bcaa4 
> 
> Diff: https://reviews.apache.org/r/32806/diff/
> 
> 
> Testing
> -------
> 
> Test suite + end-to-end tests.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to