reef-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Julia (JIRA)" <j...@apache.org>
Subject [jira] [Updated] (REEF-1347) Properly Inject Network Service in Java
Date Wed, 20 Apr 2016 22:21:25 GMT

     [ https://issues.apache.org/jira/browse/REEF-1347?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]

Julia updated REEF-1347:
------------------------
    Description: 
Currently in GroupCommDriverImpl, NetworkService  is injected using a new Injector with lot
of bindVolatileParameter(). A correct way is to 

* use existing injector in driver to inject the NetworkService, ideally put it in the constructor
of GroupCommDriverImpl so that to just simply get it injected when GroupCommDriverImpl is
injected. 
* For parameters in NetworkService constructor, they should be set in the Configuration before
GroupCommDriverImpl is injected. 

Looking at NetworkService constructor, there are also some issues:
* Codec<T> codec should be not defines as a NamedParameter. It should be able to be
injected directly.
* Similarly, TransportFactory doesn't need to be a NamedParameter. We just need to bind a
injectable implementation to the interface. 
* Same for IdentifierFactory and EventHandler<Message<T>>, it doesn'tneed to be
a named parameter. 

NameResolver also has issue. We should just put NameResolver in GroupCommDriverImpl constructor
and bind the nameResolverConf before GroupCommDriverImpl  is injected. Most likely, those
should be part of the driver configuration. 

Once above issues are fixed, NetworkServiceTest also need to be updated:
* Set configuration and inject NetworkService in a proper way.
* If additional configuration is added for a forkInjector, it should be bound to a new configuration
and use that configuration when calling forkInjector(). 
* Binding with bindVolatileInstance is not recommended unless we have to reuse an existing
object.



  was:
Currently in GroupCommDriverImpl, NetworkService  is injected using a new Injector with lot
of bindVolatileParameter(). A correct way is to 

use existing injector in driver to inject the NetworkService, ideally put it in the constructor
of GroupCommDriverImpl so that to just simply get it injected when GroupCommDriverImpl is
injected. 
For parameters in NetworkService constructor, they should be set in the Configuration before
GroupCommDriverImpl is injected. 

Looking at NetworkService constructor, there are also some issues:
Codec<T> codec should be not defines as a NamedParameter. It should be able to be injected
directly.
Similarly, TransportFactory doesn't need to be a NamedParameter. We just need to bind a injectable
implementation to the interface. 
Same for IdentifierFactory and EventHandler<Message<T>>, it doesn'tneed to be
a named parameter. 

NameResolver also has issue. We should just put NameResolver in GroupCommDriverImpl constructor
and bind the nameResolverConf before GroupCommDriverImpl  is injected. Most likely, those
should be part of the driver configuration. 

Once above issues are fixed, NetworkServiceTest also need to be updated:
* Set configuration and inject NetworkService in a proper way.
* If additional configuration is added for a forkInjector, it should be bound to a new configuration
and use that configuration when calling forkInjector(). 
* Binding with bindVolatileInstance is not recommended unless we have to reuse an existing
object.




> Properly Inject Network Service in Java
> ---------------------------------------
>
>                 Key: REEF-1347
>                 URL: https://issues.apache.org/jira/browse/REEF-1347
>             Project: REEF
>          Issue Type: Improvement
>            Reporter: Julia
>
> Currently in GroupCommDriverImpl, NetworkService  is injected using a new Injector with
lot of bindVolatileParameter(). A correct way is to 
> * use existing injector in driver to inject the NetworkService, ideally put it in the
constructor of GroupCommDriverImpl so that to just simply get it injected when GroupCommDriverImpl
is injected. 
> * For parameters in NetworkService constructor, they should be set in the Configuration
before GroupCommDriverImpl is injected. 
> Looking at NetworkService constructor, there are also some issues:
> * Codec<T> codec should be not defines as a NamedParameter. It should be able to
be injected directly.
> * Similarly, TransportFactory doesn't need to be a NamedParameter. We just need to bind
a injectable implementation to the interface. 
> * Same for IdentifierFactory and EventHandler<Message<T>>, it doesn'tneed
to be a named parameter. 
> NameResolver also has issue. We should just put NameResolver in GroupCommDriverImpl constructor
and bind the nameResolverConf before GroupCommDriverImpl  is injected. Most likely, those
should be part of the driver configuration. 
> Once above issues are fixed, NetworkServiceTest also need to be updated:
> * Set configuration and inject NetworkService in a proper way.
> * If additional configuration is added for a forkInjector, it should be bound to a new
configuration and use that configuration when calling forkInjector(). 
> * Binding with bindVolatileInstance is not recommended unless we have to reuse an existing
object.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Mime
View raw message