On Jan 28, 2013, at 01:11 , kishore g wrote: > Hi, > > I started working on merging the changes from S4-110 to dev. There are > quite a few changes merged into the dev branch. I am trying to keep the > patch simple and trying to use the guice way of plugging things in where > ever possible. Few doubts > > ============ > https://git-wip-us.apache.org/repos/asf?p=incubator-s4.git;a=blob;f=subprojects/s4-comm/src/main/java/org/apache/s4/comm/DefaultCommModule.java;hb=eb851534 > > This line appears twice > bind(RemoteEmitters.class).to(DefaultRemoteEmitters.class).in(Scopes.SINGLETON);
Yes that's an error, maybe from the merge, and it should be fixed. Interestingly it does not seem to break things though. > > I also saw that, earlier we used to bing Listener in the configure method. > I see that its removed now, when was this removed and why? Good observation. The reason is that we need to bind the listener at the application level, i.e. once we have application classes available, so that deserializer is loaded with the correct classloader, so that application specific events can be correctly deserialized. Note that you are also pointing close to another pending issue, which is that the listener implementation is currently hardcoded. We could fix that by allowing overriding implementations in the app config. I'll add tickets to track these issues. > ============== > > App deployment seems to happen in deploymentmanager and s4bootstrap, which > one is used. Code seems to be duplicated between the two. Should we move > common methods to DeploymentUtils. The idea is to allow a fastpath for testing, in which application code/modules are already available in the classpath and don't need to be downloaded. The standard path is to go through the Server class and DeploymentManager, and download whatever modules and application classes are needed. Apart from classloading, things are expected to work the same through both execution paths. > > Hard to understand how s4node, s4bootstrap, server, deployment manager are > dependent on each other. Yes, there are some improvements we could make there, through some (re)factoring and cleanup. > > Need help here. Since the merge of S4-110 to dev was almost impossible, I > am hand picking the changes into S4-110-new branch. I will try to commit > the changes to the branch once i get something working. OK, keep us posted, Thanks! Matthieu
