> On Sept. 17, 2012, 12:49 p.m., Sijie Guo wrote: > > looks good for me. several minor comments. > > > > 1) it would be better to modify bin/hedwig script to let it accept the > > region configuration file. > > 2) it might be better to provide a default config file for the region > > configuration like what we provided in bookkeeper-server/conf for bookie > > server.
Done > On Sept. 17, 2012, 12:49 p.m., Sijie Guo wrote: > > hedwig-server/src/main/java/org/apache/hedwig/server/netty/PubSubServer.java, > > line 97 > > <https://reviews.apache.org/r/7118/diff/1/?file=155320#file155320line97> > > > > a better name might be regionConfiguration to indicate it is a client > > configuration for crossing region. Done. > On Sept. 17, 2012, 12:49 p.m., Sijie Guo wrote: > > hedwig-server/src/main/java/org/apache/hedwig/server/regions/HedwigHubClientFactory.java, > > line 62 > > <https://reviews.apache.org/r/7118/diff/1/?file=155321#file155321line62> > > > > since a separated configuration file for crossing region client is > > introduced, I think we could deprecate the isInterRegionSSLEnabled. > > > > so we don't need to pass server configuration to HedwigHubClientFactory. > > > > for backward compatibility, you need to look into region configuration > > first to see whether ssl enabled. if ssl enabled, we don't need to look > > server conf; if ssl is disabled, we check server conf to see if > > InterRegionSSLEnabled is set or not. if InterRegionSSLEnabled is set, we > > set SSLEnabled to true in region configuration and pass it here. Done - Aniruddha ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/7118/#review11609 ----------------------------------------------------------- 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 > >
