----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7118/#review11660 -----------------------------------------------------------
thanks for updating the patch. most is OK for me, except several minor comments. After that, I think it is OK to be in. hedwig-server/bin/hedwig <https://reviews.apache.org/r/7118/#comment25183> how about 'DEFAULT_REGION_CLIENT_CONF'? hedwig-server/bin/hedwig <https://reviews.apache.org/r/7118/#comment25184> how about 'HEDWIG_REGION_CLIENT_CONF'? hedwig-server/conf/hw_region_mgr_client.conf <https://reviews.apache.org/r/7118/#comment25186> how about 'hw_region_client.conf', which is same the environment variable name I recommended above. hedwig-server/conf/hw_region_mgr_client.conf <https://reviews.apache.org/r/7118/#comment25185> I think you could uncomment the setting here, like bookkeeper's conf. so it would use the default setting in the code. BTW, it would be better to add other existing client settings here. hedwig-server/conf/hwenv.sh <https://reviews.apache.org/r/7118/#comment25187> same as above. - Sijie Guo On Sept. 18, 2012, 4:27 a.m., Aniruddha Laud wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/7118/ > ----------------------------------------------------------- > > (Updated Sept. 18, 2012, 4:27 a.m.) > > > Review request for bookkeeper, Ivan Kelly and Sijie Guo. > > > Description > ------- > > Make the region manager's client configurable. > > https://issues.apache.org/jira/browse/BOOKKEEPER-397 > > > Diffs > ----- > > hedwig-server/bin/hedwig e3b06cc > hedwig-server/conf/hw_region_mgr_client.conf PRE-CREATION > hedwig-server/conf/hwenv.sh a0aeb84 > > hedwig-server/src/main/java/org/apache/hedwig/server/netty/PubSubServer.java > 5036154 > > hedwig-server/src/main/java/org/apache/hedwig/server/regions/HedwigHubClientFactory.java > fa240e2 > > hedwig-server/src/test/java/org/apache/hedwig/server/netty/TestPubSubServer.java > 5d64498 > > Diff: https://reviews.apache.org/r/7118/diff/ > > > Testing > ------- > > > Thanks, > > Aniruddha Laud > >
