-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35421/#review87842
-----------------------------------------------------------

Ship it!


This minimal fix is fine as long as we're happy with the resulting behavior. 
Just to clarify the possible confusing output, if you created a KafkaProducer 
with some normal producer configs (e.g. bootstrap.servers) and used a recent 
version of Confluent's Avro serializer where we're using AbstractConfigs for 
the serializer and pass in some configs (e.g. schema.registry.url), and both 
call logUnused, you'll first get a complaint that the schema.registry.url 
config was unused, then get a complaint that bootstrap.servers was unused.

Another possible option is to use a custom Map implementation in AbstractConfig 
that allows us to mark when keys have been used. Then we would defer calling 
logUnused() until all components that use that map for configuration had a 
chance to mark which configs they had used (which, for example, we already 
happen to do in KafkaProducer). The obvious drawbacks here are a) it's 
significantly more complexity for some pretty simple functionality and b) since 
we already use Map<?, ?> in the serializer interfaces' and AbstractConfig's 
public API, we'd have to dynamically detect this type of map in AbstractConfig. 
Perhaps having to periodically explain this issue (which I've seen a few more 
times since I initially filed this) will be a simpler solution...

- Ewen Cheslack-Postava


On June 13, 2015, 10:07 a.m., Manikumar Reddy O wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/35421/
> -----------------------------------------------------------
> 
> (Updated June 13, 2015, 10:07 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-2026
>     https://issues.apache.org/jira/browse/KAFKA-2026
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Logging of unused options values taken from this.originals
> 
> 
> Diffs
> -----
> 
>   clients/src/main/java/org/apache/kafka/common/config/AbstractConfig.java 
> c4fa058692f50abb4f47bd344119d805c60123f5 
> 
> Diff: https://reviews.apache.org/r/35421/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Manikumar Reddy O
> 
>

Reply via email to