> On Nov. 21, 2014, 7:46 p.m., Jun Rao wrote: > > core/src/main/scala/kafka/log/LogConfig.scala, lines 176-177 > > <https://reviews.apache.org/r/27634/diff/7/?file=765431#file765431line176> > > > > This is a useful genenal check for boolean type. Could we include the > > validation in ConfigDef.parseType() when parsing the boolean type? We > > probably want to make it case insensitive too. Then, we can make this a > > boolean type.
We can't just change current behaviour, since it'll break code in many places, obviously. I would suggest introducing BOOLEAN_STRICT type or even introducing notion of strategy to ConfigDef (i.e. STRICT, TOLERANT, etc.) (I prefer the first option). But IMO, this is out of scope for the current ticket. I would eagerly made these changes, but I would like to keep changes for the 1667 as local as possible. Please, let me know, what's your opinion on this. Thanks - Dmytro ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/#review62608 ----------------------------------------------------------- On Nov. 25, 2014, 11:04 a.m., Dmytro Kostiuchenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27634/ > ----------------------------------------------------------- > > (Updated Nov. 25, 2014, 11:04 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1667 > https://issues.apache.org/jira/browse/KAFKA-1667 > > > Repository: kafka > > > Description > ------- > > KAFKA-1667 Fixed bugs in LogConfig. Added test and documentation > > > KAFKA-1667 Updated tests to reflect new boolean property parsing logic > > > KAFKA-1667 renamed methods to match naming convention > > > KAFKA-1667 Added unit test to cover invalid configuration case > > > KAFKA-1667 Strict UncleanLeaderElection property parsing > > > KAFKA-1667 minor non-functional re-factoring > > > Diffs > ----- > > clients/src/main/java/org/apache/kafka/common/config/ConfigDef.java > c4cea2cc072f4db4ce014b63d226431d3766bef1 > core/src/main/scala/kafka/admin/TopicCommand.scala > 0b2735e7fc42ef9894bef1997b1f06a8ebee5439 > core/src/main/scala/kafka/log/LogConfig.scala > e48922a97727dd0b98f3ae630ebb0af3bef2373d > core/src/main/scala/kafka/utils/Utils.scala > 23aefb4715b177feae1d2f83e8b910653ea10c5f > core/src/test/scala/kafka/log/LogConfigTest.scala PRE-CREATION > core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala > f44568cb25edf25db857415119018fd4c9922f61 > > Diff: https://reviews.apache.org/r/27634/diff/ > > > Testing > ------- > > > Thanks, > > Dmytro Kostiuchenko > >
