You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@kafka.apache.org by Matthias Wessendorf <ma...@apache.org> on 2018/07/04 13:42:13 UTC

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<Header>
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<String, String> 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<String, String> 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

Re: Builder Pattern for kafka-clients in 2.x ?

Posted by Matthias Wessendorf <ma...@apache.org>.

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<Integer, String> 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 <Th...@tivo.com> 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<Header>
>
> 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<String, String> 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<String, String> 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 ?

Posted by Thomas Becker <Th...@tivo.com>.

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<Header>

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<String, String> 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<String, String> 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.