Re: Builder Pattern for kafka-clients in 2.x ?
Hi, Tommy! thanks for the feeback - I've reopened the PR adding the ctor overload. I've drafted a ProducerBuilder, which can be used like: final Producer producer = new ProducerBuilder() .partitionerClass(DefaultPartitioner.class) .keySerializer(IntegerSerializer.class) .valueSerializer(StringSerializer.class) .bootstrapServers("localhost:9092") .build(); producer.send(new ProducerRecord("mytopic", myKey, "the-val", myHeaderIterable)); Internally the builder populates a property and on build() it delegates it to the KafkaProducer ctor. code: https://gist.github.com/matzew/b98dbacf7f40f27c9f666b736a2428d3 I think this reads much nicer, than directly dealing w/ the current way for creating conumers and producers. How do people feel about this? thanks, Matthias On Thu, Jul 5, 2018 at 3:11 PM Thomas Becker wrote: > Personally, I like the idea of builders for the producer/consumer > themselves, but I'm less enthusiastic about one for ProducerRecord. Mostly > because I think the following is overly verbose/reads poorly: > > producer.send(ProducerRecord.builder() > .topic("mytopic") > .key("Key") > .value("the-val") > .headers(myHeaderIterable) > .build()); > > as compared to: > > producer.send(new ProducerRecord("mytopic", "Key", "the-val", > myHeaderIterable)); > > I think constructor overloads are fine for small data classes like this. > The producer/consumer clietns themselves have a lot of options represented > by various configuration keys, and a builder pattern makes these easily > discoverable in code. > > -Tommy > > On Wed, 2018-07-04 at 15:42 +0200, Matthias Wessendorf wrote: > > Hi, > > > I was filing KAFKA-7059 ([1]) and sent a PR adding a new ctor: > > -- > > public ProducerRecord(String topic, K key, V value, Iterable > > headers) > > --- > > > One reasonable comment on the PR was instead of doing constructor > > overloading, why not working on a builder for the ProducerRecord class. > > > I think this is generally a nice idea I was wondering if there is much > > interest in ? > > > Sample: > > --- > > final ProducerRecord myRecord = ProducerRecord.builder() // > > or an exposed builder > > .topic("mytopic") > > .key("Key") > > .value("the-val") > > .headers(myHeaderIterable) > > .build(); > > --- > > > While at it - instead of just offering a builder for the "ProducerRecord" > > class, why not adding a builder for the "KafkaProducer" and "KafkaConsumer" > > clazzes. > > > --- > > final KafkaProducer myProducer = KafkaProducer.builder() // > > or an exposed builder clazz > > .config(myProducerConfig) > > .keySerializer(myStringSerializer) > > .valueSerializer(myStringSerializer) > > .build(); > > --- > > > to even make the above more nice, I think the "ProducerConfig" (analog the > > ConsumerConfig) configuration options could be also made accesible w/ this > > fluent API - instead of properties/map, which is what now dominates the > > creation of the Consumers/Producers. > > > > Any thoughts? If there is interest, I am happy to start a KIP w/ a first > > draft of the suggested API! > > > Cheers, > > Matthias > > > [1] https://issues.apache.org/jira/browse/KAFKA-7059 > > > > > > > > This email and any attachments may contain confidential and privileged > material for the sole use of the intended recipient. Any review, copying, > or distribution of this email (or any attachments) by others is prohibited. > If you are not the intended recipient, please contact the sender > immediately and permanently delete this email and any attachments. No > employee or agent of TiVo Inc. is authorized to conclude any binding > agreement on behalf of TiVo Inc. by email. Binding agreements with TiVo > Inc. may only be made by a signed written agreement. > -- Matthias Wessendorf github: https://github.com/matzew twitter: http://twitter.com/mwessendorf
Re: Builder Pattern for kafka-clients in 2.x ?
Personally, I like the idea of builders for the producer/consumer themselves, but I'm less enthusiastic about one for ProducerRecord. Mostly because I think the following is overly verbose/reads poorly: producer.send(ProducerRecord.builder() .topic("mytopic") .key("Key") .value("the-val") .headers(myHeaderIterable) .build()); as compared to: producer.send(new ProducerRecord("mytopic", "Key", "the-val", myHeaderIterable)); I think constructor overloads are fine for small data classes like this. The producer/consumer clietns themselves have a lot of options represented by various configuration keys, and a builder pattern makes these easily discoverable in code. -Tommy On Wed, 2018-07-04 at 15:42 +0200, Matthias Wessendorf wrote: Hi, I was filing KAFKA-7059 ([1]) and sent a PR adding a new ctor: -- public ProducerRecord(String topic, K key, V value, Iterable headers) --- One reasonable comment on the PR was instead of doing constructor overloading, why not working on a builder for the ProducerRecord class. I think this is generally a nice idea I was wondering if there is much interest in ? Sample: --- final ProducerRecord myRecord = ProducerRecord.builder() // or an exposed builder .topic("mytopic") .key("Key") .value("the-val") .headers(myHeaderIterable) .build(); --- While at it - instead of just offering a builder for the "ProducerRecord" class, why not adding a builder for the "KafkaProducer" and "KafkaConsumer" clazzes. --- final KafkaProducer myProducer = KafkaProducer.builder() // or an exposed builder clazz .config(myProducerConfig) .keySerializer(myStringSerializer) .valueSerializer(myStringSerializer) .build(); --- to even make the above more nice, I think the "ProducerConfig" (analog the ConsumerConfig) configuration options could be also made accesible w/ this fluent API - instead of properties/map, which is what now dominates the creation of the Consumers/Producers. Any thoughts? If there is interest, I am happy to start a KIP w/ a first draft of the suggested API! Cheers, Matthias [1] https://issues.apache.org/jira/browse/KAFKA-7059 This email and any attachments may contain confidential and privileged material for the sole use of the intended recipient. Any review, copying, or distribution of this email (or any attachments) by others is prohibited. If you are not the intended recipient, please contact the sender immediately and permanently delete this email and any attachments. No employee or agent of TiVo Inc. is authorized to conclude any binding agreement on behalf of TiVo Inc. by email. Binding agreements with TiVo Inc. may only be made by a signed written agreement.
Builder Pattern for kafka-clients in 2.x ?
Hi, I was filing KAFKA-7059 ([1]) and sent a PR adding a new ctor: -- public ProducerRecord(String topic, K key, V value, Iterable headers) --- One reasonable comment on the PR was instead of doing constructor overloading, why not working on a builder for the ProducerRecord class. I think this is generally a nice idea I was wondering if there is much interest in ? Sample: --- final ProducerRecord myRecord = ProducerRecord.builder() // or an exposed builder .topic("mytopic") .key("Key") .value("the-val") .headers(myHeaderIterable) .build(); --- While at it - instead of just offering a builder for the "ProducerRecord" class, why not adding a builder for the "KafkaProducer" and "KafkaConsumer" clazzes. --- final KafkaProducer myProducer = KafkaProducer.builder() // or an exposed builder clazz .config(myProducerConfig) .keySerializer(myStringSerializer) .valueSerializer(myStringSerializer) .build(); --- to even make the above more nice, I think the "ProducerConfig" (analog the ConsumerConfig) configuration options could be also made accesible w/ this fluent API - instead of properties/map, which is what now dominates the creation of the Consumers/Producers. Any thoughts? If there is interest, I am happy to start a KIP w/ a first draft of the suggested API! Cheers, Matthias [1] https://issues.apache.org/jira/browse/KAFKA-7059 -- Matthias Wessendorf github: https://github.com/matzew twitter: http://twitter.com/mwessendorf