> On Nov. 7, 2014, 5 a.m., Gwen Shapira wrote: > > core/src/main/scala/kafka/utils/Utils.scala, lines 514-522 > > <https://reviews.apache.org/r/27634/diff/3/?file=751666#file751666line514> > > > > I think I'm not seeing why we need this. > > Shouldn't Scala's JavaConversion class handle this exact case? > > Dmytro Kostiuchenko wrote: > Frankly, I'm new to Scala. But I didn't found a way to convert > java.util.Properties to Map respecting defaults. > java.util.Properties have additional method getProperty() which checks > for value in nested Properties object (see Properties(Properties defaults) > constructor) if key is absent in this one. But is treated as Map, defaults > can't be accessed. Map.get(K, V) is implemented in Properties' parent — > HashTable, and thus, get() knows nothing about default properties. > > Example: > val defaults = new Properties() > defaults.put("foo", "bar") > val props = new Properties(defaults) > props.get("foo") // null > props.getProperty("foo") // "bar"
Oh, yuck. Took me a bit to realize what's going on here. Properties inherits from Map, but can actually contains another map for defaults that isn't use when casting... I'd document why this is necessary - since its not 100% trivial (at least not to me) Perhaps instead of a util method, adding another parse method to ConfigDef that will take Properties as a parameter will make things a bit cleaner. This is just my preference, so I'd wait for one of the committers to chime in here. - Gwen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/27634/#review60188 ----------------------------------------------------------- On Nov. 7, 2014, 1:28 p.m., Dmytro Kostiuchenko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/27634/ > ----------------------------------------------------------- > > (Updated Nov. 7, 2014, 1:28 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-1677 renamed methods to match naming convention > > > KAFKA-1667 Added unit test to cover invalid configuration case > > > 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 > >
