[
https://issues.apache.org/jira/browse/KAFKA-3049?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16237209#comment-16237209
]
Jeffrey Olchovy commented on KAFKA-3049:
----------------------------------------
Saw this was updated. I believe the existing PR can still merge cleanly, but
let me know if you want an updated patch / PR.
> VerifiableProperties does not respect 'default' properties of underlying
> java.util.Properties instance
> ------------------------------------------------------------------------------------------------------
>
> Key: KAFKA-3049
> URL: https://issues.apache.org/jira/browse/KAFKA-3049
> Project: Kafka
> Issue Type: Bug
> Components: config, core
> Affects Versions: 0.10.0.0, 0.11.0.0, 1.0.0
> Reporter: Jeffrey Olchovy
> Priority: Minor
> Labels: easyfix
>
> When retrieving values from the underlying {{Properties}} instance with the
> {{getString}}, {{get<Type>}}, etc. methods of a {{VerifiableProperties}}
> instance, a call to the underlying {{Properties.containsKey}} method is made.
> This method will not search the default properties values of the instance,
> rendering any default properties defined on the {{Properties}} instance
> useless.
> A practical example is shown below:
> {noformat}
> // suppose we have a base, default set of properties to supply to all
> consumer groups
> val baseProps = new Properties
> baseProps.setProperty("zookeeper.connect", "localhost:2181/kafka")
> baseProps.setProperty("zookeeper.connection.timeout.ms", "2000")
> // additional we have discrete properties instances for each consumer group
> that utilize these defaults
> val groupProps1 = new Properties(baseProps)
> groupProps1.setProperty("group.id", "test-1")
> val groupProps2 = new Properties(baseProps)
> groupProps2.setProperty("group.id", "test-2")
> // when attempting to create an instance of a high-level Consumer with the
> above properties an exception will be thrown due to the aforementioned
> problem description
> java.lang.IllegalArgumentException: requirement failed: Missing required
> property 'zookeeper.connect'
> at scala.Predef$.require(Predef.scala:233)
> at
> kafka.utils.VerifiableProperties.getString(VerifiableProperties.scala:177)
> at kafka.utils.ZKConfig.<init>(ZkUtils.scala:879)
> at kafka.consumer.ConsumerConfig.<init>(ConsumerConfig.scala:100)
> at kafka.consumer.ConsumerConfig.<init>(ConsumerConfig.scala:104)
> // however, the groupProps instances will return the correct value for
> "zookeeper.connect" when using `Properties.getProperty`
> assert(groupProps1.getProperty("zookeeper.connect", "localhost:2181/kafka"))
> assert(groupProps2.getProperty("zookeeper.connect", "localhost:2181/kafka"))
> {noformat}
> I believe it is worthwhile for Kafka to respect the default properties
> feature of {{java.util.Properties}}, and further, that Kafka should
> discourage the use of the methods on {{Properties}} that are inherited from
> {{Hashtable}} (e.g. {{containsKey}}). One can argue that
> {{VerifiableProperties}} is not 'correct' due to this behavior, but a user
> can always workaround this by creating discrete instances of {{Properties}}
> with a set of default properties manually added to each instance. However,
> this is inconvenient and may only encourage the use of the discouraged
> {{Hashtable}} methods like {{putAll}}.
> Two proposed solutions follow:
> 1. Do not delegate to the {{Properties.containsKey}} method during the
> invocation of {{VerifiableProperties.containsKey}}. One can use a null check
> in conjunction with {{getProperty}} in its place.
> 2. Treat the underlying {{Properties}} instance as immutable and assign the
> result of {{Properties.stringPropertyNames()}} to a member of
> {{VerifiableProperties}}. One can check this set of known, available property
> names, which respects the optional default properties, when
> {{VerifiableProperties.containsKey}} is invoked.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
