Re: Java Client Builder pattern for ProducerRecord - thoughts?

[Andrew Coates]

Thanks Mike
On Thu, 18 May 2017 at 21:33, Michael André Pearce <
michael.andre.pea...@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  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 onSend(ProducerRecord 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 onSend(ProducerRecord 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]

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  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 onSend(ProducerRecord 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 onSend(ProducerRecord 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
> 
> 

Java Client Builder pattern for ProducerRecord - thoughts?

[Andrew Coates]

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 onSend(ProducerRecord 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 onSend(ProducerRecord 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