Re: Review Request 35421: Patch for KAFKA-2026
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35421/#review93636 --- Ship it! Ship It! - 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
Re: Review Request 35421: Patch for KAFKA-2026
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35421/#review93543 --- Ship it! Ship It! - Jiangjie Qin 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
Review Request 35421: Patch for KAFKA-2026
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35421/ --- 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
Re: Review Request 35421: Patch for KAFKA-2026
--- 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
