> On Nov. 15, 2014, 1:48 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/log/LogConfig.scala, line 46 > > <https://reviews.apache.org/r/27634/diff/5/?file=759553#file759553line46> > > > > This is actually a hard limit.
All docs are just copied from javadoc. I've changed nothing, and thus haven't checked docs correctness. Fixed > On Nov. 15, 2014, 1:48 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/log/LogConfig.scala, lines 62-63 > > <https://reviews.apache.org/r/27634/diff/5/?file=759553#file759553line62> > > > > This actually can be controlled at the topic level. So, I just removed the note about validation purposes. Is it ok? > On Nov. 15, 2014, 1:48 a.m., Jun Rao wrote: > > core/src/test/scala/kafka/log/LogConfigTest.scala, line 52 > > <https://reviews.apache.org/r/27634/diff/5/?file=759555#file759555line52> > > > > Does return skip the rest of the config names? Yes, it is. But it is a lambda that is called for each element of the set, so test actually validates every property. Also test will fail if new property with non-trivial validation (other than positive number) is added but is not covered in test. > On Nov. 15, 2014, 1:48 a.m., Jun Rao wrote: > > core/src/main/scala/kafka/utils/Utils.scala, line 525 > > <https://reviews.apache.org/r/27634/diff/5/?file=759554#file759554line525> > > > > Could we rename this to sth like overridesAndDefaults()? IMO, whatever we choose, it won't be clear from the name what the method does. Unless method is placed in some dedicated class like PropertiesUtils. Some context is required to understand, but it is not so complicated to bother anyway. So, I would leave it as it is. Please confirm that renaming to overridesAndDefaults is necessary and I will do that. - Dmytro ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/#review61581 ----------------------------------------------------------- On Nov. 16, 2014, 5:33 p.m., Dmytro Kostiuchenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27634/ > ----------------------------------------------------------- > > (Updated Nov. 16, 2014, 5:33 p.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 > >
