Java Client Builder pattern for ProducerRecord - thoughts?

[Andrew Coates <an...@apple.com>]

Hi all,

The `ProducerRecord` type has many optional fields and the list has grown over different revisions of Kafka. Kafka supports `ProducerInterceptor`s, which often need to construct new `ProducerRecord`s, based on those passed in, copying most fields from the old to the new record, e.g.:

```java
   public ProducerRecord<K,V> onSend(ProducerRecord<K,V> record) {
       return new ProducerRecord<>(record.topic(), record.partition(), getSpecificTimestampIWantToSet(), record.key(), record.value())
   }
```

If/when a new field is next added to the `ProducerRecord` all existing interceptor implementations will fail to copy across the new field, assuming a backwards compatible constructors exist that allow the old code to compile, (which the tend to do). This makes the code brittle and leaves me with a bad taste in my mouth.

Additionally, the set of `ProducerRecord` constructors is multiplying as new optional fields are being added and not all combinations are supported, though they may be valid.

I was wondering what peoples thoughts would be to introducing a builder pattern on the producer record?  If we did and a pre-initialised builder could be obtained from any existing record, then interceptors can just set/oeverwrite the fields they care about, without additional fields being lost, so the above code becomes:

```java
   public ProducerRecord<K,V> onSend(ProducerRecord<K,V> record) {
       return record.asBuilder()
            .setTimestamp(getSpecificTimestampIWantToSet())
      .build();
   }
```

This has the benefits of less and more clear interceptor code, and the code will pass along new fields, added in a newer version, without modification. (Though interceptor authors can still make the choice to use a constructor instead, dropping new fields - but now they’d have a choice).

If people like this idea then I can create a Jira and a PR. (Would a KIP be required also?). If people don’t, I’ll move along quietly…

Thanks,

Andy

Re: Java Client Builder pattern for ProducerRecord - thoughts?

[Andrew Coates <bi...@gmail.com>]

Thanks Mike
On Thu, 18 May 2017 at 21:33, Michael André Pearce <
michael.andre.pearce@me.com> wrote:

> Hi Andrew,
>
> There is already a kip discussion exactly around this if you look for KIP
> 141 discuss thread.
>
> Cheers
> Mike
>
> Sent from my iPhone
>
> > On 18 May 2017, at 18:29, Andrew Coates <an...@apple.com> wrote:
> >
> > Hi all,
> >
> > The `ProducerRecord` type has many optional fields and the list has
> grown over different revisions of Kafka. Kafka supports
> `ProducerInterceptor`s, which often need to construct new
> `ProducerRecord`s, based on those passed in, copying most fields from the
> old to the new record, e.g.:
> >
> > ```java
> >   public ProducerRecord<K,V> onSend(ProducerRecord<K,V> record) {
> >       return new ProducerRecord<>(record.topic(), record.partition(),
> getSpecificTimestampIWantToSet(), record.key(), record.value())
> >   }
> > ```
> >
> > If/when a new field is next added to the `ProducerRecord` all existing
> interceptor implementations will fail to copy across the new field,
> assuming a backwards compatible constructors exist that allow the old code
> to compile, (which the tend to do). This makes the code brittle and leaves
> me with a bad taste in my mouth.
> >
> > Additionally, the set of `ProducerRecord` constructors is multiplying as
> new optional fields are being added and not all combinations are supported,
> though they may be valid.
> >
> > I was wondering what peoples thoughts would be to introducing a builder
> pattern on the producer record?  If we did and a pre-initialised builder
> could be obtained from any existing record, then interceptors can just
> set/oeverwrite the fields they care about, without additional fields being
> lost, so the above code becomes:
> >
> > ```java
> >   public ProducerRecord<K,V> onSend(ProducerRecord<K,V> record) {
> >       return record.asBuilder()
> >            .setTimestamp(getSpecificTimestampIWantToSet())
> >      .build();
> >   }
> > ```
> >
> > This has the benefits of less and more clear interceptor code, and the
> code will pass along new fields, added in a newer version, without
> modification. (Though interceptor authors can still make the choice to use
> a constructor instead, dropping new fields - but now they’d have a choice).
> >
> > If people like this idea then I can create a Jira and a PR. (Would a KIP
> be required also?). If people don’t, I’ll move along quietly…
> >
> > Thanks,
> >
> > Andy
> >
> >
>

Re: Java Client Builder pattern for ProducerRecord - thoughts?

[Michael André Pearce <mi...@me.com>]

Hi Andrew,

There is already a kip discussion exactly around this if you look for KIP 141 discuss thread.

Cheers
Mike

Sent from my iPhone

> On 18 May 2017, at 18:29, Andrew Coates <an...@apple.com> wrote:
> 
> Hi all,
> 
> The `ProducerRecord` type has many optional fields and the list has grown over different revisions of Kafka. Kafka supports `ProducerInterceptor`s, which often need to construct new `ProducerRecord`s, based on those passed in, copying most fields from the old to the new record, e.g.:
> 
> ```java
>   public ProducerRecord<K,V> onSend(ProducerRecord<K,V> record) {
>       return new ProducerRecord<>(record.topic(), record.partition(), getSpecificTimestampIWantToSet(), record.key(), record.value())
>   }
> ```
> 
> If/when a new field is next added to the `ProducerRecord` all existing interceptor implementations will fail to copy across the new field, assuming a backwards compatible constructors exist that allow the old code to compile, (which the tend to do). This makes the code brittle and leaves me with a bad taste in my mouth.
> 
> Additionally, the set of `ProducerRecord` constructors is multiplying as new optional fields are being added and not all combinations are supported, though they may be valid.
> 
> I was wondering what peoples thoughts would be to introducing a builder pattern on the producer record?  If we did and a pre-initialised builder could be obtained from any existing record, then interceptors can just set/oeverwrite the fields they care about, without additional fields being lost, so the above code becomes:
> 
> ```java
>   public ProducerRecord<K,V> onSend(ProducerRecord<K,V> record) {
>       return record.asBuilder()
>            .setTimestamp(getSpecificTimestampIWantToSet())
>      .build();
>   }
> ```
> 
> This has the benefits of less and more clear interceptor code, and the code will pass along new fields, added in a newer version, without modification. (Though interceptor authors can still make the choice to use a constructor instead, dropping new fields - but now they’d have a choice).
> 
> If people like this idea then I can create a Jira and a PR. (Would a KIP be required also?). If people don’t, I’ll move along quietly…
> 
> Thanks,
> 
> Andy
> 
>