> On Aug. 7, 2014, 7:58 a.m., Alexander Shraer wrote: > > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java, > > line 478 > > <https://reviews.apache.org/r/24208/diff/1-4/?file=649202#file649202line478> > > > > remove !key.equals("version") from here - the user shouldn't specify a > > version in the dynamic file. > > > > I expect this to cause some tests to fail, such as > > ReconfigRecoveryTest.java Instead of having a version in the config spec > > tests could pass the version separately as a parameter, for example to > > MainThread constructor. > > Hongchao Deng wrote: > Hi Alex. The version is loaded to dynamic properties which is parsed into > this function. The user doesn't specify a version in dynamic file but it > would still have it. I think it must have, right? > > Regarding the MainThread constructor, I have done something to handle the > case where version is added to quorumCfg: I parsed the quorumCfg to verify > that if there is any version in it and separated it out. > > Alexander Shraer wrote: > I see. We should still check that the user doesn't specify a version and > crash otherwise. Neither the static file nor the dynamic file should contain > a version (I think this means two checks). > > The thing with the test is that people will look at it as an example of > how to use the functionality. So I don't think we should be specifying a > version as part of the config if we don't want the users to do the same.
Good suggestion. We should change the test to reflect appending the version to the file. Right? I was thinking to 1. have a version parameter or 2. give a dynamic file name explicitly otherwise no dynamic file will be created. - Hongchao ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24208/#review49876 ----------------------------------------------------------- On Aug. 7, 2014, 5:02 p.m., Hongchao Deng wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24208/ > ----------------------------------------------------------- > > (Updated Aug. 7, 2014, 5:02 p.m.) > > > Review request for zookeeper. > > > Repository: zookeeper-git > > > Description > ------- > > ZOOKEEPER-1994 > > > Diffs > ----- > > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeer.java 76f0afc > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerConfig.java > c4397a1 > src/java/main/org/apache/zookeeper/server/quorum/QuorumPeerMain.java > 0a8a45a > src/java/test/org/apache/zookeeper/server/quorum/QuorumPeerTestBase.java > 89416ca > src/java/test/org/apache/zookeeper/server/quorum/ReconfigBackupTest.java > PRE-CREATION > src/java/test/org/apache/zookeeper/server/quorum/ReconfigLegacyTest.java > c05aa1b > > Diff: https://reviews.apache.org/r/24208/diff/ > > > Testing > ------- > > > Thanks, > > Hongchao Deng > >
