[ https://issues.apache.org/jira/browse/ZOOKEEPER-217?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Patrick Hunt updated ZOOKEEPER-217: ----------------------------------- Status: Open (was: Patch Available) I see a few issues that should be addressed: 1) we should use LOG.fatal instead of LOG.error when about to exit (in main for example) 2) the following exception does not indicate the config param in question -- it might be hard to track down what has to be changed in this case, in the other exceptions you indicate the particular field/variable/etc... that needs to be changed. + throw new IllegalArgumentException(myIdString + + " is not a number"); } 3) why is parse changed to "throws Exception" rather than the more explicit "throws IllegalArgumentException"? 4) specifying the usage in parse seems wrong to me: + if (args.length != 1) { + throw new IllegalArgumentException("USAGE: configFile"); + } How do you know what the usage is going to be? That's up to the main routine. This will result in confusion for the user. + throw new IllegalArgumentException("USAGE: ZooKeeperServer port dat > Errors in config file > --------------------- > > Key: ZOOKEEPER-217 > URL: https://issues.apache.org/jira/browse/ZOOKEEPER-217 > Project: Zookeeper > Issue Type: Improvement > Components: server > Affects Versions: 3.0.0 > Reporter: Flavio Paiva Junqueira > Assignee: Mahadev konar > Priority: Minor > Fix For: 3.0.1, 3.1.0 > > Attachments: ZOOKEEPER-217.patch > > > Discussing 209 with Ben today, we thought that it would be better to have the > parse method of QuorumPeerConfig returning a boolean that indicates whether > the configuration is good or not, and let the caller decide whether to exit > or not. Currently we execute a System.exit() on QuorumPeerConfig.parse when > we have a critical configuration error. -- This message is automatically generated by JIRA. - You can reply to this email to add a comment to the issue online.