----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/#review61581 -----------------------------------------------------------
Thanks for the patch. Looks good overall. A few comments below. core/src/main/scala/kafka/log/LogConfig.scala <https://reviews.apache.org/r/27634/#comment103317> This is actually a hard limit. core/src/main/scala/kafka/log/LogConfig.scala <https://reviews.apache.org/r/27634/#comment103321> Could we just put those in Object LogConfig? core/src/main/scala/kafka/log/LogConfig.scala <https://reviews.apache.org/r/27634/#comment103318> Two age. core/src/main/scala/kafka/log/LogConfig.scala <https://reviews.apache.org/r/27634/#comment103319> This actually can be controlled at the topic level. core/src/main/scala/kafka/log/LogConfig.scala <https://reviews.apache.org/r/27634/#comment103320> Leading space after ". core/src/main/scala/kafka/log/LogConfig.scala <https://reviews.apache.org/r/27634/#comment103316> The importance looks good. core/src/main/scala/kafka/utils/Utils.scala <https://reviews.apache.org/r/27634/#comment103323> Could we rename this to sth like overridesAndDefaults()? core/src/test/scala/kafka/log/LogConfigTest.scala <https://reviews.apache.org/r/27634/#comment103300> Need to add Apache license header. core/src/test/scala/kafka/log/LogConfigTest.scala <https://reviews.apache.org/r/27634/#comment103312> Does return skip the rest of the config names? - Jun Rao On Nov. 12, 2014, 11:49 a.m., Dmytro Kostiuchenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27634/ > ----------------------------------------------------------- > > (Updated Nov. 12, 2014, 11:49 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 > > > 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 > >
