You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Stephane Maarek <st...@simplemachines.com.au> on 2017/04/19 22:34:00 UTC
[DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Hi all,
My first KIP, let me know your thoughts!
https://cwiki.apache.org/confluence/display/KAFKA/KIP+141+-+ProducerRecordBuilder+Interface
Cheers,
Stephane
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Stephane Maarek <st...@simplemachines.com.au>.
Agreed! I’ll update the KIP shortly
I’m self-taught, so I guess I still have a lot to learn (
If anything, I think the withForcedPartition method could be just removed, and if users need to force partition, shouldn’t they just mandatorily use a custom Partitioner? It would achieve the same purpose, but in a much “safer” way?
It just sounds like there would be two ways to achieve the same purpose here. Nonetheless I understand it could be a breaking change
- Stephane
On 22/4/17, 4:05 am, "Matthias J. Sax" <ma...@confluent.io> wrote:
Ismael's comment is quite reasonable.
And I actually like the idea about `withForcedPartition` -- it helps
newcomers to understand the API better. Even if I don't agree with some
of you reasoning:
> I was always told that the key is solely the determining factor
Bad teacher?
> I find it incredibly unintuitive and dangerous to provide the users the ability to force a partition
Disagree completely. This is a very useful feature for advanced use
cases. Btw: users can also specify a custom partitioner -- and this
custom partitioner could also do any computation to determine the
partitions (it also has access to key and value -- thus, it could also
use the value to compute the partition).
But I like `withForcedPartition` because it indicates that the
partitioner (default or custom) is not going to be used for this case.
Btw: if we plan to make `public ProducerRecord(String topic, Integer
partition, Long timestamp, K key, V value)` protected as some point, we
should deprecate it, too.
I also like the compromise you suggest
>> public ProducerRecordBuilder(String topic, K key, V value)
>>
>> coming with withPartition and withTimestamp?
The only thing, I still don't like is that we use a builder and are thus
forced to call .build() -- boilerplate.
Maybe we would just change ProducerRecord itself? Like:
new ProducerRecord(topic, key,
value).withTimestamp(ts).withForcedPartition(p);
WDYT?
-Matthias
On 4/20/17 4:57 PM, Stephane Maarek wrote:
> Matthias: I was definitely on board with you at first, but Ismael made the comment that for:
>
> public ProducerRecord(String topic, K key, V value, Integer partition)
> public ProducerRecord(String topic, K key, V value, Long timestamp)
>
> Integer and Long are way too close in terms of meaning, and could provide a strong misuse of the timestamp / partition field.
> Therefore I started with a builder pattern for explicit argument declaration. Seems like a lot of boilerplate, but it makes things quite easy to understand.
>
> I like your point about the necessity of the key, and that users should set it to null explicitely.
>
> Damian: I like your idea of public ProducerRecordBuilder(String topic, V value)
> Finally, I also chose the withForcedPartition because in my learning of Kafka, I was always told that the key is solely the determining factor to know how a messages makes it to a partition. I find it incredibly unintuitive and dangerous to provide the users the ability to force a partition. If anything they should be providing their own key -> partition mapping, but I’m really against letting users force a partition within the producerRecord. What do you think?
>
>
> What do you both think of the more opiniated:
>
> public ProducerRecordBuilder(String topic, K key, V value)
>
> coming with withPartition and withTimestamp?
>
>
>
> On 21/4/17, 2:24 am, "Matthias J. Sax" <ma...@confluent.io> wrote:
>
> Thanks for the KIP!
>
> While I agree, that the current API is not perfect, I am not sure if a
> builder pattern does make sense here, because it's not too many parameters.
>
> IMHO, builder pattern makes sense if there are many optional parameters.
> For a ProducerRecord, I think there are only 2 optional parameters:
> partition and timestamp.
>
> I don't think key should be optional, because uses should be "forced" to
> think about the key argument as it effects the partitioning. Thus,
> providing an explicit `null` if there is no key seems reasonable to me.
>
> Overall I think that providing 3 overloads would be sufficient:
>
> > public ProducerRecord(String topic, K key, V value, Integer partition)
> > public ProducerRecord(String topic, K key, V value, Long timestamp)
> > public ProducerRecord(String topic, K key, V value, Integer partition, Long timestamp)
>
>
> Just my 2 cents.
>
> -Matthias
>
>
> On 4/20/17 4:20 AM, Damian Guy wrote:
> > Hi Stephane,
> >
> > Thanks for the KIP. Overall it looks ok, though i think the builder should
> > enforce the required parameters by supplying them via the constructor, i.e,
> >
> > public ProducerRecordBuilder(String topic, V value)
> >
> > You can then remove the withValue and withTopic methods
> >
> > I also think withForcedPartition should just be withPartition
> >
> > Thanks,
> > Damian
> >
> > On Wed, 19 Apr 2017 at 23:34 Stephane Maarek <st...@simplemachines.com.au>
> > wrote:
> >
> >> Hi all,
> >>
> >> My first KIP, let me know your thoughts!
> >>
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP+141+-+ProducerRecordBuilder+Interface
> >>
> >>
> >> Cheers,
> >> Stephane
> >>
> >
>
>
>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by "Matthias J. Sax" <ma...@confluent.io>.
Ismael's comment is quite reasonable.
And I actually like the idea about `withForcedPartition` -- it helps
newcomers to understand the API better. Even if I don't agree with some
of you reasoning:
> I was always told that the key is solely the determining factor
Bad teacher?
> I find it incredibly unintuitive and dangerous to provide the users the ability to force a partition
Disagree completely. This is a very useful feature for advanced use
cases. Btw: users can also specify a custom partitioner -- and this
custom partitioner could also do any computation to determine the
partitions (it also has access to key and value -- thus, it could also
use the value to compute the partition).
But I like `withForcedPartition` because it indicates that the
partitioner (default or custom) is not going to be used for this case.
Btw: if we plan to make `public ProducerRecord(String topic, Integer
partition, Long timestamp, K key, V value)` protected as some point, we
should deprecate it, too.
I also like the compromise you suggest
>> public ProducerRecordBuilder(String topic, K key, V value)
>>
>> coming with withPartition and withTimestamp?
The only thing, I still don't like is that we use a builder and are thus
forced to call .build() -- boilerplate.
Maybe we would just change ProducerRecord itself? Like:
new ProducerRecord(topic, key,
value).withTimestamp(ts).withForcedPartition(p);
WDYT?
-Matthias
On 4/20/17 4:57 PM, Stephane Maarek wrote:
> Matthias: I was definitely on board with you at first, but Ismael made the comment that for:
>
> public ProducerRecord(String topic, K key, V value, Integer partition)
> public ProducerRecord(String topic, K key, V value, Long timestamp)
>
> Integer and Long are way too close in terms of meaning, and could provide a strong misuse of the timestamp / partition field.
> Therefore I started with a builder pattern for explicit argument declaration. Seems like a lot of boilerplate, but it makes things quite easy to understand.
>
> I like your point about the necessity of the key, and that users should set it to null explicitely.
>
> Damian: I like your idea of public ProducerRecordBuilder(String topic, V value)
> Finally, I also chose the withForcedPartition because in my learning of Kafka, I was always told that the key is solely the determining factor to know how a messages makes it to a partition. I find it incredibly unintuitive and dangerous to provide the users the ability to force a partition. If anything they should be providing their own key -> partition mapping, but I’m really against letting users force a partition within the producerRecord. What do you think?
>
>
> What do you both think of the more opiniated:
>
> public ProducerRecordBuilder(String topic, K key, V value)
>
> coming with withPartition and withTimestamp?
>
>
>
> On 21/4/17, 2:24 am, "Matthias J. Sax" <ma...@confluent.io> wrote:
>
> Thanks for the KIP!
>
> While I agree, that the current API is not perfect, I am not sure if a
> builder pattern does make sense here, because it's not too many parameters.
>
> IMHO, builder pattern makes sense if there are many optional parameters.
> For a ProducerRecord, I think there are only 2 optional parameters:
> partition and timestamp.
>
> I don't think key should be optional, because uses should be "forced" to
> think about the key argument as it effects the partitioning. Thus,
> providing an explicit `null` if there is no key seems reasonable to me.
>
> Overall I think that providing 3 overloads would be sufficient:
>
> > public ProducerRecord(String topic, K key, V value, Integer partition)
> > public ProducerRecord(String topic, K key, V value, Long timestamp)
> > public ProducerRecord(String topic, K key, V value, Integer partition, Long timestamp)
>
>
> Just my 2 cents.
>
> -Matthias
>
>
> On 4/20/17 4:20 AM, Damian Guy wrote:
> > Hi Stephane,
> >
> > Thanks for the KIP. Overall it looks ok, though i think the builder should
> > enforce the required parameters by supplying them via the constructor, i.e,
> >
> > public ProducerRecordBuilder(String topic, V value)
> >
> > You can then remove the withValue and withTopic methods
> >
> > I also think withForcedPartition should just be withPartition
> >
> > Thanks,
> > Damian
> >
> > On Wed, 19 Apr 2017 at 23:34 Stephane Maarek <st...@simplemachines.com.au>
> > wrote:
> >
> >> Hi all,
> >>
> >> My first KIP, let me know your thoughts!
> >>
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP+141+-+ProducerRecordBuilder+Interface
> >>
> >>
> >> Cheers,
> >> Stephane
> >>
> >
>
>
>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Michal Borowiecki <mi...@openbet.com>.
However, if you're used to working with immutables, it can feel natural
that withPartition would return a new object, so it could be more prone
to mistakes.
Cheers,
Michal
On 23/04/17 10:41, Michal Borowiecki wrote:
>
> IMHO, the ProducerRecord is anyway not immutable until send, since key
> and value don't have to immutable, until serialized on send.
>
> So if immutability is a concern, I think it would have to be enforced
> in send as Mike suggested, don't see much point in enforcing
> immutability prior to send.
>
> Therefore the with pattern should not be discarded just for that reason.
>
> Thanks,
>
> Michal
>
>
> On 23/04/17 06:34, Stephane Maarek wrote:
>> Good call.
>> That\u2019s when I heavily miss Scala Case classes and options. You get clarity on optional vs mandatory fields, one constructor, and immutability. If losing Immutability is an issue, then the with pattern is a no-go and then I\u2019ll just add a missing constructor the way Ismael described it. That\u2019ll make the PR way simpler, with limited refactoring.
>>
>> Regarding the ConsumerRecord, I\u2019m happy to have a look, but it\u2019s the first time I see it concretely. When would you manually construct such a record? Isn\u2019t the client handling all that for you behind the scene?
>>
>>
>> On 23/4/17, 3:21 pm, "Michael Pearce"<Mi...@ig.com> wrote:
>>
>> If moving to a wither pattern instead of a builder. How will this enforce immutability? Eg current PR it is now changing to allow possible change values once set.
>>
>> Or are you proposing to change it to a mutable record? And move to a closable record similar to the closing of the headers on send.
>>
>> How about also the consumer record, is this also being looked at so we don't have two very different styles.
>>
>> Cheers
>> Mike
>>
>>
>>
>> Sent using OWA for iPhone
>> ________________________________________
>> From:ismaelj@gmail.com <is...@gmail.com> on behalf of Ismael Juma<is...@juma.me.uk>
>> Sent: Saturday, April 22, 2017 11:53:45 PM
>> To:dev@kafka.apache.org
>> Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
>>
>> On Sat, Apr 22, 2017 at 6:16 PM, Matthias J. Sax<ma...@confluent.io>
>> wrote:
>>
>> > I think Ismael's suggestion is a valid alternative.
>> >
>> > However, `timestamp` is an optional field and thus we should have at
>> > least two constructors for this:
>> >
>> > - ProducerRecord(String topic, K key, V value)
>>
>> - ProducerRecord(String topic, K key, V value, Long timestamp)
>> >
>>
>> Yes, the other one already exists.
>>
>> Ismael
>> The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
>>
>>
>>
>
> --
> Signature
> <http://www.openbet.com/> Michal Borowiecki
> Senior Software Engineer L4
> T: +44 208 742 1600
>
>
> +44 203 249 8448
>
>
>
> E: michal.borowiecki@openbet.com
> W: www.openbet.com <http://www.openbet.com/>
>
>
> OpenBet Ltd
>
> Chiswick Park Building 9
>
> 566 Chiswick High Rd
>
> London
>
> W4 5XT
>
> UK
>
>
> <https://www.openbet.com/email_promo>
>
> This message is confidential and intended only for the addressee. If
> you have received this message in error, please immediately notify the
> postmaster@openbet.com <ma...@openbet.com> and delete it
> from your system as well as any copies. The content of e-mails as well
> as traffic data may be monitored by OpenBet for employment and
> security purposes. To protect the environment please do not print this
> e-mail unless necessary. OpenBet Ltd. Registered Office: Chiswick Park
> Building 9, 566 Chiswick High Road, London, W4 5XT, United Kingdom. A
> company registered in England and Wales. Registered no. 3134634. VAT
> no. GB927523612
>
--
Signature
<http://www.openbet.com/> Michal Borowiecki
Senior Software Engineer L4
T: +44 208 742 1600
+44 203 249 8448
E: michal.borowiecki@openbet.com
W: www.openbet.com <http://www.openbet.com/>
OpenBet Ltd
Chiswick Park Building 9
566 Chiswick High Rd
London
W4 5XT
UK
<https://www.openbet.com/email_promo>
This message is confidential and intended only for the addressee. If you
have received this message in error, please immediately notify the
postmaster@openbet.com <ma...@openbet.com> and delete it
from your system as well as any copies. The content of e-mails as well
as traffic data may be monitored by OpenBet for employment and security
purposes. To protect the environment please do not print this e-mail
unless necessary. OpenBet Ltd. Registered Office: Chiswick Park Building
9, 566 Chiswick High Road, London, W4 5XT, United Kingdom. A company
registered in England and Wales. Registered no. 3134634. VAT no.
GB927523612
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Michal Borowiecki <mi...@openbet.com>.
IMHO, the ProducerRecord is anyway not immutable until send, since key
and value don't have to immutable, until serialized on send.
So if immutability is a concern, I think it would have to be enforced in
send as Mike suggested, don't see much point in enforcing immutability
prior to send.
Therefore the with pattern should not be discarded just for that reason.
Thanks,
Michal
On 23/04/17 06:34, Stephane Maarek wrote:
> Good call.
> That\u2019s when I heavily miss Scala Case classes and options. You get clarity on optional vs mandatory fields, one constructor, and immutability. If losing Immutability is an issue, then the with pattern is a no-go and then I\u2019ll just add a missing constructor the way Ismael described it. That\u2019ll make the PR way simpler, with limited refactoring.
>
> Regarding the ConsumerRecord, I\u2019m happy to have a look, but it\u2019s the first time I see it concretely. When would you manually construct such a record? Isn\u2019t the client handling all that for you behind the scene?
>
>
> On 23/4/17, 3:21 pm, "Michael Pearce" <Mi...@ig.com> wrote:
>
> If moving to a wither pattern instead of a builder. How will this enforce immutability? Eg current PR it is now changing to allow possible change values once set.
>
> Or are you proposing to change it to a mutable record? And move to a closable record similar to the closing of the headers on send.
>
> How about also the consumer record, is this also being looked at so we don't have two very different styles.
>
> Cheers
> Mike
>
>
>
> Sent using OWA for iPhone
> ________________________________________
> From: ismaelj@gmail.com <is...@gmail.com> on behalf of Ismael Juma <is...@juma.me.uk>
> Sent: Saturday, April 22, 2017 11:53:45 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
>
> On Sat, Apr 22, 2017 at 6:16 PM, Matthias J. Sax <ma...@confluent.io>
> wrote:
>
> > I think Ismael's suggestion is a valid alternative.
> >
> > However, `timestamp` is an optional field and thus we should have at
> > least two constructors for this:
> >
> > - ProducerRecord(String topic, K key, V value)
>
> - ProducerRecord(String topic, K key, V value, Long timestamp)
> >
>
> Yes, the other one already exists.
>
> Ismael
> The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
>
>
>
--
Signature
<http://www.openbet.com/> Michal Borowiecki
Senior Software Engineer L4
T: +44 208 742 1600
+44 203 249 8448
E: michal.borowiecki@openbet.com
W: www.openbet.com <http://www.openbet.com/>
OpenBet Ltd
Chiswick Park Building 9
566 Chiswick High Rd
London
W4 5XT
UK
<https://www.openbet.com/email_promo>
This message is confidential and intended only for the addressee. If you
have received this message in error, please immediately notify the
postmaster@openbet.com <ma...@openbet.com> and delete it
from your system as well as any copies. The content of e-mails as well
as traffic data may be monitored by OpenBet for employment and security
purposes. To protect the environment please do not print this e-mail
unless necessary. OpenBet Ltd. Registered Office: Chiswick Park Building
9, 566 Chiswick High Road, London, W4 5XT, United Kingdom. A company
registered in England and Wales. Registered no. 3134634. VAT no.
GB927523612
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Stephane Maarek <st...@simplemachines.com.au>.
Good call.
That’s when I heavily miss Scala Case classes and options. You get clarity on optional vs mandatory fields, one constructor, and immutability. If losing Immutability is an issue, then the with pattern is a no-go and then I’ll just add a missing constructor the way Ismael described it. That’ll make the PR way simpler, with limited refactoring.
Regarding the ConsumerRecord, I’m happy to have a look, but it’s the first time I see it concretely. When would you manually construct such a record? Isn’t the client handling all that for you behind the scene?
On 23/4/17, 3:21 pm, "Michael Pearce" <Mi...@ig.com> wrote:
If moving to a wither pattern instead of a builder. How will this enforce immutability? Eg current PR it is now changing to allow possible change values once set.
Or are you proposing to change it to a mutable record? And move to a closable record similar to the closing of the headers on send.
How about also the consumer record, is this also being looked at so we don't have two very different styles.
Cheers
Mike
Sent using OWA for iPhone
________________________________________
From: ismaelj@gmail.com <is...@gmail.com> on behalf of Ismael Juma <is...@juma.me.uk>
Sent: Saturday, April 22, 2017 11:53:45 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
On Sat, Apr 22, 2017 at 6:16 PM, Matthias J. Sax <ma...@confluent.io>
wrote:
> I think Ismael's suggestion is a valid alternative.
>
> However, `timestamp` is an optional field and thus we should have at
> least two constructors for this:
>
> - ProducerRecord(String topic, K key, V value)
- ProducerRecord(String topic, K key, V value, Long timestamp)
>
Yes, the other one already exists.
Ismael
The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Michael Pearce <Mi...@ig.com>.
If moving to a wither pattern instead of a builder. How will this enforce immutability? Eg current PR it is now changing to allow possible change values once set.
Or are you proposing to change it to a mutable record? And move to a closable record similar to the closing of the headers on send.
How about also the consumer record, is this also being looked at so we don't have two very different styles.
Cheers
Mike
Sent using OWA for iPhone
________________________________________
From: ismaelj@gmail.com <is...@gmail.com> on behalf of Ismael Juma <is...@juma.me.uk>
Sent: Saturday, April 22, 2017 11:53:45 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
On Sat, Apr 22, 2017 at 6:16 PM, Matthias J. Sax <ma...@confluent.io>
wrote:
> I think Ismael's suggestion is a valid alternative.
>
> However, `timestamp` is an optional field and thus we should have at
> least two constructors for this:
>
> - ProducerRecord(String topic, K key, V value)
- ProducerRecord(String topic, K key, V value, Long timestamp)
>
Yes, the other one already exists.
Ismael
The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Ismael Juma <is...@juma.me.uk>.
On Sat, Apr 22, 2017 at 6:16 PM, Matthias J. Sax <ma...@confluent.io>
wrote:
> I think Ismael's suggestion is a valid alternative.
>
> However, `timestamp` is an optional field and thus we should have at
> least two constructors for this:
>
> - ProducerRecord(String topic, K key, V value)
- ProducerRecord(String topic, K key, V value, Long timestamp)
>
Yes, the other one already exists.
Ismael
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by "Matthias J. Sax" <ma...@confluent.io>.
I think Ismael's suggestion is a valid alternative.
However, `timestamp` is an optional field and thus we should have at
least two constructors for this:
- ProducerRecord(String topic, K key, V value)
- ProducerRecord(String topic, K key, V value, Long timestamp)
> If anything, I think the withForcedPartition method could be just removed, and if users need to force partition, shouldn’t they just mandatorily use a custom Partitioner? It would achieve the same purpose, but in a much “safer” way?
> It just sounds like there would be two ways to achieve the same purpose here. Nonetheless I understand it could be a breaking change
There is an important difference here: If a Partitioner is used, you can
change the strategy via config, while if you specify the partition by
constructor it's hard coded. Thus, while the first gives you more
flexibility, the second allows you to guard against miss configuration.
Furthermore, the decision to which partition you want to send might
depend not just on the key and value (you might even want to send the
same record to multiple partitions). This would not be possible using
the Partitioner, that should only use the give parameters to compute the
partition.
It might sound like a subtle difference, but IMHO you cannot say one is
safer (or better) than the other. Both are valid alternatives and it
depends on the use case. But I agree, that a custom partitioner covers
the lion's share of use cases :)
-Matthias
On 4/21/17 5:50 PM, Stephane Maarek wrote:
> Hi Ismael,
>
> Good points
> I think I was headed in that direction: https://github.com/apache/kafka/pull/2894
> 1. That’s a possibility. I’m just unsure about how the message format will evolve in future versions, because adding constructors is painful if more parameters come into play. The approach above (KIP / PR) can easily allow for message format extensions
> 2. /3 Not a bad idea, maybe I’ll explore that as well. Just looking for feedback on the KIP / PR first as it’s been updated 15 minutes ago
>
>
> On 22/4/17, 10:38 am, "Ismael Juma" <ismaelj@gmail.com on behalf of ismael@juma.me.uk> wrote:
>
> Thanks for the KIP. A possible alternative:
>
> 1. Add constructor ProducerRecord(String topic, K key, V value, Long
> timestamp). This provides an unambiguous constructor that allows one to
> pass a timestamp without a partition, which is the main requirement of the
> KIP.
>
> We could also consider:
>
> 2. Add a couple of `createWithPartition` static factory methods to replace
> the existing constructors that take a partition. The idea being that
> passing a partition is different enough that it should be called out
> specifically.
>
> 3. Deprecate the existing constructors that take a partition so that we can
> remove them (or make one of them private/protected) in a future release
>
> Because ProducerRecord is used so widely, we should make sure that there is
> real value in doing 2 and 3. Otherwise, we should stick to 1.
>
> Ismael
>
> On Fri, Apr 21, 2017 at 12:57 AM, Stephane Maarek <
> stephane@simplemachines.com.au> wrote:
>
> > Matthias: I was definitely on board with you at first, but Ismael made the
> > comment that for:
> >
> > public ProducerRecord(String topic, K key, V value, Integer partition)
> > public ProducerRecord(String topic, K key, V value, Long timestamp)
> >
> > Integer and Long are way too close in terms of meaning, and could provide
> > a strong misuse of the timestamp / partition field.
> > Therefore I started with a builder pattern for explicit argument
> > declaration. Seems like a lot of boilerplate, but it makes things quite
> > easy to understand.
> >
> > I like your point about the necessity of the key, and that users should
> > set it to null explicitely.
> >
> > Damian: I like your idea of public ProducerRecordBuilder(String topic, V
> > value)
> > Finally, I also chose the withForcedPartition because in my learning of
> > Kafka, I was always told that the key is solely the determining factor to
> > know how a messages makes it to a partition. I find it incredibly
> > unintuitive and dangerous to provide the users the ability to force a
> > partition. If anything they should be providing their own key -> partition
> > mapping, but I’m really against letting users force a partition within the
> > producerRecord. What do you think?
> >
> >
> > What do you both think of the more opiniated:
> >
> > public ProducerRecordBuilder(String topic, K key, V value)
> >
> > coming with withPartition and withTimestamp?
> >
> >
> >
> > On 21/4/17, 2:24 am, "Matthias J. Sax" <ma...@confluent.io> wrote:
> >
> > Thanks for the KIP!
> >
> > While I agree, that the current API is not perfect, I am not sure if a
> > builder pattern does make sense here, because it's not too many
> > parameters.
> >
> > IMHO, builder pattern makes sense if there are many optional
> > parameters.
> > For a ProducerRecord, I think there are only 2 optional parameters:
> > partition and timestamp.
> >
> > I don't think key should be optional, because uses should be "forced"
> > to
> > think about the key argument as it effects the partitioning. Thus,
> > providing an explicit `null` if there is no key seems reasonable to me.
> >
> > Overall I think that providing 3 overloads would be sufficient:
> >
> > > public ProducerRecord(String topic, K key, V value, Integer
> > partition)
> > > public ProducerRecord(String topic, K key, V value, Long timestamp)
> > > public ProducerRecord(String topic, K key, V value, Integer
> > partition, Long timestamp)
> >
> >
> > Just my 2 cents.
> >
> > -Matthias
> >
> >
> > On 4/20/17 4:20 AM, Damian Guy wrote:
> > > Hi Stephane,
> > >
> > > Thanks for the KIP. Overall it looks ok, though i think the builder
> > should
> > > enforce the required parameters by supplying them via the
> > constructor, i.e,
> > >
> > > public ProducerRecordBuilder(String topic, V value)
> > >
> > > You can then remove the withValue and withTopic methods
> > >
> > > I also think withForcedPartition should just be withPartition
> > >
> > > Thanks,
> > > Damian
> > >
> > > On Wed, 19 Apr 2017 at 23:34 Stephane Maarek <
> > stephane@simplemachines.com.au>
> > > wrote:
> > >
> > >> Hi all,
> > >>
> > >> My first KIP, let me know your thoughts!
> > >>
> > >> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> > 141+-+ProducerRecordBuilder+Interface
> > >>
> > >>
> > >> Cheers,
> > >> Stephane
> > >>
> > >
> >
> >
> >
> >
> >
>
>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Stephane Maarek <st...@simplemachines.com.au>.
Hi Ismael,
Good points
I think I was headed in that direction: https://github.com/apache/kafka/pull/2894
1. That’s a possibility. I’m just unsure about how the message format will evolve in future versions, because adding constructors is painful if more parameters come into play. The approach above (KIP / PR) can easily allow for message format extensions
2. /3 Not a bad idea, maybe I’ll explore that as well. Just looking for feedback on the KIP / PR first as it’s been updated 15 minutes ago
On 22/4/17, 10:38 am, "Ismael Juma" <ismaelj@gmail.com on behalf of ismael@juma.me.uk> wrote:
Thanks for the KIP. A possible alternative:
1. Add constructor ProducerRecord(String topic, K key, V value, Long
timestamp). This provides an unambiguous constructor that allows one to
pass a timestamp without a partition, which is the main requirement of the
KIP.
We could also consider:
2. Add a couple of `createWithPartition` static factory methods to replace
the existing constructors that take a partition. The idea being that
passing a partition is different enough that it should be called out
specifically.
3. Deprecate the existing constructors that take a partition so that we can
remove them (or make one of them private/protected) in a future release
Because ProducerRecord is used so widely, we should make sure that there is
real value in doing 2 and 3. Otherwise, we should stick to 1.
Ismael
On Fri, Apr 21, 2017 at 12:57 AM, Stephane Maarek <
stephane@simplemachines.com.au> wrote:
> Matthias: I was definitely on board with you at first, but Ismael made the
> comment that for:
>
> public ProducerRecord(String topic, K key, V value, Integer partition)
> public ProducerRecord(String topic, K key, V value, Long timestamp)
>
> Integer and Long are way too close in terms of meaning, and could provide
> a strong misuse of the timestamp / partition field.
> Therefore I started with a builder pattern for explicit argument
> declaration. Seems like a lot of boilerplate, but it makes things quite
> easy to understand.
>
> I like your point about the necessity of the key, and that users should
> set it to null explicitely.
>
> Damian: I like your idea of public ProducerRecordBuilder(String topic, V
> value)
> Finally, I also chose the withForcedPartition because in my learning of
> Kafka, I was always told that the key is solely the determining factor to
> know how a messages makes it to a partition. I find it incredibly
> unintuitive and dangerous to provide the users the ability to force a
> partition. If anything they should be providing their own key -> partition
> mapping, but I’m really against letting users force a partition within the
> producerRecord. What do you think?
>
>
> What do you both think of the more opiniated:
>
> public ProducerRecordBuilder(String topic, K key, V value)
>
> coming with withPartition and withTimestamp?
>
>
>
> On 21/4/17, 2:24 am, "Matthias J. Sax" <ma...@confluent.io> wrote:
>
> Thanks for the KIP!
>
> While I agree, that the current API is not perfect, I am not sure if a
> builder pattern does make sense here, because it's not too many
> parameters.
>
> IMHO, builder pattern makes sense if there are many optional
> parameters.
> For a ProducerRecord, I think there are only 2 optional parameters:
> partition and timestamp.
>
> I don't think key should be optional, because uses should be "forced"
> to
> think about the key argument as it effects the partitioning. Thus,
> providing an explicit `null` if there is no key seems reasonable to me.
>
> Overall I think that providing 3 overloads would be sufficient:
>
> > public ProducerRecord(String topic, K key, V value, Integer
> partition)
> > public ProducerRecord(String topic, K key, V value, Long timestamp)
> > public ProducerRecord(String topic, K key, V value, Integer
> partition, Long timestamp)
>
>
> Just my 2 cents.
>
> -Matthias
>
>
> On 4/20/17 4:20 AM, Damian Guy wrote:
> > Hi Stephane,
> >
> > Thanks for the KIP. Overall it looks ok, though i think the builder
> should
> > enforce the required parameters by supplying them via the
> constructor, i.e,
> >
> > public ProducerRecordBuilder(String topic, V value)
> >
> > You can then remove the withValue and withTopic methods
> >
> > I also think withForcedPartition should just be withPartition
> >
> > Thanks,
> > Damian
> >
> > On Wed, 19 Apr 2017 at 23:34 Stephane Maarek <
> stephane@simplemachines.com.au>
> > wrote:
> >
> >> Hi all,
> >>
> >> My first KIP, let me know your thoughts!
> >>
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> 141+-+ProducerRecordBuilder+Interface
> >>
> >>
> >> Cheers,
> >> Stephane
> >>
> >
>
>
>
>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Ismael Juma <is...@juma.me.uk>.
Thanks for the KIP. A possible alternative:
1. Add constructor ProducerRecord(String topic, K key, V value, Long
timestamp). This provides an unambiguous constructor that allows one to
pass a timestamp without a partition, which is the main requirement of the
KIP.
We could also consider:
2. Add a couple of `createWithPartition` static factory methods to replace
the existing constructors that take a partition. The idea being that
passing a partition is different enough that it should be called out
specifically.
3. Deprecate the existing constructors that take a partition so that we can
remove them (or make one of them private/protected) in a future release
Because ProducerRecord is used so widely, we should make sure that there is
real value in doing 2 and 3. Otherwise, we should stick to 1.
Ismael
On Fri, Apr 21, 2017 at 12:57 AM, Stephane Maarek <
stephane@simplemachines.com.au> wrote:
> Matthias: I was definitely on board with you at first, but Ismael made the
> comment that for:
>
> public ProducerRecord(String topic, K key, V value, Integer partition)
> public ProducerRecord(String topic, K key, V value, Long timestamp)
>
> Integer and Long are way too close in terms of meaning, and could provide
> a strong misuse of the timestamp / partition field.
> Therefore I started with a builder pattern for explicit argument
> declaration. Seems like a lot of boilerplate, but it makes things quite
> easy to understand.
>
> I like your point about the necessity of the key, and that users should
> set it to null explicitely.
>
> Damian: I like your idea of public ProducerRecordBuilder(String topic, V
> value)
> Finally, I also chose the withForcedPartition because in my learning of
> Kafka, I was always told that the key is solely the determining factor to
> know how a messages makes it to a partition. I find it incredibly
> unintuitive and dangerous to provide the users the ability to force a
> partition. If anything they should be providing their own key -> partition
> mapping, but I’m really against letting users force a partition within the
> producerRecord. What do you think?
>
>
> What do you both think of the more opiniated:
>
> public ProducerRecordBuilder(String topic, K key, V value)
>
> coming with withPartition and withTimestamp?
>
>
>
> On 21/4/17, 2:24 am, "Matthias J. Sax" <ma...@confluent.io> wrote:
>
> Thanks for the KIP!
>
> While I agree, that the current API is not perfect, I am not sure if a
> builder pattern does make sense here, because it's not too many
> parameters.
>
> IMHO, builder pattern makes sense if there are many optional
> parameters.
> For a ProducerRecord, I think there are only 2 optional parameters:
> partition and timestamp.
>
> I don't think key should be optional, because uses should be "forced"
> to
> think about the key argument as it effects the partitioning. Thus,
> providing an explicit `null` if there is no key seems reasonable to me.
>
> Overall I think that providing 3 overloads would be sufficient:
>
> > public ProducerRecord(String topic, K key, V value, Integer
> partition)
> > public ProducerRecord(String topic, K key, V value, Long timestamp)
> > public ProducerRecord(String topic, K key, V value, Integer
> partition, Long timestamp)
>
>
> Just my 2 cents.
>
> -Matthias
>
>
> On 4/20/17 4:20 AM, Damian Guy wrote:
> > Hi Stephane,
> >
> > Thanks for the KIP. Overall it looks ok, though i think the builder
> should
> > enforce the required parameters by supplying them via the
> constructor, i.e,
> >
> > public ProducerRecordBuilder(String topic, V value)
> >
> > You can then remove the withValue and withTopic methods
> >
> > I also think withForcedPartition should just be withPartition
> >
> > Thanks,
> > Damian
> >
> > On Wed, 19 Apr 2017 at 23:34 Stephane Maarek <
> stephane@simplemachines.com.au>
> > wrote:
> >
> >> Hi all,
> >>
> >> My first KIP, let me know your thoughts!
> >>
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> 141+-+ProducerRecordBuilder+Interface
> >>
> >>
> >> Cheers,
> >> Stephane
> >>
> >
>
>
>
>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Stephane Maarek <st...@simplemachines.com.au>.
Matthias: I was definitely on board with you at first, but Ismael made the comment that for:
public ProducerRecord(String topic, K key, V value, Integer partition)
public ProducerRecord(String topic, K key, V value, Long timestamp)
Integer and Long are way too close in terms of meaning, and could provide a strong misuse of the timestamp / partition field.
Therefore I started with a builder pattern for explicit argument declaration. Seems like a lot of boilerplate, but it makes things quite easy to understand.
I like your point about the necessity of the key, and that users should set it to null explicitely.
Damian: I like your idea of public ProducerRecordBuilder(String topic, V value)
Finally, I also chose the withForcedPartition because in my learning of Kafka, I was always told that the key is solely the determining factor to know how a messages makes it to a partition. I find it incredibly unintuitive and dangerous to provide the users the ability to force a partition. If anything they should be providing their own key -> partition mapping, but I’m really against letting users force a partition within the producerRecord. What do you think?
What do you both think of the more opiniated:
public ProducerRecordBuilder(String topic, K key, V value)
coming with withPartition and withTimestamp?
On 21/4/17, 2:24 am, "Matthias J. Sax" <ma...@confluent.io> wrote:
Thanks for the KIP!
While I agree, that the current API is not perfect, I am not sure if a
builder pattern does make sense here, because it's not too many parameters.
IMHO, builder pattern makes sense if there are many optional parameters.
For a ProducerRecord, I think there are only 2 optional parameters:
partition and timestamp.
I don't think key should be optional, because uses should be "forced" to
think about the key argument as it effects the partitioning. Thus,
providing an explicit `null` if there is no key seems reasonable to me.
Overall I think that providing 3 overloads would be sufficient:
> public ProducerRecord(String topic, K key, V value, Integer partition)
> public ProducerRecord(String topic, K key, V value, Long timestamp)
> public ProducerRecord(String topic, K key, V value, Integer partition, Long timestamp)
Just my 2 cents.
-Matthias
On 4/20/17 4:20 AM, Damian Guy wrote:
> Hi Stephane,
>
> Thanks for the KIP. Overall it looks ok, though i think the builder should
> enforce the required parameters by supplying them via the constructor, i.e,
>
> public ProducerRecordBuilder(String topic, V value)
>
> You can then remove the withValue and withTopic methods
>
> I also think withForcedPartition should just be withPartition
>
> Thanks,
> Damian
>
> On Wed, 19 Apr 2017 at 23:34 Stephane Maarek <st...@simplemachines.com.au>
> wrote:
>
>> Hi all,
>>
>> My first KIP, let me know your thoughts!
>>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+141+-+ProducerRecordBuilder+Interface
>>
>>
>> Cheers,
>> Stephane
>>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by "Matthias J. Sax" <ma...@confluent.io>.
Thanks for the KIP!
While I agree, that the current API is not perfect, I am not sure if a
builder pattern does make sense here, because it's not too many parameters.
IMHO, builder pattern makes sense if there are many optional parameters.
For a ProducerRecord, I think there are only 2 optional parameters:
partition and timestamp.
I don't think key should be optional, because uses should be "forced" to
think about the key argument as it effects the partitioning. Thus,
providing an explicit `null` if there is no key seems reasonable to me.
Overall I think that providing 3 overloads would be sufficient:
> public ProducerRecord(String topic, K key, V value, Integer partition)
> public ProducerRecord(String topic, K key, V value, Long timestamp)
> public ProducerRecord(String topic, K key, V value, Integer partition, Long timestamp)
Just my 2 cents.
-Matthias
On 4/20/17 4:20 AM, Damian Guy wrote:
> Hi Stephane,
>
> Thanks for the KIP. Overall it looks ok, though i think the builder should
> enforce the required parameters by supplying them via the constructor, i.e,
>
> public ProducerRecordBuilder(String topic, V value)
>
> You can then remove the withValue and withTopic methods
>
> I also think withForcedPartition should just be withPartition
>
> Thanks,
> Damian
>
> On Wed, 19 Apr 2017 at 23:34 Stephane Maarek <st...@simplemachines.com.au>
> wrote:
>
>> Hi all,
>>
>> My first KIP, let me know your thoughts!
>>
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+141+-+ProducerRecordBuilder+Interface
>>
>>
>> Cheers,
>> Stephane
>>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Damian Guy <da...@gmail.com>.
Hi Stephane,
Thanks for the KIP. Overall it looks ok, though i think the builder should
enforce the required parameters by supplying them via the constructor, i.e,
public ProducerRecordBuilder(String topic, V value)
You can then remove the withValue and withTopic methods
I also think withForcedPartition should just be withPartition
Thanks,
Damian
On Wed, 19 Apr 2017 at 23:34 Stephane Maarek <st...@simplemachines.com.au>
wrote:
> Hi all,
>
> My first KIP, let me know your thoughts!
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP+141+-+ProducerRecordBuilder+Interface
>
>
> Cheers,
> Stephane
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Michael André Pearce <mi...@me.com>.
If it's a choice of either or. I would vote keep as is. At least then people can write their own api wrappers easily with not many lines of code, like the one supplied.
Sent from my iPhone
> On 1 May 2017, at 18:34, Matthias J. Sax <ma...@confluent.io> wrote:
>
> Hi,
>
> I am personally not a big fan of providing two APIs to do the same
> thing. If we believe that one API is better than the other, we should
> indicate this by deprecating the old API IMHO.
>
> Just my two cents.
>
>
> -Matthias
>
>
>> On 4/30/17 11:05 PM, Michael Pearce wrote:
>> See
>>
>> https://hc.apache.org/httpcomponents-client-ga/tutorial/html/fluent.html
>>
>> Doesn't cause much issue over there where you have a fluent api wrapper for those who like that style, and the original more verbose api.
>>
>> Surely it would be better than removing a way of doing things that everyone has got used to and built their code around ala constructors approach. And simply provide a wrapper to provide a per field way of doing things.
>>
>>
>>
>>
>>
>> Sent using OWA for iPhone
>> ________________________________________
>> From: Stephane Maarek <st...@simplemachines.com.au>
>> Sent: Monday, May 1, 2017 6:37:44 AM
>> To: dev@kafka.apache.org
>> Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
>>
>> I’m not sure how people would feel about having two distinct methods to build the same object?
>> An API wrapper may be useful, but it doesn’t bring opinion about how one should program, that’s just driven by the docs.
>> I’m okay with that, but we need concensus
>>
>>
>> On 1/5/17, 6:08 am, "Michael Pearce" <Mi...@ig.com> wrote:
>>
>> Why not, instead of deprecating or removing whats there, as noted, its a point of preference, think about something that could wrap the existing, but provide an api that for you is cleaner?
>>
>> e.g. here's a sample idea building on a fluent api way. (this wraps the producer and producer records so no changes needed)
>>
>> https://gist.github.com/michaelandrepearce/de0f5ad4aa7d39d243781741c58c293e
>>
>> In future as new items further add to Producer Record, they just become new methods in the fluent API, as it builds the ProducerRecord using the most exhaustive constructor.
>>
>>
>>
>> ________________________________________
>> From: Matthias J. Sax <ma...@confluent.io>
>> Sent: Saturday, April 29, 2017 6:52 PM
>> To: dev@kafka.apache.org
>> Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
>>
>> I understand that we cannot just break stuff (btw: also not for
>> Streams!). But deprecating does not break anything, so I don't think
>> it's a big deal to change the API as long as we keep the old API as
>> deprecated.
>>
>>
>> -Matthias
>>
>>> On 4/29/17 9:28 AM, Jay Kreps wrote:
>>> Hey Matthias,
>>>
>>> Yeah I agree, I'm not against change as a general thing! I also think if
>>> you look back on the last two years, we completely rewrote the producer and
>>> consumer APIs, reworked the binary protocol many times over, and added the
>>> connector and stream processing apis, both major new additions. So I don't
>>> think we're in too much danger of stagnating!
>>>
>>> My two cents was just around breaking compatibility for trivial changes
>>> like constructor => builder. I think this only applies to the producer,
>>> consumer, and connect apis which are heavily embedded in hundreds of
>>> ecosystem components that depend on them. This is different from direct
>>> usage. If we break the streams api it is really no big deal---apps just
>>> need to rebuild when they upgrade, not the end of the world at all. However
>>> because many intermediate things depend on the Kafka producer you can cause
>>> these weird situations where your app depends on two third party things
>>> that use Kafka and each requires different, incompatible versions. We did
>>> this a lot in earlier versions of Kafka and it was the cause of much angst
>>> (and an ingrained general reluctance to upgrade) from our users.
>>>
>>> I still think we may have to break things, i just don't think we should do
>>> it for things like builders vs direct constructors which i think are kind
>>> of a debatable matter of taste.
>>>
>>> -Jay
>>>
>>>
>>>
>>> On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <ma...@confluent.io>
>>> wrote:
>>>
>>>> Hey Jay,
>>>>
>>>> I understand your concern, and for sure, we will need to keep the
>>>> current constructors deprecated for a long time (ie, many years).
>>>>
>>>> But if we don't make the move, we will not be able to improve. And I
>>>> think warnings about using deprecated APIs is an acceptable price to
>>>> pay. And the API improvements will help new people who adopt Kafka to
>>>> get started more easily.
>>>>
>>>> Otherwise Kafka might end up as many other enterprise software with a
>>>> lots of old stuff that is kept forever because nobody has the guts to
>>>> improve/change it.
>>>>
>>>> Of course, we can still improve the docs of the deprecated constructors,
>>>> too.
>>>>
>>>> Just my two cents.
>>>>
>>>>
>>>> -Matthias
>>>>
>>>>> On 4/23/17 3:37 PM, Jay Kreps wrote:
>>>>> Hey guys,
>>>>>
>>>>> I definitely think that the constructors could have been better designed,
>>>>> but I think given that they're in heavy use I don't think this proposal
>>>>> will improve things. Deprecating constructors just leaves everyone with
>>>>> lots of warnings and crossed out things. We can't actually delete the
>>>>> methods because lots of code needs to be usable across multiple Kafka
>>>>> versions, right? So we aren't picking between the original approach
>>>> (worse)
>>>>> and the new approach (better); what we are proposing is a perpetual
>>>>> mingling of the original style and the new style with a bunch of
>>>> deprecated
>>>>> stuff, which I think is worst of all.
>>>>>
>>>>> I'd vote for just documenting the meaning of null in the ProducerRecord
>>>>> constructor.
>>>>>
>>>>> -Jay
>>>>>
>>>>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
>>>>> stephane@simplemachines.com.au> wrote:
>>>>>
>>>>>> Hi all,
>>>>>>
>>>>>> My first KIP, let me know your thoughts!
>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
>>>>>> 141+-+ProducerRecordBuilder+Interface
>>>>>>
>>>>>>
>>>>>> Cheers,
>>>>>> Stephane
>>>>>>
>>>>>
>>>>
>>>>
>>>
>> The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
>>
>>
>>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Jay Kreps <ja...@confluent.io>.
Hey Matthias,
Yeah, I think we mostly agree. I think if we think the new way is a lot
better, then I agree keeping it deprecated for a long while then removing
is a viable path. To me this case just seemed like a minor thing so the 1+
years of having a deprecated way in common use and a new way and the
resulting confusion just seems unnecessary, especially since this is so
much in the common path of the most basic kafka usage. My experience with
other apis like MapReduce or Google Collections that did this was pretty
negative.
Big time agreement that doing both is not the right thing to do. People
aren't looking for 4 different ways of creating a ProducerRecord, that just
generates confusion.
-Jay
On Mon, May 1, 2017 at 10:34 AM, Matthias J. Sax <ma...@confluent.io>
wrote:
> Hi,
>
> I am personally not a big fan of providing two APIs to do the same
> thing. If we believe that one API is better than the other, we should
> indicate this by deprecating the old API IMHO.
>
> Just my two cents.
>
>
> -Matthias
>
>
> On 4/30/17 11:05 PM, Michael Pearce wrote:
> > See
> >
> > https://hc.apache.org/httpcomponents-client-ga/tutorial/html/fluent.html
> >
> > Doesn't cause much issue over there where you have a fluent api wrapper
> for those who like that style, and the original more verbose api.
> >
> > Surely it would be better than removing a way of doing things that
> everyone has got used to and built their code around ala constructors
> approach. And simply provide a wrapper to provide a per field way of doing
> things.
> >
> >
> >
> >
> >
> > Sent using OWA for iPhone
> > ________________________________________
> > From: Stephane Maarek <st...@simplemachines.com.au>
> > Sent: Monday, May 1, 2017 6:37:44 AM
> > To: dev@kafka.apache.org
> > Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
> >
> > I’m not sure how people would feel about having two distinct methods to
> build the same object?
> > An API wrapper may be useful, but it doesn’t bring opinion about how one
> should program, that’s just driven by the docs.
> > I’m okay with that, but we need concensus
> >
> >
> > On 1/5/17, 6:08 am, "Michael Pearce" <Mi...@ig.com> wrote:
> >
> > Why not, instead of deprecating or removing whats there, as noted,
> its a point of preference, think about something that could wrap the
> existing, but provide an api that for you is cleaner?
> >
> > e.g. here's a sample idea building on a fluent api way. (this wraps
> the producer and producer records so no changes needed)
> >
> > https://gist.github.com/michaelandrepearce/
> de0f5ad4aa7d39d243781741c58c293e
> >
> > In future as new items further add to Producer Record, they just
> become new methods in the fluent API, as it builds the ProducerRecord using
> the most exhaustive constructor.
> >
> >
> >
> > ________________________________________
> > From: Matthias J. Sax <ma...@confluent.io>
> > Sent: Saturday, April 29, 2017 6:52 PM
> > To: dev@kafka.apache.org
> > Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
> >
> > I understand that we cannot just break stuff (btw: also not for
> > Streams!). But deprecating does not break anything, so I don't think
> > it's a big deal to change the API as long as we keep the old API as
> > deprecated.
> >
> >
> > -Matthias
> >
> > On 4/29/17 9:28 AM, Jay Kreps wrote:
> > > Hey Matthias,
> > >
> > > Yeah I agree, I'm not against change as a general thing! I also
> think if
> > > you look back on the last two years, we completely rewrote the
> producer and
> > > consumer APIs, reworked the binary protocol many times over, and
> added the
> > > connector and stream processing apis, both major new additions. So
> I don't
> > > think we're in too much danger of stagnating!
> > >
> > > My two cents was just around breaking compatibility for trivial
> changes
> > > like constructor => builder. I think this only applies to the
> producer,
> > > consumer, and connect apis which are heavily embedded in hundreds
> of
> > > ecosystem components that depend on them. This is different from
> direct
> > > usage. If we break the streams api it is really no big deal---apps
> just
> > > need to rebuild when they upgrade, not the end of the world at
> all. However
> > > because many intermediate things depend on the Kafka producer you
> can cause
> > > these weird situations where your app depends on two third party
> things
> > > that use Kafka and each requires different, incompatible versions.
> We did
> > > this a lot in earlier versions of Kafka and it was the cause of
> much angst
> > > (and an ingrained general reluctance to upgrade) from our users.
> > >
> > > I still think we may have to break things, i just don't think we
> should do
> > > it for things like builders vs direct constructors which i think
> are kind
> > > of a debatable matter of taste.
> > >
> > > -Jay
> > >
> > >
> > >
> > > On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <
> matthias@confluent.io>
> > > wrote:
> > >
> > >> Hey Jay,
> > >>
> > >> I understand your concern, and for sure, we will need to keep the
> > >> current constructors deprecated for a long time (ie, many years).
> > >>
> > >> But if we don't make the move, we will not be able to improve.
> And I
> > >> think warnings about using deprecated APIs is an acceptable price
> to
> > >> pay. And the API improvements will help new people who adopt
> Kafka to
> > >> get started more easily.
> > >>
> > >> Otherwise Kafka might end up as many other enterprise software
> with a
> > >> lots of old stuff that is kept forever because nobody has the
> guts to
> > >> improve/change it.
> > >>
> > >> Of course, we can still improve the docs of the deprecated
> constructors,
> > >> too.
> > >>
> > >> Just my two cents.
> > >>
> > >>
> > >> -Matthias
> > >>
> > >> On 4/23/17 3:37 PM, Jay Kreps wrote:
> > >>> Hey guys,
> > >>>
> > >>> I definitely think that the constructors could have been better
> designed,
> > >>> but I think given that they're in heavy use I don't think this
> proposal
> > >>> will improve things. Deprecating constructors just leaves
> everyone with
> > >>> lots of warnings and crossed out things. We can't actually
> delete the
> > >>> methods because lots of code needs to be usable across multiple
> Kafka
> > >>> versions, right? So we aren't picking between the original
> approach
> > >> (worse)
> > >>> and the new approach (better); what we are proposing is a
> perpetual
> > >>> mingling of the original style and the new style with a bunch of
> > >> deprecated
> > >>> stuff, which I think is worst of all.
> > >>>
> > >>> I'd vote for just documenting the meaning of null in the
> ProducerRecord
> > >>> constructor.
> > >>>
> > >>> -Jay
> > >>>
> > >>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> > >>> stephane@simplemachines.com.au> wrote:
> > >>>
> > >>>> Hi all,
> > >>>>
> > >>>> My first KIP, let me know your thoughts!
> > >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> > >>>> 141+-+ProducerRecordBuilder+Interface
> > >>>>
> > >>>>
> > >>>> Cheers,
> > >>>> Stephane
> > >>>>
> > >>>
> > >>
> > >>
> > >
> > The information contained in this email is strictly confidential and
> for the use of the addressee only, unless otherwise indicated. If you are
> not the intended recipient, please do not read, copy, use or disclose to
> others this message or any attachment. Please also notify the sender by
> replying to this email or by telephone (+44(020 7896 0011) and then delete
> the email and any copies of it. Opinions, conclusion (etc) that do not
> relate to the official business of this company shall be understood as
> neither given nor endorsed by it. IG is a trading name of IG Markets
> Limited (a company registered in England and Wales, company number
> 04008957) and IG Index Limited (a company registered in England and Wales,
> company number 01190902). Registered address at Cannon Bridge House, 25
> Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number
> 195355) and IG Index Limited (register number 114059) are authorised and
> regulated by the Financial Conduct Authority.
> >
> >
> >
>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by "Matthias J. Sax" <ma...@confluent.io>.
Hi,
I am personally not a big fan of providing two APIs to do the same
thing. If we believe that one API is better than the other, we should
indicate this by deprecating the old API IMHO.
Just my two cents.
-Matthias
On 4/30/17 11:05 PM, Michael Pearce wrote:
> See
>
> https://hc.apache.org/httpcomponents-client-ga/tutorial/html/fluent.html
>
> Doesn't cause much issue over there where you have a fluent api wrapper for those who like that style, and the original more verbose api.
>
> Surely it would be better than removing a way of doing things that everyone has got used to and built their code around ala constructors approach. And simply provide a wrapper to provide a per field way of doing things.
>
>
>
>
>
> Sent using OWA for iPhone
> ________________________________________
> From: Stephane Maarek <st...@simplemachines.com.au>
> Sent: Monday, May 1, 2017 6:37:44 AM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
>
> I’m not sure how people would feel about having two distinct methods to build the same object?
> An API wrapper may be useful, but it doesn’t bring opinion about how one should program, that’s just driven by the docs.
> I’m okay with that, but we need concensus
>
>
> On 1/5/17, 6:08 am, "Michael Pearce" <Mi...@ig.com> wrote:
>
> Why not, instead of deprecating or removing whats there, as noted, its a point of preference, think about something that could wrap the existing, but provide an api that for you is cleaner?
>
> e.g. here's a sample idea building on a fluent api way. (this wraps the producer and producer records so no changes needed)
>
> https://gist.github.com/michaelandrepearce/de0f5ad4aa7d39d243781741c58c293e
>
> In future as new items further add to Producer Record, they just become new methods in the fluent API, as it builds the ProducerRecord using the most exhaustive constructor.
>
>
>
> ________________________________________
> From: Matthias J. Sax <ma...@confluent.io>
> Sent: Saturday, April 29, 2017 6:52 PM
> To: dev@kafka.apache.org
> Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
>
> I understand that we cannot just break stuff (btw: also not for
> Streams!). But deprecating does not break anything, so I don't think
> it's a big deal to change the API as long as we keep the old API as
> deprecated.
>
>
> -Matthias
>
> On 4/29/17 9:28 AM, Jay Kreps wrote:
> > Hey Matthias,
> >
> > Yeah I agree, I'm not against change as a general thing! I also think if
> > you look back on the last two years, we completely rewrote the producer and
> > consumer APIs, reworked the binary protocol many times over, and added the
> > connector and stream processing apis, both major new additions. So I don't
> > think we're in too much danger of stagnating!
> >
> > My two cents was just around breaking compatibility for trivial changes
> > like constructor => builder. I think this only applies to the producer,
> > consumer, and connect apis which are heavily embedded in hundreds of
> > ecosystem components that depend on them. This is different from direct
> > usage. If we break the streams api it is really no big deal---apps just
> > need to rebuild when they upgrade, not the end of the world at all. However
> > because many intermediate things depend on the Kafka producer you can cause
> > these weird situations where your app depends on two third party things
> > that use Kafka and each requires different, incompatible versions. We did
> > this a lot in earlier versions of Kafka and it was the cause of much angst
> > (and an ingrained general reluctance to upgrade) from our users.
> >
> > I still think we may have to break things, i just don't think we should do
> > it for things like builders vs direct constructors which i think are kind
> > of a debatable matter of taste.
> >
> > -Jay
> >
> >
> >
> > On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <ma...@confluent.io>
> > wrote:
> >
> >> Hey Jay,
> >>
> >> I understand your concern, and for sure, we will need to keep the
> >> current constructors deprecated for a long time (ie, many years).
> >>
> >> But if we don't make the move, we will not be able to improve. And I
> >> think warnings about using deprecated APIs is an acceptable price to
> >> pay. And the API improvements will help new people who adopt Kafka to
> >> get started more easily.
> >>
> >> Otherwise Kafka might end up as many other enterprise software with a
> >> lots of old stuff that is kept forever because nobody has the guts to
> >> improve/change it.
> >>
> >> Of course, we can still improve the docs of the deprecated constructors,
> >> too.
> >>
> >> Just my two cents.
> >>
> >>
> >> -Matthias
> >>
> >> On 4/23/17 3:37 PM, Jay Kreps wrote:
> >>> Hey guys,
> >>>
> >>> I definitely think that the constructors could have been better designed,
> >>> but I think given that they're in heavy use I don't think this proposal
> >>> will improve things. Deprecating constructors just leaves everyone with
> >>> lots of warnings and crossed out things. We can't actually delete the
> >>> methods because lots of code needs to be usable across multiple Kafka
> >>> versions, right? So we aren't picking between the original approach
> >> (worse)
> >>> and the new approach (better); what we are proposing is a perpetual
> >>> mingling of the original style and the new style with a bunch of
> >> deprecated
> >>> stuff, which I think is worst of all.
> >>>
> >>> I'd vote for just documenting the meaning of null in the ProducerRecord
> >>> constructor.
> >>>
> >>> -Jay
> >>>
> >>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> >>> stephane@simplemachines.com.au> wrote:
> >>>
> >>>> Hi all,
> >>>>
> >>>> My first KIP, let me know your thoughts!
> >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> >>>> 141+-+ProducerRecordBuilder+Interface
> >>>>
> >>>>
> >>>> Cheers,
> >>>> Stephane
> >>>>
> >>>
> >>
> >>
> >
> The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
>
>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Michael Pearce <Mi...@ig.com>.
See
https://hc.apache.org/httpcomponents-client-ga/tutorial/html/fluent.html
Doesn't cause much issue over there where you have a fluent api wrapper for those who like that style, and the original more verbose api.
Surely it would be better than removing a way of doing things that everyone has got used to and built their code around ala constructors approach. And simply provide a wrapper to provide a per field way of doing things.
Sent using OWA for iPhone
________________________________________
From: Stephane Maarek <st...@simplemachines.com.au>
Sent: Monday, May 1, 2017 6:37:44 AM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
I’m not sure how people would feel about having two distinct methods to build the same object?
An API wrapper may be useful, but it doesn’t bring opinion about how one should program, that’s just driven by the docs.
I’m okay with that, but we need concensus
On 1/5/17, 6:08 am, "Michael Pearce" <Mi...@ig.com> wrote:
Why not, instead of deprecating or removing whats there, as noted, its a point of preference, think about something that could wrap the existing, but provide an api that for you is cleaner?
e.g. here's a sample idea building on a fluent api way. (this wraps the producer and producer records so no changes needed)
https://gist.github.com/michaelandrepearce/de0f5ad4aa7d39d243781741c58c293e
In future as new items further add to Producer Record, they just become new methods in the fluent API, as it builds the ProducerRecord using the most exhaustive constructor.
________________________________________
From: Matthias J. Sax <ma...@confluent.io>
Sent: Saturday, April 29, 2017 6:52 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
I understand that we cannot just break stuff (btw: also not for
Streams!). But deprecating does not break anything, so I don't think
it's a big deal to change the API as long as we keep the old API as
deprecated.
-Matthias
On 4/29/17 9:28 AM, Jay Kreps wrote:
> Hey Matthias,
>
> Yeah I agree, I'm not against change as a general thing! I also think if
> you look back on the last two years, we completely rewrote the producer and
> consumer APIs, reworked the binary protocol many times over, and added the
> connector and stream processing apis, both major new additions. So I don't
> think we're in too much danger of stagnating!
>
> My two cents was just around breaking compatibility for trivial changes
> like constructor => builder. I think this only applies to the producer,
> consumer, and connect apis which are heavily embedded in hundreds of
> ecosystem components that depend on them. This is different from direct
> usage. If we break the streams api it is really no big deal---apps just
> need to rebuild when they upgrade, not the end of the world at all. However
> because many intermediate things depend on the Kafka producer you can cause
> these weird situations where your app depends on two third party things
> that use Kafka and each requires different, incompatible versions. We did
> this a lot in earlier versions of Kafka and it was the cause of much angst
> (and an ingrained general reluctance to upgrade) from our users.
>
> I still think we may have to break things, i just don't think we should do
> it for things like builders vs direct constructors which i think are kind
> of a debatable matter of taste.
>
> -Jay
>
>
>
> On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <ma...@confluent.io>
> wrote:
>
>> Hey Jay,
>>
>> I understand your concern, and for sure, we will need to keep the
>> current constructors deprecated for a long time (ie, many years).
>>
>> But if we don't make the move, we will not be able to improve. And I
>> think warnings about using deprecated APIs is an acceptable price to
>> pay. And the API improvements will help new people who adopt Kafka to
>> get started more easily.
>>
>> Otherwise Kafka might end up as many other enterprise software with a
>> lots of old stuff that is kept forever because nobody has the guts to
>> improve/change it.
>>
>> Of course, we can still improve the docs of the deprecated constructors,
>> too.
>>
>> Just my two cents.
>>
>>
>> -Matthias
>>
>> On 4/23/17 3:37 PM, Jay Kreps wrote:
>>> Hey guys,
>>>
>>> I definitely think that the constructors could have been better designed,
>>> but I think given that they're in heavy use I don't think this proposal
>>> will improve things. Deprecating constructors just leaves everyone with
>>> lots of warnings and crossed out things. We can't actually delete the
>>> methods because lots of code needs to be usable across multiple Kafka
>>> versions, right? So we aren't picking between the original approach
>> (worse)
>>> and the new approach (better); what we are proposing is a perpetual
>>> mingling of the original style and the new style with a bunch of
>> deprecated
>>> stuff, which I think is worst of all.
>>>
>>> I'd vote for just documenting the meaning of null in the ProducerRecord
>>> constructor.
>>>
>>> -Jay
>>>
>>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
>>> stephane@simplemachines.com.au> wrote:
>>>
>>>> Hi all,
>>>>
>>>> My first KIP, let me know your thoughts!
>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
>>>> 141+-+ProducerRecordBuilder+Interface
>>>>
>>>>
>>>> Cheers,
>>>> Stephane
>>>>
>>>
>>
>>
>
The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Stephane Maarek <st...@simplemachines.com.au>.
I’m not sure how people would feel about having two distinct methods to build the same object?
An API wrapper may be useful, but it doesn’t bring opinion about how one should program, that’s just driven by the docs.
I’m okay with that, but we need concensus
On 1/5/17, 6:08 am, "Michael Pearce" <Mi...@ig.com> wrote:
Why not, instead of deprecating or removing whats there, as noted, its a point of preference, think about something that could wrap the existing, but provide an api that for you is cleaner?
e.g. here's a sample idea building on a fluent api way. (this wraps the producer and producer records so no changes needed)
https://gist.github.com/michaelandrepearce/de0f5ad4aa7d39d243781741c58c293e
In future as new items further add to Producer Record, they just become new methods in the fluent API, as it builds the ProducerRecord using the most exhaustive constructor.
________________________________________
From: Matthias J. Sax <ma...@confluent.io>
Sent: Saturday, April 29, 2017 6:52 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
I understand that we cannot just break stuff (btw: also not for
Streams!). But deprecating does not break anything, so I don't think
it's a big deal to change the API as long as we keep the old API as
deprecated.
-Matthias
On 4/29/17 9:28 AM, Jay Kreps wrote:
> Hey Matthias,
>
> Yeah I agree, I'm not against change as a general thing! I also think if
> you look back on the last two years, we completely rewrote the producer and
> consumer APIs, reworked the binary protocol many times over, and added the
> connector and stream processing apis, both major new additions. So I don't
> think we're in too much danger of stagnating!
>
> My two cents was just around breaking compatibility for trivial changes
> like constructor => builder. I think this only applies to the producer,
> consumer, and connect apis which are heavily embedded in hundreds of
> ecosystem components that depend on them. This is different from direct
> usage. If we break the streams api it is really no big deal---apps just
> need to rebuild when they upgrade, not the end of the world at all. However
> because many intermediate things depend on the Kafka producer you can cause
> these weird situations where your app depends on two third party things
> that use Kafka and each requires different, incompatible versions. We did
> this a lot in earlier versions of Kafka and it was the cause of much angst
> (and an ingrained general reluctance to upgrade) from our users.
>
> I still think we may have to break things, i just don't think we should do
> it for things like builders vs direct constructors which i think are kind
> of a debatable matter of taste.
>
> -Jay
>
>
>
> On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <ma...@confluent.io>
> wrote:
>
>> Hey Jay,
>>
>> I understand your concern, and for sure, we will need to keep the
>> current constructors deprecated for a long time (ie, many years).
>>
>> But if we don't make the move, we will not be able to improve. And I
>> think warnings about using deprecated APIs is an acceptable price to
>> pay. And the API improvements will help new people who adopt Kafka to
>> get started more easily.
>>
>> Otherwise Kafka might end up as many other enterprise software with a
>> lots of old stuff that is kept forever because nobody has the guts to
>> improve/change it.
>>
>> Of course, we can still improve the docs of the deprecated constructors,
>> too.
>>
>> Just my two cents.
>>
>>
>> -Matthias
>>
>> On 4/23/17 3:37 PM, Jay Kreps wrote:
>>> Hey guys,
>>>
>>> I definitely think that the constructors could have been better designed,
>>> but I think given that they're in heavy use I don't think this proposal
>>> will improve things. Deprecating constructors just leaves everyone with
>>> lots of warnings and crossed out things. We can't actually delete the
>>> methods because lots of code needs to be usable across multiple Kafka
>>> versions, right? So we aren't picking between the original approach
>> (worse)
>>> and the new approach (better); what we are proposing is a perpetual
>>> mingling of the original style and the new style with a bunch of
>> deprecated
>>> stuff, which I think is worst of all.
>>>
>>> I'd vote for just documenting the meaning of null in the ProducerRecord
>>> constructor.
>>>
>>> -Jay
>>>
>>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
>>> stephane@simplemachines.com.au> wrote:
>>>
>>>> Hi all,
>>>>
>>>> My first KIP, let me know your thoughts!
>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
>>>> 141+-+ProducerRecordBuilder+Interface
>>>>
>>>>
>>>> Cheers,
>>>> Stephane
>>>>
>>>
>>
>>
>
The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Michael Pearce <Mi...@ig.com>.
Why not, instead of deprecating or removing whats there, as noted, its a point of preference, think about something that could wrap the existing, but provide an api that for you is cleaner?
e.g. here's a sample idea building on a fluent api way. (this wraps the producer and producer records so no changes needed)
https://gist.github.com/michaelandrepearce/de0f5ad4aa7d39d243781741c58c293e
In future as new items further add to Producer Record, they just become new methods in the fluent API, as it builds the ProducerRecord using the most exhaustive constructor.
________________________________________
From: Matthias J. Sax <ma...@confluent.io>
Sent: Saturday, April 29, 2017 6:52 PM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
I understand that we cannot just break stuff (btw: also not for
Streams!). But deprecating does not break anything, so I don't think
it's a big deal to change the API as long as we keep the old API as
deprecated.
-Matthias
On 4/29/17 9:28 AM, Jay Kreps wrote:
> Hey Matthias,
>
> Yeah I agree, I'm not against change as a general thing! I also think if
> you look back on the last two years, we completely rewrote the producer and
> consumer APIs, reworked the binary protocol many times over, and added the
> connector and stream processing apis, both major new additions. So I don't
> think we're in too much danger of stagnating!
>
> My two cents was just around breaking compatibility for trivial changes
> like constructor => builder. I think this only applies to the producer,
> consumer, and connect apis which are heavily embedded in hundreds of
> ecosystem components that depend on them. This is different from direct
> usage. If we break the streams api it is really no big deal---apps just
> need to rebuild when they upgrade, not the end of the world at all. However
> because many intermediate things depend on the Kafka producer you can cause
> these weird situations where your app depends on two third party things
> that use Kafka and each requires different, incompatible versions. We did
> this a lot in earlier versions of Kafka and it was the cause of much angst
> (and an ingrained general reluctance to upgrade) from our users.
>
> I still think we may have to break things, i just don't think we should do
> it for things like builders vs direct constructors which i think are kind
> of a debatable matter of taste.
>
> -Jay
>
>
>
> On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <ma...@confluent.io>
> wrote:
>
>> Hey Jay,
>>
>> I understand your concern, and for sure, we will need to keep the
>> current constructors deprecated for a long time (ie, many years).
>>
>> But if we don't make the move, we will not be able to improve. And I
>> think warnings about using deprecated APIs is an acceptable price to
>> pay. And the API improvements will help new people who adopt Kafka to
>> get started more easily.
>>
>> Otherwise Kafka might end up as many other enterprise software with a
>> lots of old stuff that is kept forever because nobody has the guts to
>> improve/change it.
>>
>> Of course, we can still improve the docs of the deprecated constructors,
>> too.
>>
>> Just my two cents.
>>
>>
>> -Matthias
>>
>> On 4/23/17 3:37 PM, Jay Kreps wrote:
>>> Hey guys,
>>>
>>> I definitely think that the constructors could have been better designed,
>>> but I think given that they're in heavy use I don't think this proposal
>>> will improve things. Deprecating constructors just leaves everyone with
>>> lots of warnings and crossed out things. We can't actually delete the
>>> methods because lots of code needs to be usable across multiple Kafka
>>> versions, right? So we aren't picking between the original approach
>> (worse)
>>> and the new approach (better); what we are proposing is a perpetual
>>> mingling of the original style and the new style with a bunch of
>> deprecated
>>> stuff, which I think is worst of all.
>>>
>>> I'd vote for just documenting the meaning of null in the ProducerRecord
>>> constructor.
>>>
>>> -Jay
>>>
>>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
>>> stephane@simplemachines.com.au> wrote:
>>>
>>>> Hi all,
>>>>
>>>> My first KIP, let me know your thoughts!
>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
>>>> 141+-+ProducerRecordBuilder+Interface
>>>>
>>>>
>>>> Cheers,
>>>> Stephane
>>>>
>>>
>>
>>
>
The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Andrew Coates <bi...@gmail.com>.
Thanks Mike, I was just about to do the same!
I think the benefit of the builder pattern, or the `with` pattern, is
greatly improved if you take interceptor implementations into account.
On Fri, 19 May 2017 at 08:29 Michael Pearce <Mi...@ig.com> wrote:
> And I just failed at a basic copy/paste exercise (bangs head on wall)
>
> Try again:
>
>
> > 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
> >
> >
>
> On 19/05/2017, 08:27, "Michael Pearce" <Mi...@ig.com> wrote:
>
> Just copying in from the mail list, another use case / +1 for the
> builder pattern, so its kept with the KIP thread.
>
>
>
> On 18/05/2017, 22:06, "Andrew Coates" <bi...@gmail.com>
> wrote:
>
> 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
>
>
> On 08/05/2017, 18:26, "Colin McCabe" <cm...@apache.org> wrote:
>
> Hadoop had a very similar issue in two places: in the constructor
> of
> MiniDFSCluster, and with the FileSystem#create API. In both cases,
> people kept adding more and more function overloads until the APIs
> got
> very ugly and hard to understand. This is especially the case
> when some
> of the parameters were just ints or other basic types. Do you
> need the
> Path, FsPermission, boolean, int, short, long, Progressable,
> InetSocketAddress[] overload or the Path, FsPermission,
> EnumSet<CreateFlag>, int, short, long, Progressable, ChecksumOpt
> one?
> Or one of the other literally dozen+ overloads? Ever since that
> experience I've been in favor of creating builder objects whenever
> it
> seems like you will end up in that situation.
>
> As Jay mentions, Hadoop also had a very difficult time deprecating
> APIs.
> We tried to deprecate org.apache.hadoop.fs.FileSystem in favor of
> org.apache.hadoop.fs.FileContext, and it just didn't take (people
> kept
> using the old API, and still are). A similar story could be told
> with
> the mapred vs. mapreduce APIs.
>
> There were a few issues that made this fail. The first was that
> there
> was a big installed base of existing programs, and people often
> used
> those existing programs as examples. And they all used the old
> APIs.
> The second is that all the projects built on top of Hadoop's APIs,
> like
> HBase, Spark, etc., wanted to use an API that could support
> multiple
> versions, which meant using the old API. Finally, the new APIs
> were not
> better for users.
>
> Hopefully, someday Java will get named arguments with default
> parameters, similar to Scala. Until that point, I think it's
> sensible
> to just use a builder whenever you are in the scenario "I have
> half a
> dozen arguments to this public API, and I think I might have half a
> dozen more in a year or two". So my vote would be to add the
> builders
> and keep the existing functions as well.
>
> best,
> Colin
>
>
> On Thu, May 4, 2017, at 21:23, Michael André Pearce wrote:
> > My vote would be with 2, then 3 then 1.
> >
> > Could I suggest maybe an option 4.
> >
> > that is option 2 but with a note that there is an intent in 1/2
> years
> > time to deprecate the old way (under another kip). This way books
> > materials can be updated over a period, code won't be making
> compile
> > warnings today. And also we can judge if two ways is really
> causing an
> > issue or not, but does hint to users to start actively using the
> new way.
> >
> > Think when the new Java clients were released they released as
> like a
> > beta, it was a release or two after that the old clients were
> deprecated,
> > and still to be removed.
> >
> > Sent from my iPhone
> >
> > > On 4 May 2017, at 21:18, Matthias J. Sax <
> matthias@confluent.io> wrote:
> > >
> > > We can go either way. I just pointed out, what I would prefer
> -- it's
> > > also quite subjective.
> > >
> > > The least invasive change would be to add new constructors and
> update
> > > the JavaDocs to point out the semantics of `partition`
> parameter.
> > >
> > > However, I still like the builder pattern: ProducerRecord has 6
> > > parameters with only 2 being mandatory (topic and either key
> or value).
> > > Thus, to have a complete set of overloads we would need many
> more
> > > constructors. Right now, it feels to be "incomplete" and as if
> the
> > > offered constructors got picked "randomly".
> > >
> > > I got convinced though, that deprecation is not strictly
> required for
> > > this change. If we go with option (2), it might be good to
> update the
> > > JavaDocs of the current API to point to the new one as
> "recommended to use".
> > >
> > >
> > >
> > > -Matthias
> > >
> > >
> > >> On 5/3/17 10:47 PM, Ewen Cheslack-Postava wrote:
> > >> Stephane,
> > >>
> > >> VOTES are really on-demand based on the author, but obviously
> it's good to
> > >> come to some level of consensus in the DISCUSS thread before
> initiating a
> > >> vote. I think the request for comments/votes on your 3
> options is a
> > >> reasonable way to gauge current opinions.
> > >>
> > >> For myself, I think either 1 or 3 are good options, and I
> think at least
> > >> Matthias & Jay are in agreement -- basically have one
> preferred, but
> > >> possibly support 2 approaches for awhile.
> > >>
> > >> I think 3 is the right way to go long term -- I don't expect
> so many more
> > >> built-in fields to be added, but then again I didn't expect
> this much churn
> > >> this quickly (headers were a surprise for me). We've gotten
> to enough
> > >> parameters that a builder is more effective. It sucks a bit
> for existing
> > >> users that rely on the constructors, but a full major release
> cycle (at the
> > >> minimum) is a pretty significant window, and we can always
> choose to extend
> > >> the window longer if we want to give people more time to
> transition. To me,
> > >> the biggest problem is all the tutorials and content that we
> *can't*
> > >> control -- there's a ton of code and tutorials out there that
> will still
> > >> reference the constructors, and those will last far longer
> than any
> > >> deprecation period we put in place.
> > >>
> > >> -Ewen
> > >>
> > >> On Wed, May 3, 2017 at 5:46 PM, Stephane Maarek <
> > >> stephane@simplemachines.com.au> wrote:
> > >>
> > >>> How do votes works?
> > >>>
> > >>> I feel there are 3 options right here, and I’d like a pre
> vote before a
> > >>> real vote?
> > >>> 1) Adding constructors. Could get messy over time,
> especially with headers
> > >>> coming into play, and future possible improvement to the
> message format
> > >>> 2) Adding a builder / nicer looking API (like fluent) to
> help build a
> > >>> ProducerRecord in a safe way. Issue here are two ways of
> building a
> > >>> ProducerRecord can bring confusion
> > >>> 3) Same as 2), but deprecating all the constructors. May be
> too much of an
> > >>> aggressive strategy
> > >>>
> > >>>
> > >>> I’m happy to go over 2), update the docs, and tell people
> this is the
> > >>> “preferred” way. Won’t outdate all the literature on Kafka,
> but I feel this
> > >>> set people up for success in the future.
> > >>> Thoughts / pre vote?
> > >>>
> > >>> On 3/5/17, 4:20 pm, "Ewen Cheslack-Postava" <
> ewen@confluent.io> wrote:
> > >>>
> > >>> I understand the convenience of pointing at a JIRA/PR,
> but can we put
> > >>> the
> > >>> concrete changes proposed in the JIRA (under "Proposed
> Changes"). I
> > >>> don't
> > >>> think voting on the KIP would be reasonable otherwise
> since the changes
> > >>> under vote could change arbitrarily...
> > >>>
> > >>> I'm increasingly skeptical of adding more convenience
> constructors --
> > >>> the
> > >>> current patch adds timestamps, we're about to add headers
> as well (for
> > >>> core, for Connect we have
> > >>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > >>> 145+-+Expose+Record+Headers+in+Kafka+Connect
> > >>> in flight). It just continues to get messier over time.
> > >>>
> > >>> I think builders in the right context are useful, as long
> as they
> > >>> exceed a
> > >>> certain number of parameters (SchemaBuilder in Connect is
> an artifact
> > >>> of
> > >>> that position). I don't think a transition period with 2
> ways to
> > >>> construct
> > >>> an object is actually a problem -- if there's always an
> "all N
> > >>> parameters"
> > >>> version of the constructor, all other constructors are
> just convenience
> > >>> shortcuts, but the Builder provides a shorthand.
> > >>>
> > >>> I also agree w/ Ismael that deprecating to aggressively
> is bad -- we
> > >>> added
> > >>> the APIs instead of a builder and there's not any real
> maintenance
> > >>> cost, so
> > >>> why add the deprecation? I don't want to suggest actually
> adding such
> > >>> an
> > >>> annotation, but the real issue here is that one API will
> become
> > >>> "preferred"
> > >>> for some time.
> > >>>
> > >>> -Ewen
> > >>>
> > >>>> On Tue, May 2, 2017 at 1:15 AM, Ismael Juma <
> ismael@juma.me.uk> wrote:
> > >>>>
> > >>>> Hi Matthias,
> > >>>>
> > >>>> Deprecating widely used APIs is a big deal. Build warnings
> are a
> > >>> nuisance
> > >>>> and can potentially break the build for those who have a
> > >>> zero-warnings
> > >>>> policy (which is good practice). It creates a bunch of busy
> work for
> > >>> our
> > >>>> users and various resources like books, blog posts, etc.
> become out
> > >>> of
> > >>>> date.
> > >>>>
> > >>>> This does not mean that we should not do it, but the
> benefit has to
> > >>> be
> > >>>> worth it and we should not do it lightly.
> > >>>>
> > >>>> Ismael
> > >>>>
> > >>>> On Sat, Apr 29, 2017 at 6:52 PM, Matthias J. Sax <
> > >>> matthias@confluent.io>
> > >>>> wrote:
> > >>>>
> > >>>>> I understand that we cannot just break stuff (btw: also
> not for
> > >>>>> Streams!). But deprecating does not break anything, so I
> don't
> > >>> think
> > >>>>> it's a big deal to change the API as long as we keep the
> old API as
> > >>>>> deprecated.
> > >>>>>
> > >>>>>
> > >>>>> -Matthias
> > >>>>>
> > >>>>>> On 4/29/17 9:28 AM, Jay Kreps wrote:
> > >>>>>> Hey Matthias,
> > >>>>>>
> > >>>>>> Yeah I agree, I'm not against change as a general thing!
> I also
> > >>> think
> > >>>> if
> > >>>>>> you look back on the last two years, we completely
> rewrote the
> > >>> producer
> > >>>>> and
> > >>>>>> consumer APIs, reworked the binary protocol many times
> over, and
> > >>> added
> > >>>>> the
> > >>>>>> connector and stream processing apis, both major new
> additions.
> > >>> So I
> > >>>>> don't
> > >>>>>> think we're in too much danger of stagnating!
> > >>>>>>
> > >>>>>> My two cents was just around breaking compatibility for
> trivial
> > >>> changes
> > >>>>>> like constructor => builder. I think this only applies to
> the
> > >>> producer,
> > >>>>>> consumer, and connect apis which are heavily embedded in
> > >>> hundreds of
> > >>>>>> ecosystem components that depend on them. This is
> different from
> > >>> direct
> > >>>>>> usage. If we break the streams api it is really no big
> > >>> deal---apps just
> > >>>>>> need to rebuild when they upgrade, not the end of the
> world at
> > >>> all.
> > >>>>> However
> > >>>>>> because many intermediate things depend on the Kafka
> producer
> > >>> you can
> > >>>>> cause
> > >>>>>> these weird situations where your app depends on two
> third party
> > >>> things
> > >>>>>> that use Kafka and each requires different, incompatible
> > >>> versions. We
> > >>>> did
> > >>>>>> this a lot in earlier versions of Kafka and it was the
> cause of
> > >>> much
> > >>>>> angst
> > >>>>>> (and an ingrained general reluctance to upgrade) from our
> users.
> > >>>>>>
> > >>>>>> I still think we may have to break things, i just don't
> think we
> > >>> should
> > >>>>> do
> > >>>>>> it for things like builders vs direct constructors which
> i think
> > >>> are
> > >>>> kind
> > >>>>>> of a debatable matter of taste.
> > >>>>>>
> > >>>>>> -Jay
> > >>>>>>
> > >>>>>>
> > >>>>>>
> > >>>>>> On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <
> > >>>> matthias@confluent.io>
> > >>>>>> wrote:
> > >>>>>>
> > >>>>>>> Hey Jay,
> > >>>>>>>
> > >>>>>>> I understand your concern, and for sure, we will need to
> keep
> > >>> the
> > >>>>>>> current constructors deprecated for a long time (ie, many
> > >>> years).
> > >>>>>>>
> > >>>>>>> But if we don't make the move, we will not be able to
> improve.
> > >>> And I
> > >>>>>>> think warnings about using deprecated APIs is an
> acceptable
> > >>> price to
> > >>>>>>> pay. And the API improvements will help new people who
> adopt
> > >>> Kafka to
> > >>>>>>> get started more easily.
> > >>>>>>>
> > >>>>>>> Otherwise Kafka might end up as many other enterprise
> software
> > >>> with a
> > >>>>>>> lots of old stuff that is kept forever because nobody
> has the
> > >>> guts to
> > >>>>>>> improve/change it.
> > >>>>>>>
> > >>>>>>> Of course, we can still improve the docs of the
> deprecated
> > >>>> constructors,
> > >>>>>>> too.
> > >>>>>>>
> > >>>>>>> Just my two cents.
> > >>>>>>>
> > >>>>>>>
> > >>>>>>> -Matthias
> > >>>>>>>
> > >>>>>>>> On 4/23/17 3:37 PM, Jay Kreps wrote:
> > >>>>>>>> Hey guys,
> > >>>>>>>>
> > >>>>>>>> I definitely think that the constructors could have
> been better
> > >>>>> designed,
> > >>>>>>>> but I think given that they're in heavy use I don't
> think this
> > >>>> proposal
> > >>>>>>>> will improve things. Deprecating constructors just
> leaves
> > >>> everyone
> > >>>> with
> > >>>>>>>> lots of warnings and crossed out things. We can't
> actually
> > >>> delete the
> > >>>>>>>> methods because lots of code needs to be usable across
> > >>> multiple Kafka
> > >>>>>>>> versions, right? So we aren't picking between the
> original
> > >>> approach
> > >>>>>>> (worse)
> > >>>>>>>> and the new approach (better); what we are proposing is
> a
> > >>> perpetual
> > >>>>>>>> mingling of the original style and the new style with a
> bunch
> > >>> of
> > >>>>>>> deprecated
> > >>>>>>>> stuff, which I think is worst of all.
> > >>>>>>>>
> > >>>>>>>> I'd vote for just documenting the meaning of null in the
> > >>>> ProducerRecord
> > >>>>>>>> constructor.
> > >>>>>>>>
> > >>>>>>>> -Jay
> > >>>>>>>>
> > >>>>>>>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> > >>>>>>>> stephane@simplemachines.com.au> wrote:
> > >>>>>>>>
> > >>>>>>>>> Hi all,
> > >>>>>>>>>
> > >>>>>>>>> My first KIP, let me know your thoughts!
> > >>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> > >>>>>>>>> 141+-+ProducerRecordBuilder+Interface
> > >>>>>>>>>
> > >>>>>>>>>
> > >>>>>>>>> Cheers,
> > >>>>>>>>> Stephane
> > >>>>>>>>>
> > >>>>>>>>
> > >>>>>>>
> > >>>>>>>
> > >>>>>>
> > >>>>>
> > >>>>>
> > >>>>
> > >>>
> > >>>
> > >>>
> > >>>
> > >>
> > >
>
>
> The information contained in this email is strictly confidential and
> for the use of the addressee only, unless otherwise indicated. If you are
> not the intended recipient, please do not read, copy, use or disclose to
> others this message or any attachment. Please also notify the sender by
> replying to this email or by telephone (+44(020 7896 0011) and then delete
> the email and any copies of it. Opinions, conclusion (etc) that do not
> relate to the official business of this company shall be understood as
> neither given nor endorsed by it. IG is a trading name of IG Markets
> Limited (a company registered in England and Wales, company number
> 04008957) and IG Index Limited (a company registered in England and Wales,
> company number 01190902). Registered address at Cannon Bridge House, 25
> Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number
> 195355) and IG Index Limited (register number 114059) are authorised and
> regulated by the Financial Conduct Authority.
>
>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Michael Pearce <Mi...@ig.com>.
And I just failed at a basic copy/paste exercise (bangs head on wall)
Try again:
> 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
>
>
On 19/05/2017, 08:27, "Michael Pearce" <Mi...@ig.com> wrote:
Just copying in from the mail list, another use case / +1 for the builder pattern, so its kept with the KIP thread.
On 18/05/2017, 22:06, "Andrew Coates" <bi...@gmail.com> wrote:
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
On 08/05/2017, 18:26, "Colin McCabe" <cm...@apache.org> wrote:
Hadoop had a very similar issue in two places: in the constructor of
MiniDFSCluster, and with the FileSystem#create API. In both cases,
people kept adding more and more function overloads until the APIs got
very ugly and hard to understand. This is especially the case when some
of the parameters were just ints or other basic types. Do you need the
Path, FsPermission, boolean, int, short, long, Progressable,
InetSocketAddress[] overload or the Path, FsPermission,
EnumSet<CreateFlag>, int, short, long, Progressable, ChecksumOpt one?
Or one of the other literally dozen+ overloads? Ever since that
experience I've been in favor of creating builder objects whenever it
seems like you will end up in that situation.
As Jay mentions, Hadoop also had a very difficult time deprecating APIs.
We tried to deprecate org.apache.hadoop.fs.FileSystem in favor of
org.apache.hadoop.fs.FileContext, and it just didn't take (people kept
using the old API, and still are). A similar story could be told with
the mapred vs. mapreduce APIs.
There were a few issues that made this fail. The first was that there
was a big installed base of existing programs, and people often used
those existing programs as examples. And they all used the old APIs.
The second is that all the projects built on top of Hadoop's APIs, like
HBase, Spark, etc., wanted to use an API that could support multiple
versions, which meant using the old API. Finally, the new APIs were not
better for users.
Hopefully, someday Java will get named arguments with default
parameters, similar to Scala. Until that point, I think it's sensible
to just use a builder whenever you are in the scenario "I have half a
dozen arguments to this public API, and I think I might have half a
dozen more in a year or two". So my vote would be to add the builders
and keep the existing functions as well.
best,
Colin
On Thu, May 4, 2017, at 21:23, Michael André Pearce wrote:
> My vote would be with 2, then 3 then 1.
>
> Could I suggest maybe an option 4.
>
> that is option 2 but with a note that there is an intent in 1/2 years
> time to deprecate the old way (under another kip). This way books
> materials can be updated over a period, code won't be making compile
> warnings today. And also we can judge if two ways is really causing an
> issue or not, but does hint to users to start actively using the new way.
>
> Think when the new Java clients were released they released as like a
> beta, it was a release or two after that the old clients were deprecated,
> and still to be removed.
>
> Sent from my iPhone
>
> > On 4 May 2017, at 21:18, Matthias J. Sax <ma...@confluent.io> wrote:
> >
> > We can go either way. I just pointed out, what I would prefer -- it's
> > also quite subjective.
> >
> > The least invasive change would be to add new constructors and update
> > the JavaDocs to point out the semantics of `partition` parameter.
> >
> > However, I still like the builder pattern: ProducerRecord has 6
> > parameters with only 2 being mandatory (topic and either key or value).
> > Thus, to have a complete set of overloads we would need many more
> > constructors. Right now, it feels to be "incomplete" and as if the
> > offered constructors got picked "randomly".
> >
> > I got convinced though, that deprecation is not strictly required for
> > this change. If we go with option (2), it might be good to update the
> > JavaDocs of the current API to point to the new one as "recommended to use".
> >
> >
> >
> > -Matthias
> >
> >
> >> On 5/3/17 10:47 PM, Ewen Cheslack-Postava wrote:
> >> Stephane,
> >>
> >> VOTES are really on-demand based on the author, but obviously it's good to
> >> come to some level of consensus in the DISCUSS thread before initiating a
> >> vote. I think the request for comments/votes on your 3 options is a
> >> reasonable way to gauge current opinions.
> >>
> >> For myself, I think either 1 or 3 are good options, and I think at least
> >> Matthias & Jay are in agreement -- basically have one preferred, but
> >> possibly support 2 approaches for awhile.
> >>
> >> I think 3 is the right way to go long term -- I don't expect so many more
> >> built-in fields to be added, but then again I didn't expect this much churn
> >> this quickly (headers were a surprise for me). We've gotten to enough
> >> parameters that a builder is more effective. It sucks a bit for existing
> >> users that rely on the constructors, but a full major release cycle (at the
> >> minimum) is a pretty significant window, and we can always choose to extend
> >> the window longer if we want to give people more time to transition. To me,
> >> the biggest problem is all the tutorials and content that we *can't*
> >> control -- there's a ton of code and tutorials out there that will still
> >> reference the constructors, and those will last far longer than any
> >> deprecation period we put in place.
> >>
> >> -Ewen
> >>
> >> On Wed, May 3, 2017 at 5:46 PM, Stephane Maarek <
> >> stephane@simplemachines.com.au> wrote:
> >>
> >>> How do votes works?
> >>>
> >>> I feel there are 3 options right here, and I’d like a pre vote before a
> >>> real vote?
> >>> 1) Adding constructors. Could get messy over time, especially with headers
> >>> coming into play, and future possible improvement to the message format
> >>> 2) Adding a builder / nicer looking API (like fluent) to help build a
> >>> ProducerRecord in a safe way. Issue here are two ways of building a
> >>> ProducerRecord can bring confusion
> >>> 3) Same as 2), but deprecating all the constructors. May be too much of an
> >>> aggressive strategy
> >>>
> >>>
> >>> I’m happy to go over 2), update the docs, and tell people this is the
> >>> “preferred” way. Won’t outdate all the literature on Kafka, but I feel this
> >>> set people up for success in the future.
> >>> Thoughts / pre vote?
> >>>
> >>> On 3/5/17, 4:20 pm, "Ewen Cheslack-Postava" <ew...@confluent.io> wrote:
> >>>
> >>> I understand the convenience of pointing at a JIRA/PR, but can we put
> >>> the
> >>> concrete changes proposed in the JIRA (under "Proposed Changes"). I
> >>> don't
> >>> think voting on the KIP would be reasonable otherwise since the changes
> >>> under vote could change arbitrarily...
> >>>
> >>> I'm increasingly skeptical of adding more convenience constructors --
> >>> the
> >>> current patch adds timestamps, we're about to add headers as well (for
> >>> core, for Connect we have
> >>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >>> 145+-+Expose+Record+Headers+in+Kafka+Connect
> >>> in flight). It just continues to get messier over time.
> >>>
> >>> I think builders in the right context are useful, as long as they
> >>> exceed a
> >>> certain number of parameters (SchemaBuilder in Connect is an artifact
> >>> of
> >>> that position). I don't think a transition period with 2 ways to
> >>> construct
> >>> an object is actually a problem -- if there's always an "all N
> >>> parameters"
> >>> version of the constructor, all other constructors are just convenience
> >>> shortcuts, but the Builder provides a shorthand.
> >>>
> >>> I also agree w/ Ismael that deprecating to aggressively is bad -- we
> >>> added
> >>> the APIs instead of a builder and there's not any real maintenance
> >>> cost, so
> >>> why add the deprecation? I don't want to suggest actually adding such
> >>> an
> >>> annotation, but the real issue here is that one API will become
> >>> "preferred"
> >>> for some time.
> >>>
> >>> -Ewen
> >>>
> >>>> On Tue, May 2, 2017 at 1:15 AM, Ismael Juma <is...@juma.me.uk> wrote:
> >>>>
> >>>> Hi Matthias,
> >>>>
> >>>> Deprecating widely used APIs is a big deal. Build warnings are a
> >>> nuisance
> >>>> and can potentially break the build for those who have a
> >>> zero-warnings
> >>>> policy (which is good practice). It creates a bunch of busy work for
> >>> our
> >>>> users and various resources like books, blog posts, etc. become out
> >>> of
> >>>> date.
> >>>>
> >>>> This does not mean that we should not do it, but the benefit has to
> >>> be
> >>>> worth it and we should not do it lightly.
> >>>>
> >>>> Ismael
> >>>>
> >>>> On Sat, Apr 29, 2017 at 6:52 PM, Matthias J. Sax <
> >>> matthias@confluent.io>
> >>>> wrote:
> >>>>
> >>>>> I understand that we cannot just break stuff (btw: also not for
> >>>>> Streams!). But deprecating does not break anything, so I don't
> >>> think
> >>>>> it's a big deal to change the API as long as we keep the old API as
> >>>>> deprecated.
> >>>>>
> >>>>>
> >>>>> -Matthias
> >>>>>
> >>>>>> On 4/29/17 9:28 AM, Jay Kreps wrote:
> >>>>>> Hey Matthias,
> >>>>>>
> >>>>>> Yeah I agree, I'm not against change as a general thing! I also
> >>> think
> >>>> if
> >>>>>> you look back on the last two years, we completely rewrote the
> >>> producer
> >>>>> and
> >>>>>> consumer APIs, reworked the binary protocol many times over, and
> >>> added
> >>>>> the
> >>>>>> connector and stream processing apis, both major new additions.
> >>> So I
> >>>>> don't
> >>>>>> think we're in too much danger of stagnating!
> >>>>>>
> >>>>>> My two cents was just around breaking compatibility for trivial
> >>> changes
> >>>>>> like constructor => builder. I think this only applies to the
> >>> producer,
> >>>>>> consumer, and connect apis which are heavily embedded in
> >>> hundreds of
> >>>>>> ecosystem components that depend on them. This is different from
> >>> direct
> >>>>>> usage. If we break the streams api it is really no big
> >>> deal---apps just
> >>>>>> need to rebuild when they upgrade, not the end of the world at
> >>> all.
> >>>>> However
> >>>>>> because many intermediate things depend on the Kafka producer
> >>> you can
> >>>>> cause
> >>>>>> these weird situations where your app depends on two third party
> >>> things
> >>>>>> that use Kafka and each requires different, incompatible
> >>> versions. We
> >>>> did
> >>>>>> this a lot in earlier versions of Kafka and it was the cause of
> >>> much
> >>>>> angst
> >>>>>> (and an ingrained general reluctance to upgrade) from our users.
> >>>>>>
> >>>>>> I still think we may have to break things, i just don't think we
> >>> should
> >>>>> do
> >>>>>> it for things like builders vs direct constructors which i think
> >>> are
> >>>> kind
> >>>>>> of a debatable matter of taste.
> >>>>>>
> >>>>>> -Jay
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <
> >>>> matthias@confluent.io>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> Hey Jay,
> >>>>>>>
> >>>>>>> I understand your concern, and for sure, we will need to keep
> >>> the
> >>>>>>> current constructors deprecated for a long time (ie, many
> >>> years).
> >>>>>>>
> >>>>>>> But if we don't make the move, we will not be able to improve.
> >>> And I
> >>>>>>> think warnings about using deprecated APIs is an acceptable
> >>> price to
> >>>>>>> pay. And the API improvements will help new people who adopt
> >>> Kafka to
> >>>>>>> get started more easily.
> >>>>>>>
> >>>>>>> Otherwise Kafka might end up as many other enterprise software
> >>> with a
> >>>>>>> lots of old stuff that is kept forever because nobody has the
> >>> guts to
> >>>>>>> improve/change it.
> >>>>>>>
> >>>>>>> Of course, we can still improve the docs of the deprecated
> >>>> constructors,
> >>>>>>> too.
> >>>>>>>
> >>>>>>> Just my two cents.
> >>>>>>>
> >>>>>>>
> >>>>>>> -Matthias
> >>>>>>>
> >>>>>>>> On 4/23/17 3:37 PM, Jay Kreps wrote:
> >>>>>>>> Hey guys,
> >>>>>>>>
> >>>>>>>> I definitely think that the constructors could have been better
> >>>>> designed,
> >>>>>>>> but I think given that they're in heavy use I don't think this
> >>>> proposal
> >>>>>>>> will improve things. Deprecating constructors just leaves
> >>> everyone
> >>>> with
> >>>>>>>> lots of warnings and crossed out things. We can't actually
> >>> delete the
> >>>>>>>> methods because lots of code needs to be usable across
> >>> multiple Kafka
> >>>>>>>> versions, right? So we aren't picking between the original
> >>> approach
> >>>>>>> (worse)
> >>>>>>>> and the new approach (better); what we are proposing is a
> >>> perpetual
> >>>>>>>> mingling of the original style and the new style with a bunch
> >>> of
> >>>>>>> deprecated
> >>>>>>>> stuff, which I think is worst of all.
> >>>>>>>>
> >>>>>>>> I'd vote for just documenting the meaning of null in the
> >>>> ProducerRecord
> >>>>>>>> constructor.
> >>>>>>>>
> >>>>>>>> -Jay
> >>>>>>>>
> >>>>>>>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> >>>>>>>> stephane@simplemachines.com.au> wrote:
> >>>>>>>>
> >>>>>>>>> Hi all,
> >>>>>>>>>
> >>>>>>>>> My first KIP, let me know your thoughts!
> >>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> >>>>>>>>> 141+-+ProducerRecordBuilder+Interface
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Cheers,
> >>>>>>>>> Stephane
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>
> >>>
> >>>
> >>>
> >>
> >
The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Michael Pearce <Mi...@ig.com>.
Just copying in from the mail list, another use case / +1 for the builder pattern, so its kept with the KIP thread.
On 18/05/2017, 22:06, "Andrew Coates" <bi...@gmail.com> wrote:
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
On 08/05/2017, 18:26, "Colin McCabe" <cm...@apache.org> wrote:
Hadoop had a very similar issue in two places: in the constructor of
MiniDFSCluster, and with the FileSystem#create API. In both cases,
people kept adding more and more function overloads until the APIs got
very ugly and hard to understand. This is especially the case when some
of the parameters were just ints or other basic types. Do you need the
Path, FsPermission, boolean, int, short, long, Progressable,
InetSocketAddress[] overload or the Path, FsPermission,
EnumSet<CreateFlag>, int, short, long, Progressable, ChecksumOpt one?
Or one of the other literally dozen+ overloads? Ever since that
experience I've been in favor of creating builder objects whenever it
seems like you will end up in that situation.
As Jay mentions, Hadoop also had a very difficult time deprecating APIs.
We tried to deprecate org.apache.hadoop.fs.FileSystem in favor of
org.apache.hadoop.fs.FileContext, and it just didn't take (people kept
using the old API, and still are). A similar story could be told with
the mapred vs. mapreduce APIs.
There were a few issues that made this fail. The first was that there
was a big installed base of existing programs, and people often used
those existing programs as examples. And they all used the old APIs.
The second is that all the projects built on top of Hadoop's APIs, like
HBase, Spark, etc., wanted to use an API that could support multiple
versions, which meant using the old API. Finally, the new APIs were not
better for users.
Hopefully, someday Java will get named arguments with default
parameters, similar to Scala. Until that point, I think it's sensible
to just use a builder whenever you are in the scenario "I have half a
dozen arguments to this public API, and I think I might have half a
dozen more in a year or two". So my vote would be to add the builders
and keep the existing functions as well.
best,
Colin
On Thu, May 4, 2017, at 21:23, Michael André Pearce wrote:
> My vote would be with 2, then 3 then 1.
>
> Could I suggest maybe an option 4.
>
> that is option 2 but with a note that there is an intent in 1/2 years
> time to deprecate the old way (under another kip). This way books
> materials can be updated over a period, code won't be making compile
> warnings today. And also we can judge if two ways is really causing an
> issue or not, but does hint to users to start actively using the new way.
>
> Think when the new Java clients were released they released as like a
> beta, it was a release or two after that the old clients were deprecated,
> and still to be removed.
>
> Sent from my iPhone
>
> > On 4 May 2017, at 21:18, Matthias J. Sax <ma...@confluent.io> wrote:
> >
> > We can go either way. I just pointed out, what I would prefer -- it's
> > also quite subjective.
> >
> > The least invasive change would be to add new constructors and update
> > the JavaDocs to point out the semantics of `partition` parameter.
> >
> > However, I still like the builder pattern: ProducerRecord has 6
> > parameters with only 2 being mandatory (topic and either key or value).
> > Thus, to have a complete set of overloads we would need many more
> > constructors. Right now, it feels to be "incomplete" and as if the
> > offered constructors got picked "randomly".
> >
> > I got convinced though, that deprecation is not strictly required for
> > this change. If we go with option (2), it might be good to update the
> > JavaDocs of the current API to point to the new one as "recommended to use".
> >
> >
> >
> > -Matthias
> >
> >
> >> On 5/3/17 10:47 PM, Ewen Cheslack-Postava wrote:
> >> Stephane,
> >>
> >> VOTES are really on-demand based on the author, but obviously it's good to
> >> come to some level of consensus in the DISCUSS thread before initiating a
> >> vote. I think the request for comments/votes on your 3 options is a
> >> reasonable way to gauge current opinions.
> >>
> >> For myself, I think either 1 or 3 are good options, and I think at least
> >> Matthias & Jay are in agreement -- basically have one preferred, but
> >> possibly support 2 approaches for awhile.
> >>
> >> I think 3 is the right way to go long term -- I don't expect so many more
> >> built-in fields to be added, but then again I didn't expect this much churn
> >> this quickly (headers were a surprise for me). We've gotten to enough
> >> parameters that a builder is more effective. It sucks a bit for existing
> >> users that rely on the constructors, but a full major release cycle (at the
> >> minimum) is a pretty significant window, and we can always choose to extend
> >> the window longer if we want to give people more time to transition. To me,
> >> the biggest problem is all the tutorials and content that we *can't*
> >> control -- there's a ton of code and tutorials out there that will still
> >> reference the constructors, and those will last far longer than any
> >> deprecation period we put in place.
> >>
> >> -Ewen
> >>
> >> On Wed, May 3, 2017 at 5:46 PM, Stephane Maarek <
> >> stephane@simplemachines.com.au> wrote:
> >>
> >>> How do votes works?
> >>>
> >>> I feel there are 3 options right here, and I’d like a pre vote before a
> >>> real vote?
> >>> 1) Adding constructors. Could get messy over time, especially with headers
> >>> coming into play, and future possible improvement to the message format
> >>> 2) Adding a builder / nicer looking API (like fluent) to help build a
> >>> ProducerRecord in a safe way. Issue here are two ways of building a
> >>> ProducerRecord can bring confusion
> >>> 3) Same as 2), but deprecating all the constructors. May be too much of an
> >>> aggressive strategy
> >>>
> >>>
> >>> I’m happy to go over 2), update the docs, and tell people this is the
> >>> “preferred” way. Won’t outdate all the literature on Kafka, but I feel this
> >>> set people up for success in the future.
> >>> Thoughts / pre vote?
> >>>
> >>> On 3/5/17, 4:20 pm, "Ewen Cheslack-Postava" <ew...@confluent.io> wrote:
> >>>
> >>> I understand the convenience of pointing at a JIRA/PR, but can we put
> >>> the
> >>> concrete changes proposed in the JIRA (under "Proposed Changes"). I
> >>> don't
> >>> think voting on the KIP would be reasonable otherwise since the changes
> >>> under vote could change arbitrarily...
> >>>
> >>> I'm increasingly skeptical of adding more convenience constructors --
> >>> the
> >>> current patch adds timestamps, we're about to add headers as well (for
> >>> core, for Connect we have
> >>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >>> 145+-+Expose+Record+Headers+in+Kafka+Connect
> >>> in flight). It just continues to get messier over time.
> >>>
> >>> I think builders in the right context are useful, as long as they
> >>> exceed a
> >>> certain number of parameters (SchemaBuilder in Connect is an artifact
> >>> of
> >>> that position). I don't think a transition period with 2 ways to
> >>> construct
> >>> an object is actually a problem -- if there's always an "all N
> >>> parameters"
> >>> version of the constructor, all other constructors are just convenience
> >>> shortcuts, but the Builder provides a shorthand.
> >>>
> >>> I also agree w/ Ismael that deprecating to aggressively is bad -- we
> >>> added
> >>> the APIs instead of a builder and there's not any real maintenance
> >>> cost, so
> >>> why add the deprecation? I don't want to suggest actually adding such
> >>> an
> >>> annotation, but the real issue here is that one API will become
> >>> "preferred"
> >>> for some time.
> >>>
> >>> -Ewen
> >>>
> >>>> On Tue, May 2, 2017 at 1:15 AM, Ismael Juma <is...@juma.me.uk> wrote:
> >>>>
> >>>> Hi Matthias,
> >>>>
> >>>> Deprecating widely used APIs is a big deal. Build warnings are a
> >>> nuisance
> >>>> and can potentially break the build for those who have a
> >>> zero-warnings
> >>>> policy (which is good practice). It creates a bunch of busy work for
> >>> our
> >>>> users and various resources like books, blog posts, etc. become out
> >>> of
> >>>> date.
> >>>>
> >>>> This does not mean that we should not do it, but the benefit has to
> >>> be
> >>>> worth it and we should not do it lightly.
> >>>>
> >>>> Ismael
> >>>>
> >>>> On Sat, Apr 29, 2017 at 6:52 PM, Matthias J. Sax <
> >>> matthias@confluent.io>
> >>>> wrote:
> >>>>
> >>>>> I understand that we cannot just break stuff (btw: also not for
> >>>>> Streams!). But deprecating does not break anything, so I don't
> >>> think
> >>>>> it's a big deal to change the API as long as we keep the old API as
> >>>>> deprecated.
> >>>>>
> >>>>>
> >>>>> -Matthias
> >>>>>
> >>>>>> On 4/29/17 9:28 AM, Jay Kreps wrote:
> >>>>>> Hey Matthias,
> >>>>>>
> >>>>>> Yeah I agree, I'm not against change as a general thing! I also
> >>> think
> >>>> if
> >>>>>> you look back on the last two years, we completely rewrote the
> >>> producer
> >>>>> and
> >>>>>> consumer APIs, reworked the binary protocol many times over, and
> >>> added
> >>>>> the
> >>>>>> connector and stream processing apis, both major new additions.
> >>> So I
> >>>>> don't
> >>>>>> think we're in too much danger of stagnating!
> >>>>>>
> >>>>>> My two cents was just around breaking compatibility for trivial
> >>> changes
> >>>>>> like constructor => builder. I think this only applies to the
> >>> producer,
> >>>>>> consumer, and connect apis which are heavily embedded in
> >>> hundreds of
> >>>>>> ecosystem components that depend on them. This is different from
> >>> direct
> >>>>>> usage. If we break the streams api it is really no big
> >>> deal---apps just
> >>>>>> need to rebuild when they upgrade, not the end of the world at
> >>> all.
> >>>>> However
> >>>>>> because many intermediate things depend on the Kafka producer
> >>> you can
> >>>>> cause
> >>>>>> these weird situations where your app depends on two third party
> >>> things
> >>>>>> that use Kafka and each requires different, incompatible
> >>> versions. We
> >>>> did
> >>>>>> this a lot in earlier versions of Kafka and it was the cause of
> >>> much
> >>>>> angst
> >>>>>> (and an ingrained general reluctance to upgrade) from our users.
> >>>>>>
> >>>>>> I still think we may have to break things, i just don't think we
> >>> should
> >>>>> do
> >>>>>> it for things like builders vs direct constructors which i think
> >>> are
> >>>> kind
> >>>>>> of a debatable matter of taste.
> >>>>>>
> >>>>>> -Jay
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <
> >>>> matthias@confluent.io>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> Hey Jay,
> >>>>>>>
> >>>>>>> I understand your concern, and for sure, we will need to keep
> >>> the
> >>>>>>> current constructors deprecated for a long time (ie, many
> >>> years).
> >>>>>>>
> >>>>>>> But if we don't make the move, we will not be able to improve.
> >>> And I
> >>>>>>> think warnings about using deprecated APIs is an acceptable
> >>> price to
> >>>>>>> pay. And the API improvements will help new people who adopt
> >>> Kafka to
> >>>>>>> get started more easily.
> >>>>>>>
> >>>>>>> Otherwise Kafka might end up as many other enterprise software
> >>> with a
> >>>>>>> lots of old stuff that is kept forever because nobody has the
> >>> guts to
> >>>>>>> improve/change it.
> >>>>>>>
> >>>>>>> Of course, we can still improve the docs of the deprecated
> >>>> constructors,
> >>>>>>> too.
> >>>>>>>
> >>>>>>> Just my two cents.
> >>>>>>>
> >>>>>>>
> >>>>>>> -Matthias
> >>>>>>>
> >>>>>>>> On 4/23/17 3:37 PM, Jay Kreps wrote:
> >>>>>>>> Hey guys,
> >>>>>>>>
> >>>>>>>> I definitely think that the constructors could have been better
> >>>>> designed,
> >>>>>>>> but I think given that they're in heavy use I don't think this
> >>>> proposal
> >>>>>>>> will improve things. Deprecating constructors just leaves
> >>> everyone
> >>>> with
> >>>>>>>> lots of warnings and crossed out things. We can't actually
> >>> delete the
> >>>>>>>> methods because lots of code needs to be usable across
> >>> multiple Kafka
> >>>>>>>> versions, right? So we aren't picking between the original
> >>> approach
> >>>>>>> (worse)
> >>>>>>>> and the new approach (better); what we are proposing is a
> >>> perpetual
> >>>>>>>> mingling of the original style and the new style with a bunch
> >>> of
> >>>>>>> deprecated
> >>>>>>>> stuff, which I think is worst of all.
> >>>>>>>>
> >>>>>>>> I'd vote for just documenting the meaning of null in the
> >>>> ProducerRecord
> >>>>>>>> constructor.
> >>>>>>>>
> >>>>>>>> -Jay
> >>>>>>>>
> >>>>>>>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> >>>>>>>> stephane@simplemachines.com.au> wrote:
> >>>>>>>>
> >>>>>>>>> Hi all,
> >>>>>>>>>
> >>>>>>>>> My first KIP, let me know your thoughts!
> >>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> >>>>>>>>> 141+-+ProducerRecordBuilder+Interface
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Cheers,
> >>>>>>>>> Stephane
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>
> >>>
> >>>
> >>>
> >>
> >
The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Colin McCabe <cm...@apache.org>.
Hadoop had a very similar issue in two places: in the constructor of
MiniDFSCluster, and with the FileSystem#create API. In both cases,
people kept adding more and more function overloads until the APIs got
very ugly and hard to understand. This is especially the case when some
of the parameters were just ints or other basic types. Do you need the
Path, FsPermission, boolean, int, short, long, Progressable,
InetSocketAddress[] overload or the Path, FsPermission,
EnumSet<CreateFlag>, int, short, long, Progressable, ChecksumOpt one?
Or one of the other literally dozen+ overloads? Ever since that
experience I've been in favor of creating builder objects whenever it
seems like you will end up in that situation.
As Jay mentions, Hadoop also had a very difficult time deprecating APIs.
We tried to deprecate org.apache.hadoop.fs.FileSystem in favor of
org.apache.hadoop.fs.FileContext, and it just didn't take (people kept
using the old API, and still are). A similar story could be told with
the mapred vs. mapreduce APIs.
There were a few issues that made this fail. The first was that there
was a big installed base of existing programs, and people often used
those existing programs as examples. And they all used the old APIs.
The second is that all the projects built on top of Hadoop's APIs, like
HBase, Spark, etc., wanted to use an API that could support multiple
versions, which meant using the old API. Finally, the new APIs were not
better for users.
Hopefully, someday Java will get named arguments with default
parameters, similar to Scala. Until that point, I think it's sensible
to just use a builder whenever you are in the scenario "I have half a
dozen arguments to this public API, and I think I might have half a
dozen more in a year or two". So my vote would be to add the builders
and keep the existing functions as well.
best,
Colin
On Thu, May 4, 2017, at 21:23, Michael André Pearce wrote:
> My vote would be with 2, then 3 then 1.
>
> Could I suggest maybe an option 4.
>
> that is option 2 but with a note that there is an intent in 1/2 years
> time to deprecate the old way (under another kip). This way books
> materials can be updated over a period, code won't be making compile
> warnings today. And also we can judge if two ways is really causing an
> issue or not, but does hint to users to start actively using the new way.
>
> Think when the new Java clients were released they released as like a
> beta, it was a release or two after that the old clients were deprecated,
> and still to be removed.
>
> Sent from my iPhone
>
> > On 4 May 2017, at 21:18, Matthias J. Sax <ma...@confluent.io> wrote:
> >
> > We can go either way. I just pointed out, what I would prefer -- it's
> > also quite subjective.
> >
> > The least invasive change would be to add new constructors and update
> > the JavaDocs to point out the semantics of `partition` parameter.
> >
> > However, I still like the builder pattern: ProducerRecord has 6
> > parameters with only 2 being mandatory (topic and either key or value).
> > Thus, to have a complete set of overloads we would need many more
> > constructors. Right now, it feels to be "incomplete" and as if the
> > offered constructors got picked "randomly".
> >
> > I got convinced though, that deprecation is not strictly required for
> > this change. If we go with option (2), it might be good to update the
> > JavaDocs of the current API to point to the new one as "recommended to use".
> >
> >
> >
> > -Matthias
> >
> >
> >> On 5/3/17 10:47 PM, Ewen Cheslack-Postava wrote:
> >> Stephane,
> >>
> >> VOTES are really on-demand based on the author, but obviously it's good to
> >> come to some level of consensus in the DISCUSS thread before initiating a
> >> vote. I think the request for comments/votes on your 3 options is a
> >> reasonable way to gauge current opinions.
> >>
> >> For myself, I think either 1 or 3 are good options, and I think at least
> >> Matthias & Jay are in agreement -- basically have one preferred, but
> >> possibly support 2 approaches for awhile.
> >>
> >> I think 3 is the right way to go long term -- I don't expect so many more
> >> built-in fields to be added, but then again I didn't expect this much churn
> >> this quickly (headers were a surprise for me). We've gotten to enough
> >> parameters that a builder is more effective. It sucks a bit for existing
> >> users that rely on the constructors, but a full major release cycle (at the
> >> minimum) is a pretty significant window, and we can always choose to extend
> >> the window longer if we want to give people more time to transition. To me,
> >> the biggest problem is all the tutorials and content that we *can't*
> >> control -- there's a ton of code and tutorials out there that will still
> >> reference the constructors, and those will last far longer than any
> >> deprecation period we put in place.
> >>
> >> -Ewen
> >>
> >> On Wed, May 3, 2017 at 5:46 PM, Stephane Maarek <
> >> stephane@simplemachines.com.au> wrote:
> >>
> >>> How do votes works?
> >>>
> >>> I feel there are 3 options right here, and I’d like a pre vote before a
> >>> real vote?
> >>> 1) Adding constructors. Could get messy over time, especially with headers
> >>> coming into play, and future possible improvement to the message format
> >>> 2) Adding a builder / nicer looking API (like fluent) to help build a
> >>> ProducerRecord in a safe way. Issue here are two ways of building a
> >>> ProducerRecord can bring confusion
> >>> 3) Same as 2), but deprecating all the constructors. May be too much of an
> >>> aggressive strategy
> >>>
> >>>
> >>> I’m happy to go over 2), update the docs, and tell people this is the
> >>> “preferred” way. Won’t outdate all the literature on Kafka, but I feel this
> >>> set people up for success in the future.
> >>> Thoughts / pre vote?
> >>>
> >>> On 3/5/17, 4:20 pm, "Ewen Cheslack-Postava" <ew...@confluent.io> wrote:
> >>>
> >>> I understand the convenience of pointing at a JIRA/PR, but can we put
> >>> the
> >>> concrete changes proposed in the JIRA (under "Proposed Changes"). I
> >>> don't
> >>> think voting on the KIP would be reasonable otherwise since the changes
> >>> under vote could change arbitrarily...
> >>>
> >>> I'm increasingly skeptical of adding more convenience constructors --
> >>> the
> >>> current patch adds timestamps, we're about to add headers as well (for
> >>> core, for Connect we have
> >>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >>> 145+-+Expose+Record+Headers+in+Kafka+Connect
> >>> in flight). It just continues to get messier over time.
> >>>
> >>> I think builders in the right context are useful, as long as they
> >>> exceed a
> >>> certain number of parameters (SchemaBuilder in Connect is an artifact
> >>> of
> >>> that position). I don't think a transition period with 2 ways to
> >>> construct
> >>> an object is actually a problem -- if there's always an "all N
> >>> parameters"
> >>> version of the constructor, all other constructors are just convenience
> >>> shortcuts, but the Builder provides a shorthand.
> >>>
> >>> I also agree w/ Ismael that deprecating to aggressively is bad -- we
> >>> added
> >>> the APIs instead of a builder and there's not any real maintenance
> >>> cost, so
> >>> why add the deprecation? I don't want to suggest actually adding such
> >>> an
> >>> annotation, but the real issue here is that one API will become
> >>> "preferred"
> >>> for some time.
> >>>
> >>> -Ewen
> >>>
> >>>> On Tue, May 2, 2017 at 1:15 AM, Ismael Juma <is...@juma.me.uk> wrote:
> >>>>
> >>>> Hi Matthias,
> >>>>
> >>>> Deprecating widely used APIs is a big deal. Build warnings are a
> >>> nuisance
> >>>> and can potentially break the build for those who have a
> >>> zero-warnings
> >>>> policy (which is good practice). It creates a bunch of busy work for
> >>> our
> >>>> users and various resources like books, blog posts, etc. become out
> >>> of
> >>>> date.
> >>>>
> >>>> This does not mean that we should not do it, but the benefit has to
> >>> be
> >>>> worth it and we should not do it lightly.
> >>>>
> >>>> Ismael
> >>>>
> >>>> On Sat, Apr 29, 2017 at 6:52 PM, Matthias J. Sax <
> >>> matthias@confluent.io>
> >>>> wrote:
> >>>>
> >>>>> I understand that we cannot just break stuff (btw: also not for
> >>>>> Streams!). But deprecating does not break anything, so I don't
> >>> think
> >>>>> it's a big deal to change the API as long as we keep the old API as
> >>>>> deprecated.
> >>>>>
> >>>>>
> >>>>> -Matthias
> >>>>>
> >>>>>> On 4/29/17 9:28 AM, Jay Kreps wrote:
> >>>>>> Hey Matthias,
> >>>>>>
> >>>>>> Yeah I agree, I'm not against change as a general thing! I also
> >>> think
> >>>> if
> >>>>>> you look back on the last two years, we completely rewrote the
> >>> producer
> >>>>> and
> >>>>>> consumer APIs, reworked the binary protocol many times over, and
> >>> added
> >>>>> the
> >>>>>> connector and stream processing apis, both major new additions.
> >>> So I
> >>>>> don't
> >>>>>> think we're in too much danger of stagnating!
> >>>>>>
> >>>>>> My two cents was just around breaking compatibility for trivial
> >>> changes
> >>>>>> like constructor => builder. I think this only applies to the
> >>> producer,
> >>>>>> consumer, and connect apis which are heavily embedded in
> >>> hundreds of
> >>>>>> ecosystem components that depend on them. This is different from
> >>> direct
> >>>>>> usage. If we break the streams api it is really no big
> >>> deal---apps just
> >>>>>> need to rebuild when they upgrade, not the end of the world at
> >>> all.
> >>>>> However
> >>>>>> because many intermediate things depend on the Kafka producer
> >>> you can
> >>>>> cause
> >>>>>> these weird situations where your app depends on two third party
> >>> things
> >>>>>> that use Kafka and each requires different, incompatible
> >>> versions. We
> >>>> did
> >>>>>> this a lot in earlier versions of Kafka and it was the cause of
> >>> much
> >>>>> angst
> >>>>>> (and an ingrained general reluctance to upgrade) from our users.
> >>>>>>
> >>>>>> I still think we may have to break things, i just don't think we
> >>> should
> >>>>> do
> >>>>>> it for things like builders vs direct constructors which i think
> >>> are
> >>>> kind
> >>>>>> of a debatable matter of taste.
> >>>>>>
> >>>>>> -Jay
> >>>>>>
> >>>>>>
> >>>>>>
> >>>>>> On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <
> >>>> matthias@confluent.io>
> >>>>>> wrote:
> >>>>>>
> >>>>>>> Hey Jay,
> >>>>>>>
> >>>>>>> I understand your concern, and for sure, we will need to keep
> >>> the
> >>>>>>> current constructors deprecated for a long time (ie, many
> >>> years).
> >>>>>>>
> >>>>>>> But if we don't make the move, we will not be able to improve.
> >>> And I
> >>>>>>> think warnings about using deprecated APIs is an acceptable
> >>> price to
> >>>>>>> pay. And the API improvements will help new people who adopt
> >>> Kafka to
> >>>>>>> get started more easily.
> >>>>>>>
> >>>>>>> Otherwise Kafka might end up as many other enterprise software
> >>> with a
> >>>>>>> lots of old stuff that is kept forever because nobody has the
> >>> guts to
> >>>>>>> improve/change it.
> >>>>>>>
> >>>>>>> Of course, we can still improve the docs of the deprecated
> >>>> constructors,
> >>>>>>> too.
> >>>>>>>
> >>>>>>> Just my two cents.
> >>>>>>>
> >>>>>>>
> >>>>>>> -Matthias
> >>>>>>>
> >>>>>>>> On 4/23/17 3:37 PM, Jay Kreps wrote:
> >>>>>>>> Hey guys,
> >>>>>>>>
> >>>>>>>> I definitely think that the constructors could have been better
> >>>>> designed,
> >>>>>>>> but I think given that they're in heavy use I don't think this
> >>>> proposal
> >>>>>>>> will improve things. Deprecating constructors just leaves
> >>> everyone
> >>>> with
> >>>>>>>> lots of warnings and crossed out things. We can't actually
> >>> delete the
> >>>>>>>> methods because lots of code needs to be usable across
> >>> multiple Kafka
> >>>>>>>> versions, right? So we aren't picking between the original
> >>> approach
> >>>>>>> (worse)
> >>>>>>>> and the new approach (better); what we are proposing is a
> >>> perpetual
> >>>>>>>> mingling of the original style and the new style with a bunch
> >>> of
> >>>>>>> deprecated
> >>>>>>>> stuff, which I think is worst of all.
> >>>>>>>>
> >>>>>>>> I'd vote for just documenting the meaning of null in the
> >>>> ProducerRecord
> >>>>>>>> constructor.
> >>>>>>>>
> >>>>>>>> -Jay
> >>>>>>>>
> >>>>>>>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> >>>>>>>> stephane@simplemachines.com.au> wrote:
> >>>>>>>>
> >>>>>>>>> Hi all,
> >>>>>>>>>
> >>>>>>>>> My first KIP, let me know your thoughts!
> >>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> >>>>>>>>> 141+-+ProducerRecordBuilder+Interface
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>> Cheers,
> >>>>>>>>> Stephane
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>
> >>>>>>>
> >>>>>>
> >>>>>
> >>>>>
> >>>>
> >>>
> >>>
> >>>
> >>>
> >>
> >
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Michael André Pearce <mi...@me.com>.
My vote would be with 2, then 3 then 1.
Could I suggest maybe an option 4.
that is option 2 but with a note that there is an intent in 1/2 years time to deprecate the old way (under another kip). This way books materials can be updated over a period, code won't be making compile warnings today. And also we can judge if two ways is really causing an issue or not, but does hint to users to start actively using the new way.
Think when the new Java clients were released they released as like a beta, it was a release or two after that the old clients were deprecated, and still to be removed.
Sent from my iPhone
> On 4 May 2017, at 21:18, Matthias J. Sax <ma...@confluent.io> wrote:
>
> We can go either way. I just pointed out, what I would prefer -- it's
> also quite subjective.
>
> The least invasive change would be to add new constructors and update
> the JavaDocs to point out the semantics of `partition` parameter.
>
> However, I still like the builder pattern: ProducerRecord has 6
> parameters with only 2 being mandatory (topic and either key or value).
> Thus, to have a complete set of overloads we would need many more
> constructors. Right now, it feels to be "incomplete" and as if the
> offered constructors got picked "randomly".
>
> I got convinced though, that deprecation is not strictly required for
> this change. If we go with option (2), it might be good to update the
> JavaDocs of the current API to point to the new one as "recommended to use".
>
>
>
> -Matthias
>
>
>> On 5/3/17 10:47 PM, Ewen Cheslack-Postava wrote:
>> Stephane,
>>
>> VOTES are really on-demand based on the author, but obviously it's good to
>> come to some level of consensus in the DISCUSS thread before initiating a
>> vote. I think the request for comments/votes on your 3 options is a
>> reasonable way to gauge current opinions.
>>
>> For myself, I think either 1 or 3 are good options, and I think at least
>> Matthias & Jay are in agreement -- basically have one preferred, but
>> possibly support 2 approaches for awhile.
>>
>> I think 3 is the right way to go long term -- I don't expect so many more
>> built-in fields to be added, but then again I didn't expect this much churn
>> this quickly (headers were a surprise for me). We've gotten to enough
>> parameters that a builder is more effective. It sucks a bit for existing
>> users that rely on the constructors, but a full major release cycle (at the
>> minimum) is a pretty significant window, and we can always choose to extend
>> the window longer if we want to give people more time to transition. To me,
>> the biggest problem is all the tutorials and content that we *can't*
>> control -- there's a ton of code and tutorials out there that will still
>> reference the constructors, and those will last far longer than any
>> deprecation period we put in place.
>>
>> -Ewen
>>
>> On Wed, May 3, 2017 at 5:46 PM, Stephane Maarek <
>> stephane@simplemachines.com.au> wrote:
>>
>>> How do votes works?
>>>
>>> I feel there are 3 options right here, and I’d like a pre vote before a
>>> real vote?
>>> 1) Adding constructors. Could get messy over time, especially with headers
>>> coming into play, and future possible improvement to the message format
>>> 2) Adding a builder / nicer looking API (like fluent) to help build a
>>> ProducerRecord in a safe way. Issue here are two ways of building a
>>> ProducerRecord can bring confusion
>>> 3) Same as 2), but deprecating all the constructors. May be too much of an
>>> aggressive strategy
>>>
>>>
>>> I’m happy to go over 2), update the docs, and tell people this is the
>>> “preferred” way. Won’t outdate all the literature on Kafka, but I feel this
>>> set people up for success in the future.
>>> Thoughts / pre vote?
>>>
>>> On 3/5/17, 4:20 pm, "Ewen Cheslack-Postava" <ew...@confluent.io> wrote:
>>>
>>> I understand the convenience of pointing at a JIRA/PR, but can we put
>>> the
>>> concrete changes proposed in the JIRA (under "Proposed Changes"). I
>>> don't
>>> think voting on the KIP would be reasonable otherwise since the changes
>>> under vote could change arbitrarily...
>>>
>>> I'm increasingly skeptical of adding more convenience constructors --
>>> the
>>> current patch adds timestamps, we're about to add headers as well (for
>>> core, for Connect we have
>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>>> 145+-+Expose+Record+Headers+in+Kafka+Connect
>>> in flight). It just continues to get messier over time.
>>>
>>> I think builders in the right context are useful, as long as they
>>> exceed a
>>> certain number of parameters (SchemaBuilder in Connect is an artifact
>>> of
>>> that position). I don't think a transition period with 2 ways to
>>> construct
>>> an object is actually a problem -- if there's always an "all N
>>> parameters"
>>> version of the constructor, all other constructors are just convenience
>>> shortcuts, but the Builder provides a shorthand.
>>>
>>> I also agree w/ Ismael that deprecating to aggressively is bad -- we
>>> added
>>> the APIs instead of a builder and there's not any real maintenance
>>> cost, so
>>> why add the deprecation? I don't want to suggest actually adding such
>>> an
>>> annotation, but the real issue here is that one API will become
>>> "preferred"
>>> for some time.
>>>
>>> -Ewen
>>>
>>>> On Tue, May 2, 2017 at 1:15 AM, Ismael Juma <is...@juma.me.uk> wrote:
>>>>
>>>> Hi Matthias,
>>>>
>>>> Deprecating widely used APIs is a big deal. Build warnings are a
>>> nuisance
>>>> and can potentially break the build for those who have a
>>> zero-warnings
>>>> policy (which is good practice). It creates a bunch of busy work for
>>> our
>>>> users and various resources like books, blog posts, etc. become out
>>> of
>>>> date.
>>>>
>>>> This does not mean that we should not do it, but the benefit has to
>>> be
>>>> worth it and we should not do it lightly.
>>>>
>>>> Ismael
>>>>
>>>> On Sat, Apr 29, 2017 at 6:52 PM, Matthias J. Sax <
>>> matthias@confluent.io>
>>>> wrote:
>>>>
>>>>> I understand that we cannot just break stuff (btw: also not for
>>>>> Streams!). But deprecating does not break anything, so I don't
>>> think
>>>>> it's a big deal to change the API as long as we keep the old API as
>>>>> deprecated.
>>>>>
>>>>>
>>>>> -Matthias
>>>>>
>>>>>> On 4/29/17 9:28 AM, Jay Kreps wrote:
>>>>>> Hey Matthias,
>>>>>>
>>>>>> Yeah I agree, I'm not against change as a general thing! I also
>>> think
>>>> if
>>>>>> you look back on the last two years, we completely rewrote the
>>> producer
>>>>> and
>>>>>> consumer APIs, reworked the binary protocol many times over, and
>>> added
>>>>> the
>>>>>> connector and stream processing apis, both major new additions.
>>> So I
>>>>> don't
>>>>>> think we're in too much danger of stagnating!
>>>>>>
>>>>>> My two cents was just around breaking compatibility for trivial
>>> changes
>>>>>> like constructor => builder. I think this only applies to the
>>> producer,
>>>>>> consumer, and connect apis which are heavily embedded in
>>> hundreds of
>>>>>> ecosystem components that depend on them. This is different from
>>> direct
>>>>>> usage. If we break the streams api it is really no big
>>> deal---apps just
>>>>>> need to rebuild when they upgrade, not the end of the world at
>>> all.
>>>>> However
>>>>>> because many intermediate things depend on the Kafka producer
>>> you can
>>>>> cause
>>>>>> these weird situations where your app depends on two third party
>>> things
>>>>>> that use Kafka and each requires different, incompatible
>>> versions. We
>>>> did
>>>>>> this a lot in earlier versions of Kafka and it was the cause of
>>> much
>>>>> angst
>>>>>> (and an ingrained general reluctance to upgrade) from our users.
>>>>>>
>>>>>> I still think we may have to break things, i just don't think we
>>> should
>>>>> do
>>>>>> it for things like builders vs direct constructors which i think
>>> are
>>>> kind
>>>>>> of a debatable matter of taste.
>>>>>>
>>>>>> -Jay
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <
>>>> matthias@confluent.io>
>>>>>> wrote:
>>>>>>
>>>>>>> Hey Jay,
>>>>>>>
>>>>>>> I understand your concern, and for sure, we will need to keep
>>> the
>>>>>>> current constructors deprecated for a long time (ie, many
>>> years).
>>>>>>>
>>>>>>> But if we don't make the move, we will not be able to improve.
>>> And I
>>>>>>> think warnings about using deprecated APIs is an acceptable
>>> price to
>>>>>>> pay. And the API improvements will help new people who adopt
>>> Kafka to
>>>>>>> get started more easily.
>>>>>>>
>>>>>>> Otherwise Kafka might end up as many other enterprise software
>>> with a
>>>>>>> lots of old stuff that is kept forever because nobody has the
>>> guts to
>>>>>>> improve/change it.
>>>>>>>
>>>>>>> Of course, we can still improve the docs of the deprecated
>>>> constructors,
>>>>>>> too.
>>>>>>>
>>>>>>> Just my two cents.
>>>>>>>
>>>>>>>
>>>>>>> -Matthias
>>>>>>>
>>>>>>>> On 4/23/17 3:37 PM, Jay Kreps wrote:
>>>>>>>> Hey guys,
>>>>>>>>
>>>>>>>> I definitely think that the constructors could have been better
>>>>> designed,
>>>>>>>> but I think given that they're in heavy use I don't think this
>>>> proposal
>>>>>>>> will improve things. Deprecating constructors just leaves
>>> everyone
>>>> with
>>>>>>>> lots of warnings and crossed out things. We can't actually
>>> delete the
>>>>>>>> methods because lots of code needs to be usable across
>>> multiple Kafka
>>>>>>>> versions, right? So we aren't picking between the original
>>> approach
>>>>>>> (worse)
>>>>>>>> and the new approach (better); what we are proposing is a
>>> perpetual
>>>>>>>> mingling of the original style and the new style with a bunch
>>> of
>>>>>>> deprecated
>>>>>>>> stuff, which I think is worst of all.
>>>>>>>>
>>>>>>>> I'd vote for just documenting the meaning of null in the
>>>> ProducerRecord
>>>>>>>> constructor.
>>>>>>>>
>>>>>>>> -Jay
>>>>>>>>
>>>>>>>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
>>>>>>>> stephane@simplemachines.com.au> wrote:
>>>>>>>>
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> My first KIP, let me know your thoughts!
>>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
>>>>>>>>> 141+-+ProducerRecordBuilder+Interface
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Cheers,
>>>>>>>>> Stephane
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>>
>>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by "Matthias J. Sax" <ma...@confluent.io>.
We can go either way. I just pointed out, what I would prefer -- it's
also quite subjective.
The least invasive change would be to add new constructors and update
the JavaDocs to point out the semantics of `partition` parameter.
However, I still like the builder pattern: ProducerRecord has 6
parameters with only 2 being mandatory (topic and either key or value).
Thus, to have a complete set of overloads we would need many more
constructors. Right now, it feels to be "incomplete" and as if the
offered constructors got picked "randomly".
I got convinced though, that deprecation is not strictly required for
this change. If we go with option (2), it might be good to update the
JavaDocs of the current API to point to the new one as "recommended to use".
-Matthias
On 5/3/17 10:47 PM, Ewen Cheslack-Postava wrote:
> Stephane,
>
> VOTES are really on-demand based on the author, but obviously it's good to
> come to some level of consensus in the DISCUSS thread before initiating a
> vote. I think the request for comments/votes on your 3 options is a
> reasonable way to gauge current opinions.
>
> For myself, I think either 1 or 3 are good options, and I think at least
> Matthias & Jay are in agreement -- basically have one preferred, but
> possibly support 2 approaches for awhile.
>
> I think 3 is the right way to go long term -- I don't expect so many more
> built-in fields to be added, but then again I didn't expect this much churn
> this quickly (headers were a surprise for me). We've gotten to enough
> parameters that a builder is more effective. It sucks a bit for existing
> users that rely on the constructors, but a full major release cycle (at the
> minimum) is a pretty significant window, and we can always choose to extend
> the window longer if we want to give people more time to transition. To me,
> the biggest problem is all the tutorials and content that we *can't*
> control -- there's a ton of code and tutorials out there that will still
> reference the constructors, and those will last far longer than any
> deprecation period we put in place.
>
> -Ewen
>
> On Wed, May 3, 2017 at 5:46 PM, Stephane Maarek <
> stephane@simplemachines.com.au> wrote:
>
>> How do votes works?
>>
>> I feel there are 3 options right here, and I’d like a pre vote before a
>> real vote?
>> 1) Adding constructors. Could get messy over time, especially with headers
>> coming into play, and future possible improvement to the message format
>> 2) Adding a builder / nicer looking API (like fluent) to help build a
>> ProducerRecord in a safe way. Issue here are two ways of building a
>> ProducerRecord can bring confusion
>> 3) Same as 2), but deprecating all the constructors. May be too much of an
>> aggressive strategy
>>
>>
>> I’m happy to go over 2), update the docs, and tell people this is the
>> “preferred” way. Won’t outdate all the literature on Kafka, but I feel this
>> set people up for success in the future.
>> Thoughts / pre vote?
>>
>> On 3/5/17, 4:20 pm, "Ewen Cheslack-Postava" <ew...@confluent.io> wrote:
>>
>> I understand the convenience of pointing at a JIRA/PR, but can we put
>> the
>> concrete changes proposed in the JIRA (under "Proposed Changes"). I
>> don't
>> think voting on the KIP would be reasonable otherwise since the changes
>> under vote could change arbitrarily...
>>
>> I'm increasingly skeptical of adding more convenience constructors --
>> the
>> current patch adds timestamps, we're about to add headers as well (for
>> core, for Connect we have
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> 145+-+Expose+Record+Headers+in+Kafka+Connect
>> in flight). It just continues to get messier over time.
>>
>> I think builders in the right context are useful, as long as they
>> exceed a
>> certain number of parameters (SchemaBuilder in Connect is an artifact
>> of
>> that position). I don't think a transition period with 2 ways to
>> construct
>> an object is actually a problem -- if there's always an "all N
>> parameters"
>> version of the constructor, all other constructors are just convenience
>> shortcuts, but the Builder provides a shorthand.
>>
>> I also agree w/ Ismael that deprecating to aggressively is bad -- we
>> added
>> the APIs instead of a builder and there's not any real maintenance
>> cost, so
>> why add the deprecation? I don't want to suggest actually adding such
>> an
>> annotation, but the real issue here is that one API will become
>> "preferred"
>> for some time.
>>
>> -Ewen
>>
>> On Tue, May 2, 2017 at 1:15 AM, Ismael Juma <is...@juma.me.uk> wrote:
>>
>> > Hi Matthias,
>> >
>> > Deprecating widely used APIs is a big deal. Build warnings are a
>> nuisance
>> > and can potentially break the build for those who have a
>> zero-warnings
>> > policy (which is good practice). It creates a bunch of busy work for
>> our
>> > users and various resources like books, blog posts, etc. become out
>> of
>> > date.
>> >
>> > This does not mean that we should not do it, but the benefit has to
>> be
>> > worth it and we should not do it lightly.
>> >
>> > Ismael
>> >
>> > On Sat, Apr 29, 2017 at 6:52 PM, Matthias J. Sax <
>> matthias@confluent.io>
>> > wrote:
>> >
>> > > I understand that we cannot just break stuff (btw: also not for
>> > > Streams!). But deprecating does not break anything, so I don't
>> think
>> > > it's a big deal to change the API as long as we keep the old API as
>> > > deprecated.
>> > >
>> > >
>> > > -Matthias
>> > >
>> > > On 4/29/17 9:28 AM, Jay Kreps wrote:
>> > > > Hey Matthias,
>> > > >
>> > > > Yeah I agree, I'm not against change as a general thing! I also
>> think
>> > if
>> > > > you look back on the last two years, we completely rewrote the
>> producer
>> > > and
>> > > > consumer APIs, reworked the binary protocol many times over, and
>> added
>> > > the
>> > > > connector and stream processing apis, both major new additions.
>> So I
>> > > don't
>> > > > think we're in too much danger of stagnating!
>> > > >
>> > > > My two cents was just around breaking compatibility for trivial
>> changes
>> > > > like constructor => builder. I think this only applies to the
>> producer,
>> > > > consumer, and connect apis which are heavily embedded in
>> hundreds of
>> > > > ecosystem components that depend on them. This is different from
>> direct
>> > > > usage. If we break the streams api it is really no big
>> deal---apps just
>> > > > need to rebuild when they upgrade, not the end of the world at
>> all.
>> > > However
>> > > > because many intermediate things depend on the Kafka producer
>> you can
>> > > cause
>> > > > these weird situations where your app depends on two third party
>> things
>> > > > that use Kafka and each requires different, incompatible
>> versions. We
>> > did
>> > > > this a lot in earlier versions of Kafka and it was the cause of
>> much
>> > > angst
>> > > > (and an ingrained general reluctance to upgrade) from our users.
>> > > >
>> > > > I still think we may have to break things, i just don't think we
>> should
>> > > do
>> > > > it for things like builders vs direct constructors which i think
>> are
>> > kind
>> > > > of a debatable matter of taste.
>> > > >
>> > > > -Jay
>> > > >
>> > > >
>> > > >
>> > > > On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <
>> > matthias@confluent.io>
>> > > > wrote:
>> > > >
>> > > >> Hey Jay,
>> > > >>
>> > > >> I understand your concern, and for sure, we will need to keep
>> the
>> > > >> current constructors deprecated for a long time (ie, many
>> years).
>> > > >>
>> > > >> But if we don't make the move, we will not be able to improve.
>> And I
>> > > >> think warnings about using deprecated APIs is an acceptable
>> price to
>> > > >> pay. And the API improvements will help new people who adopt
>> Kafka to
>> > > >> get started more easily.
>> > > >>
>> > > >> Otherwise Kafka might end up as many other enterprise software
>> with a
>> > > >> lots of old stuff that is kept forever because nobody has the
>> guts to
>> > > >> improve/change it.
>> > > >>
>> > > >> Of course, we can still improve the docs of the deprecated
>> > constructors,
>> > > >> too.
>> > > >>
>> > > >> Just my two cents.
>> > > >>
>> > > >>
>> > > >> -Matthias
>> > > >>
>> > > >> On 4/23/17 3:37 PM, Jay Kreps wrote:
>> > > >>> Hey guys,
>> > > >>>
>> > > >>> I definitely think that the constructors could have been better
>> > > designed,
>> > > >>> but I think given that they're in heavy use I don't think this
>> > proposal
>> > > >>> will improve things. Deprecating constructors just leaves
>> everyone
>> > with
>> > > >>> lots of warnings and crossed out things. We can't actually
>> delete the
>> > > >>> methods because lots of code needs to be usable across
>> multiple Kafka
>> > > >>> versions, right? So we aren't picking between the original
>> approach
>> > > >> (worse)
>> > > >>> and the new approach (better); what we are proposing is a
>> perpetual
>> > > >>> mingling of the original style and the new style with a bunch
>> of
>> > > >> deprecated
>> > > >>> stuff, which I think is worst of all.
>> > > >>>
>> > > >>> I'd vote for just documenting the meaning of null in the
>> > ProducerRecord
>> > > >>> constructor.
>> > > >>>
>> > > >>> -Jay
>> > > >>>
>> > > >>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
>> > > >>> stephane@simplemachines.com.au> wrote:
>> > > >>>
>> > > >>>> Hi all,
>> > > >>>>
>> > > >>>> My first KIP, let me know your thoughts!
>> > > >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
>> > > >>>> 141+-+ProducerRecordBuilder+Interface
>> > > >>>>
>> > > >>>>
>> > > >>>> Cheers,
>> > > >>>> Stephane
>> > > >>>>
>> > > >>>
>> > > >>
>> > > >>
>> > > >
>> > >
>> > >
>> >
>>
>>
>>
>>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Ewen Cheslack-Postava <ew...@confluent.io>.
Stephane,
VOTES are really on-demand based on the author, but obviously it's good to
come to some level of consensus in the DISCUSS thread before initiating a
vote. I think the request for comments/votes on your 3 options is a
reasonable way to gauge current opinions.
For myself, I think either 1 or 3 are good options, and I think at least
Matthias & Jay are in agreement -- basically have one preferred, but
possibly support 2 approaches for awhile.
I think 3 is the right way to go long term -- I don't expect so many more
built-in fields to be added, but then again I didn't expect this much churn
this quickly (headers were a surprise for me). We've gotten to enough
parameters that a builder is more effective. It sucks a bit for existing
users that rely on the constructors, but a full major release cycle (at the
minimum) is a pretty significant window, and we can always choose to extend
the window longer if we want to give people more time to transition. To me,
the biggest problem is all the tutorials and content that we *can't*
control -- there's a ton of code and tutorials out there that will still
reference the constructors, and those will last far longer than any
deprecation period we put in place.
-Ewen
On Wed, May 3, 2017 at 5:46 PM, Stephane Maarek <
stephane@simplemachines.com.au> wrote:
> How do votes works?
>
> I feel there are 3 options right here, and I’d like a pre vote before a
> real vote?
> 1) Adding constructors. Could get messy over time, especially with headers
> coming into play, and future possible improvement to the message format
> 2) Adding a builder / nicer looking API (like fluent) to help build a
> ProducerRecord in a safe way. Issue here are two ways of building a
> ProducerRecord can bring confusion
> 3) Same as 2), but deprecating all the constructors. May be too much of an
> aggressive strategy
>
>
> I’m happy to go over 2), update the docs, and tell people this is the
> “preferred” way. Won’t outdate all the literature on Kafka, but I feel this
> set people up for success in the future.
> Thoughts / pre vote?
>
> On 3/5/17, 4:20 pm, "Ewen Cheslack-Postava" <ew...@confluent.io> wrote:
>
> I understand the convenience of pointing at a JIRA/PR, but can we put
> the
> concrete changes proposed in the JIRA (under "Proposed Changes"). I
> don't
> think voting on the KIP would be reasonable otherwise since the changes
> under vote could change arbitrarily...
>
> I'm increasingly skeptical of adding more convenience constructors --
> the
> current patch adds timestamps, we're about to add headers as well (for
> core, for Connect we have
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 145+-+Expose+Record+Headers+in+Kafka+Connect
> in flight). It just continues to get messier over time.
>
> I think builders in the right context are useful, as long as they
> exceed a
> certain number of parameters (SchemaBuilder in Connect is an artifact
> of
> that position). I don't think a transition period with 2 ways to
> construct
> an object is actually a problem -- if there's always an "all N
> parameters"
> version of the constructor, all other constructors are just convenience
> shortcuts, but the Builder provides a shorthand.
>
> I also agree w/ Ismael that deprecating to aggressively is bad -- we
> added
> the APIs instead of a builder and there's not any real maintenance
> cost, so
> why add the deprecation? I don't want to suggest actually adding such
> an
> annotation, but the real issue here is that one API will become
> "preferred"
> for some time.
>
> -Ewen
>
> On Tue, May 2, 2017 at 1:15 AM, Ismael Juma <is...@juma.me.uk> wrote:
>
> > Hi Matthias,
> >
> > Deprecating widely used APIs is a big deal. Build warnings are a
> nuisance
> > and can potentially break the build for those who have a
> zero-warnings
> > policy (which is good practice). It creates a bunch of busy work for
> our
> > users and various resources like books, blog posts, etc. become out
> of
> > date.
> >
> > This does not mean that we should not do it, but the benefit has to
> be
> > worth it and we should not do it lightly.
> >
> > Ismael
> >
> > On Sat, Apr 29, 2017 at 6:52 PM, Matthias J. Sax <
> matthias@confluent.io>
> > wrote:
> >
> > > I understand that we cannot just break stuff (btw: also not for
> > > Streams!). But deprecating does not break anything, so I don't
> think
> > > it's a big deal to change the API as long as we keep the old API as
> > > deprecated.
> > >
> > >
> > > -Matthias
> > >
> > > On 4/29/17 9:28 AM, Jay Kreps wrote:
> > > > Hey Matthias,
> > > >
> > > > Yeah I agree, I'm not against change as a general thing! I also
> think
> > if
> > > > you look back on the last two years, we completely rewrote the
> producer
> > > and
> > > > consumer APIs, reworked the binary protocol many times over, and
> added
> > > the
> > > > connector and stream processing apis, both major new additions.
> So I
> > > don't
> > > > think we're in too much danger of stagnating!
> > > >
> > > > My two cents was just around breaking compatibility for trivial
> changes
> > > > like constructor => builder. I think this only applies to the
> producer,
> > > > consumer, and connect apis which are heavily embedded in
> hundreds of
> > > > ecosystem components that depend on them. This is different from
> direct
> > > > usage. If we break the streams api it is really no big
> deal---apps just
> > > > need to rebuild when they upgrade, not the end of the world at
> all.
> > > However
> > > > because many intermediate things depend on the Kafka producer
> you can
> > > cause
> > > > these weird situations where your app depends on two third party
> things
> > > > that use Kafka and each requires different, incompatible
> versions. We
> > did
> > > > this a lot in earlier versions of Kafka and it was the cause of
> much
> > > angst
> > > > (and an ingrained general reluctance to upgrade) from our users.
> > > >
> > > > I still think we may have to break things, i just don't think we
> should
> > > do
> > > > it for things like builders vs direct constructors which i think
> are
> > kind
> > > > of a debatable matter of taste.
> > > >
> > > > -Jay
> > > >
> > > >
> > > >
> > > > On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <
> > matthias@confluent.io>
> > > > wrote:
> > > >
> > > >> Hey Jay,
> > > >>
> > > >> I understand your concern, and for sure, we will need to keep
> the
> > > >> current constructors deprecated for a long time (ie, many
> years).
> > > >>
> > > >> But if we don't make the move, we will not be able to improve.
> And I
> > > >> think warnings about using deprecated APIs is an acceptable
> price to
> > > >> pay. And the API improvements will help new people who adopt
> Kafka to
> > > >> get started more easily.
> > > >>
> > > >> Otherwise Kafka might end up as many other enterprise software
> with a
> > > >> lots of old stuff that is kept forever because nobody has the
> guts to
> > > >> improve/change it.
> > > >>
> > > >> Of course, we can still improve the docs of the deprecated
> > constructors,
> > > >> too.
> > > >>
> > > >> Just my two cents.
> > > >>
> > > >>
> > > >> -Matthias
> > > >>
> > > >> On 4/23/17 3:37 PM, Jay Kreps wrote:
> > > >>> Hey guys,
> > > >>>
> > > >>> I definitely think that the constructors could have been better
> > > designed,
> > > >>> but I think given that they're in heavy use I don't think this
> > proposal
> > > >>> will improve things. Deprecating constructors just leaves
> everyone
> > with
> > > >>> lots of warnings and crossed out things. We can't actually
> delete the
> > > >>> methods because lots of code needs to be usable across
> multiple Kafka
> > > >>> versions, right? So we aren't picking between the original
> approach
> > > >> (worse)
> > > >>> and the new approach (better); what we are proposing is a
> perpetual
> > > >>> mingling of the original style and the new style with a bunch
> of
> > > >> deprecated
> > > >>> stuff, which I think is worst of all.
> > > >>>
> > > >>> I'd vote for just documenting the meaning of null in the
> > ProducerRecord
> > > >>> constructor.
> > > >>>
> > > >>> -Jay
> > > >>>
> > > >>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> > > >>> stephane@simplemachines.com.au> wrote:
> > > >>>
> > > >>>> Hi all,
> > > >>>>
> > > >>>> My first KIP, let me know your thoughts!
> > > >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> > > >>>> 141+-+ProducerRecordBuilder+Interface
> > > >>>>
> > > >>>>
> > > >>>> Cheers,
> > > >>>> Stephane
> > > >>>>
> > > >>>
> > > >>
> > > >>
> > > >
> > >
> > >
> >
>
>
>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Stephane Maarek <st...@simplemachines.com.au>.
How do votes works?
I feel there are 3 options right here, and I’d like a pre vote before a real vote?
1) Adding constructors. Could get messy over time, especially with headers coming into play, and future possible improvement to the message format
2) Adding a builder / nicer looking API (like fluent) to help build a ProducerRecord in a safe way. Issue here are two ways of building a ProducerRecord can bring confusion
3) Same as 2), but deprecating all the constructors. May be too much of an aggressive strategy
I’m happy to go over 2), update the docs, and tell people this is the “preferred” way. Won’t outdate all the literature on Kafka, but I feel this set people up for success in the future.
Thoughts / pre vote?
On 3/5/17, 4:20 pm, "Ewen Cheslack-Postava" <ew...@confluent.io> wrote:
I understand the convenience of pointing at a JIRA/PR, but can we put the
concrete changes proposed in the JIRA (under "Proposed Changes"). I don't
think voting on the KIP would be reasonable otherwise since the changes
under vote could change arbitrarily...
I'm increasingly skeptical of adding more convenience constructors -- the
current patch adds timestamps, we're about to add headers as well (for
core, for Connect we have
https://cwiki.apache.org/confluence/display/KAFKA/KIP-145+-+Expose+Record+Headers+in+Kafka+Connect
in flight). It just continues to get messier over time.
I think builders in the right context are useful, as long as they exceed a
certain number of parameters (SchemaBuilder in Connect is an artifact of
that position). I don't think a transition period with 2 ways to construct
an object is actually a problem -- if there's always an "all N parameters"
version of the constructor, all other constructors are just convenience
shortcuts, but the Builder provides a shorthand.
I also agree w/ Ismael that deprecating to aggressively is bad -- we added
the APIs instead of a builder and there's not any real maintenance cost, so
why add the deprecation? I don't want to suggest actually adding such an
annotation, but the real issue here is that one API will become "preferred"
for some time.
-Ewen
On Tue, May 2, 2017 at 1:15 AM, Ismael Juma <is...@juma.me.uk> wrote:
> Hi Matthias,
>
> Deprecating widely used APIs is a big deal. Build warnings are a nuisance
> and can potentially break the build for those who have a zero-warnings
> policy (which is good practice). It creates a bunch of busy work for our
> users and various resources like books, blog posts, etc. become out of
> date.
>
> This does not mean that we should not do it, but the benefit has to be
> worth it and we should not do it lightly.
>
> Ismael
>
> On Sat, Apr 29, 2017 at 6:52 PM, Matthias J. Sax <ma...@confluent.io>
> wrote:
>
> > I understand that we cannot just break stuff (btw: also not for
> > Streams!). But deprecating does not break anything, so I don't think
> > it's a big deal to change the API as long as we keep the old API as
> > deprecated.
> >
> >
> > -Matthias
> >
> > On 4/29/17 9:28 AM, Jay Kreps wrote:
> > > Hey Matthias,
> > >
> > > Yeah I agree, I'm not against change as a general thing! I also think
> if
> > > you look back on the last two years, we completely rewrote the producer
> > and
> > > consumer APIs, reworked the binary protocol many times over, and added
> > the
> > > connector and stream processing apis, both major new additions. So I
> > don't
> > > think we're in too much danger of stagnating!
> > >
> > > My two cents was just around breaking compatibility for trivial changes
> > > like constructor => builder. I think this only applies to the producer,
> > > consumer, and connect apis which are heavily embedded in hundreds of
> > > ecosystem components that depend on them. This is different from direct
> > > usage. If we break the streams api it is really no big deal---apps just
> > > need to rebuild when they upgrade, not the end of the world at all.
> > However
> > > because many intermediate things depend on the Kafka producer you can
> > cause
> > > these weird situations where your app depends on two third party things
> > > that use Kafka and each requires different, incompatible versions. We
> did
> > > this a lot in earlier versions of Kafka and it was the cause of much
> > angst
> > > (and an ingrained general reluctance to upgrade) from our users.
> > >
> > > I still think we may have to break things, i just don't think we should
> > do
> > > it for things like builders vs direct constructors which i think are
> kind
> > > of a debatable matter of taste.
> > >
> > > -Jay
> > >
> > >
> > >
> > > On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <
> matthias@confluent.io>
> > > wrote:
> > >
> > >> Hey Jay,
> > >>
> > >> I understand your concern, and for sure, we will need to keep the
> > >> current constructors deprecated for a long time (ie, many years).
> > >>
> > >> But if we don't make the move, we will not be able to improve. And I
> > >> think warnings about using deprecated APIs is an acceptable price to
> > >> pay. And the API improvements will help new people who adopt Kafka to
> > >> get started more easily.
> > >>
> > >> Otherwise Kafka might end up as many other enterprise software with a
> > >> lots of old stuff that is kept forever because nobody has the guts to
> > >> improve/change it.
> > >>
> > >> Of course, we can still improve the docs of the deprecated
> constructors,
> > >> too.
> > >>
> > >> Just my two cents.
> > >>
> > >>
> > >> -Matthias
> > >>
> > >> On 4/23/17 3:37 PM, Jay Kreps wrote:
> > >>> Hey guys,
> > >>>
> > >>> I definitely think that the constructors could have been better
> > designed,
> > >>> but I think given that they're in heavy use I don't think this
> proposal
> > >>> will improve things. Deprecating constructors just leaves everyone
> with
> > >>> lots of warnings and crossed out things. We can't actually delete the
> > >>> methods because lots of code needs to be usable across multiple Kafka
> > >>> versions, right? So we aren't picking between the original approach
> > >> (worse)
> > >>> and the new approach (better); what we are proposing is a perpetual
> > >>> mingling of the original style and the new style with a bunch of
> > >> deprecated
> > >>> stuff, which I think is worst of all.
> > >>>
> > >>> I'd vote for just documenting the meaning of null in the
> ProducerRecord
> > >>> constructor.
> > >>>
> > >>> -Jay
> > >>>
> > >>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> > >>> stephane@simplemachines.com.au> wrote:
> > >>>
> > >>>> Hi all,
> > >>>>
> > >>>> My first KIP, let me know your thoughts!
> > >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> > >>>> 141+-+ProducerRecordBuilder+Interface
> > >>>>
> > >>>>
> > >>>> Cheers,
> > >>>> Stephane
> > >>>>
> > >>>
> > >>
> > >>
> > >
> >
> >
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Ewen Cheslack-Postava <ew...@confluent.io>.
I understand the convenience of pointing at a JIRA/PR, but can we put the
concrete changes proposed in the JIRA (under "Proposed Changes"). I don't
think voting on the KIP would be reasonable otherwise since the changes
under vote could change arbitrarily...
I'm increasingly skeptical of adding more convenience constructors -- the
current patch adds timestamps, we're about to add headers as well (for
core, for Connect we have
https://cwiki.apache.org/confluence/display/KAFKA/KIP-145+-+Expose+Record+Headers+in+Kafka+Connect
in flight). It just continues to get messier over time.
I think builders in the right context are useful, as long as they exceed a
certain number of parameters (SchemaBuilder in Connect is an artifact of
that position). I don't think a transition period with 2 ways to construct
an object is actually a problem -- if there's always an "all N parameters"
version of the constructor, all other constructors are just convenience
shortcuts, but the Builder provides a shorthand.
I also agree w/ Ismael that deprecating to aggressively is bad -- we added
the APIs instead of a builder and there's not any real maintenance cost, so
why add the deprecation? I don't want to suggest actually adding such an
annotation, but the real issue here is that one API will become "preferred"
for some time.
-Ewen
On Tue, May 2, 2017 at 1:15 AM, Ismael Juma <is...@juma.me.uk> wrote:
> Hi Matthias,
>
> Deprecating widely used APIs is a big deal. Build warnings are a nuisance
> and can potentially break the build for those who have a zero-warnings
> policy (which is good practice). It creates a bunch of busy work for our
> users and various resources like books, blog posts, etc. become out of
> date.
>
> This does not mean that we should not do it, but the benefit has to be
> worth it and we should not do it lightly.
>
> Ismael
>
> On Sat, Apr 29, 2017 at 6:52 PM, Matthias J. Sax <ma...@confluent.io>
> wrote:
>
> > I understand that we cannot just break stuff (btw: also not for
> > Streams!). But deprecating does not break anything, so I don't think
> > it's a big deal to change the API as long as we keep the old API as
> > deprecated.
> >
> >
> > -Matthias
> >
> > On 4/29/17 9:28 AM, Jay Kreps wrote:
> > > Hey Matthias,
> > >
> > > Yeah I agree, I'm not against change as a general thing! I also think
> if
> > > you look back on the last two years, we completely rewrote the producer
> > and
> > > consumer APIs, reworked the binary protocol many times over, and added
> > the
> > > connector and stream processing apis, both major new additions. So I
> > don't
> > > think we're in too much danger of stagnating!
> > >
> > > My two cents was just around breaking compatibility for trivial changes
> > > like constructor => builder. I think this only applies to the producer,
> > > consumer, and connect apis which are heavily embedded in hundreds of
> > > ecosystem components that depend on them. This is different from direct
> > > usage. If we break the streams api it is really no big deal---apps just
> > > need to rebuild when they upgrade, not the end of the world at all.
> > However
> > > because many intermediate things depend on the Kafka producer you can
> > cause
> > > these weird situations where your app depends on two third party things
> > > that use Kafka and each requires different, incompatible versions. We
> did
> > > this a lot in earlier versions of Kafka and it was the cause of much
> > angst
> > > (and an ingrained general reluctance to upgrade) from our users.
> > >
> > > I still think we may have to break things, i just don't think we should
> > do
> > > it for things like builders vs direct constructors which i think are
> kind
> > > of a debatable matter of taste.
> > >
> > > -Jay
> > >
> > >
> > >
> > > On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <
> matthias@confluent.io>
> > > wrote:
> > >
> > >> Hey Jay,
> > >>
> > >> I understand your concern, and for sure, we will need to keep the
> > >> current constructors deprecated for a long time (ie, many years).
> > >>
> > >> But if we don't make the move, we will not be able to improve. And I
> > >> think warnings about using deprecated APIs is an acceptable price to
> > >> pay. And the API improvements will help new people who adopt Kafka to
> > >> get started more easily.
> > >>
> > >> Otherwise Kafka might end up as many other enterprise software with a
> > >> lots of old stuff that is kept forever because nobody has the guts to
> > >> improve/change it.
> > >>
> > >> Of course, we can still improve the docs of the deprecated
> constructors,
> > >> too.
> > >>
> > >> Just my two cents.
> > >>
> > >>
> > >> -Matthias
> > >>
> > >> On 4/23/17 3:37 PM, Jay Kreps wrote:
> > >>> Hey guys,
> > >>>
> > >>> I definitely think that the constructors could have been better
> > designed,
> > >>> but I think given that they're in heavy use I don't think this
> proposal
> > >>> will improve things. Deprecating constructors just leaves everyone
> with
> > >>> lots of warnings and crossed out things. We can't actually delete the
> > >>> methods because lots of code needs to be usable across multiple Kafka
> > >>> versions, right? So we aren't picking between the original approach
> > >> (worse)
> > >>> and the new approach (better); what we are proposing is a perpetual
> > >>> mingling of the original style and the new style with a bunch of
> > >> deprecated
> > >>> stuff, which I think is worst of all.
> > >>>
> > >>> I'd vote for just documenting the meaning of null in the
> ProducerRecord
> > >>> constructor.
> > >>>
> > >>> -Jay
> > >>>
> > >>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> > >>> stephane@simplemachines.com.au> wrote:
> > >>>
> > >>>> Hi all,
> > >>>>
> > >>>> My first KIP, let me know your thoughts!
> > >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> > >>>> 141+-+ProducerRecordBuilder+Interface
> > >>>>
> > >>>>
> > >>>> Cheers,
> > >>>> Stephane
> > >>>>
> > >>>
> > >>
> > >>
> > >
> >
> >
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Ismael Juma <is...@juma.me.uk>.
Hi Matthias,
Deprecating widely used APIs is a big deal. Build warnings are a nuisance
and can potentially break the build for those who have a zero-warnings
policy (which is good practice). It creates a bunch of busy work for our
users and various resources like books, blog posts, etc. become out of date.
This does not mean that we should not do it, but the benefit has to be
worth it and we should not do it lightly.
Ismael
On Sat, Apr 29, 2017 at 6:52 PM, Matthias J. Sax <ma...@confluent.io>
wrote:
> I understand that we cannot just break stuff (btw: also not for
> Streams!). But deprecating does not break anything, so I don't think
> it's a big deal to change the API as long as we keep the old API as
> deprecated.
>
>
> -Matthias
>
> On 4/29/17 9:28 AM, Jay Kreps wrote:
> > Hey Matthias,
> >
> > Yeah I agree, I'm not against change as a general thing! I also think if
> > you look back on the last two years, we completely rewrote the producer
> and
> > consumer APIs, reworked the binary protocol many times over, and added
> the
> > connector and stream processing apis, both major new additions. So I
> don't
> > think we're in too much danger of stagnating!
> >
> > My two cents was just around breaking compatibility for trivial changes
> > like constructor => builder. I think this only applies to the producer,
> > consumer, and connect apis which are heavily embedded in hundreds of
> > ecosystem components that depend on them. This is different from direct
> > usage. If we break the streams api it is really no big deal---apps just
> > need to rebuild when they upgrade, not the end of the world at all.
> However
> > because many intermediate things depend on the Kafka producer you can
> cause
> > these weird situations where your app depends on two third party things
> > that use Kafka and each requires different, incompatible versions. We did
> > this a lot in earlier versions of Kafka and it was the cause of much
> angst
> > (and an ingrained general reluctance to upgrade) from our users.
> >
> > I still think we may have to break things, i just don't think we should
> do
> > it for things like builders vs direct constructors which i think are kind
> > of a debatable matter of taste.
> >
> > -Jay
> >
> >
> >
> > On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <ma...@confluent.io>
> > wrote:
> >
> >> Hey Jay,
> >>
> >> I understand your concern, and for sure, we will need to keep the
> >> current constructors deprecated for a long time (ie, many years).
> >>
> >> But if we don't make the move, we will not be able to improve. And I
> >> think warnings about using deprecated APIs is an acceptable price to
> >> pay. And the API improvements will help new people who adopt Kafka to
> >> get started more easily.
> >>
> >> Otherwise Kafka might end up as many other enterprise software with a
> >> lots of old stuff that is kept forever because nobody has the guts to
> >> improve/change it.
> >>
> >> Of course, we can still improve the docs of the deprecated constructors,
> >> too.
> >>
> >> Just my two cents.
> >>
> >>
> >> -Matthias
> >>
> >> On 4/23/17 3:37 PM, Jay Kreps wrote:
> >>> Hey guys,
> >>>
> >>> I definitely think that the constructors could have been better
> designed,
> >>> but I think given that they're in heavy use I don't think this proposal
> >>> will improve things. Deprecating constructors just leaves everyone with
> >>> lots of warnings and crossed out things. We can't actually delete the
> >>> methods because lots of code needs to be usable across multiple Kafka
> >>> versions, right? So we aren't picking between the original approach
> >> (worse)
> >>> and the new approach (better); what we are proposing is a perpetual
> >>> mingling of the original style and the new style with a bunch of
> >> deprecated
> >>> stuff, which I think is worst of all.
> >>>
> >>> I'd vote for just documenting the meaning of null in the ProducerRecord
> >>> constructor.
> >>>
> >>> -Jay
> >>>
> >>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> >>> stephane@simplemachines.com.au> wrote:
> >>>
> >>>> Hi all,
> >>>>
> >>>> My first KIP, let me know your thoughts!
> >>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> >>>> 141+-+ProducerRecordBuilder+Interface
> >>>>
> >>>>
> >>>> Cheers,
> >>>> Stephane
> >>>>
> >>>
> >>
> >>
> >
>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by "Matthias J. Sax" <ma...@confluent.io>.
I understand that we cannot just break stuff (btw: also not for
Streams!). But deprecating does not break anything, so I don't think
it's a big deal to change the API as long as we keep the old API as
deprecated.
-Matthias
On 4/29/17 9:28 AM, Jay Kreps wrote:
> Hey Matthias,
>
> Yeah I agree, I'm not against change as a general thing! I also think if
> you look back on the last two years, we completely rewrote the producer and
> consumer APIs, reworked the binary protocol many times over, and added the
> connector and stream processing apis, both major new additions. So I don't
> think we're in too much danger of stagnating!
>
> My two cents was just around breaking compatibility for trivial changes
> like constructor => builder. I think this only applies to the producer,
> consumer, and connect apis which are heavily embedded in hundreds of
> ecosystem components that depend on them. This is different from direct
> usage. If we break the streams api it is really no big deal---apps just
> need to rebuild when they upgrade, not the end of the world at all. However
> because many intermediate things depend on the Kafka producer you can cause
> these weird situations where your app depends on two third party things
> that use Kafka and each requires different, incompatible versions. We did
> this a lot in earlier versions of Kafka and it was the cause of much angst
> (and an ingrained general reluctance to upgrade) from our users.
>
> I still think we may have to break things, i just don't think we should do
> it for things like builders vs direct constructors which i think are kind
> of a debatable matter of taste.
>
> -Jay
>
>
>
> On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <ma...@confluent.io>
> wrote:
>
>> Hey Jay,
>>
>> I understand your concern, and for sure, we will need to keep the
>> current constructors deprecated for a long time (ie, many years).
>>
>> But if we don't make the move, we will not be able to improve. And I
>> think warnings about using deprecated APIs is an acceptable price to
>> pay. And the API improvements will help new people who adopt Kafka to
>> get started more easily.
>>
>> Otherwise Kafka might end up as many other enterprise software with a
>> lots of old stuff that is kept forever because nobody has the guts to
>> improve/change it.
>>
>> Of course, we can still improve the docs of the deprecated constructors,
>> too.
>>
>> Just my two cents.
>>
>>
>> -Matthias
>>
>> On 4/23/17 3:37 PM, Jay Kreps wrote:
>>> Hey guys,
>>>
>>> I definitely think that the constructors could have been better designed,
>>> but I think given that they're in heavy use I don't think this proposal
>>> will improve things. Deprecating constructors just leaves everyone with
>>> lots of warnings and crossed out things. We can't actually delete the
>>> methods because lots of code needs to be usable across multiple Kafka
>>> versions, right? So we aren't picking between the original approach
>> (worse)
>>> and the new approach (better); what we are proposing is a perpetual
>>> mingling of the original style and the new style with a bunch of
>> deprecated
>>> stuff, which I think is worst of all.
>>>
>>> I'd vote for just documenting the meaning of null in the ProducerRecord
>>> constructor.
>>>
>>> -Jay
>>>
>>> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
>>> stephane@simplemachines.com.au> wrote:
>>>
>>>> Hi all,
>>>>
>>>> My first KIP, let me know your thoughts!
>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
>>>> 141+-+ProducerRecordBuilder+Interface
>>>>
>>>>
>>>> Cheers,
>>>> Stephane
>>>>
>>>
>>
>>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Jay Kreps <ja...@confluent.io>.
Hey Matthias,
Yeah I agree, I'm not against change as a general thing! I also think if
you look back on the last two years, we completely rewrote the producer and
consumer APIs, reworked the binary protocol many times over, and added the
connector and stream processing apis, both major new additions. So I don't
think we're in too much danger of stagnating!
My two cents was just around breaking compatibility for trivial changes
like constructor => builder. I think this only applies to the producer,
consumer, and connect apis which are heavily embedded in hundreds of
ecosystem components that depend on them. This is different from direct
usage. If we break the streams api it is really no big deal---apps just
need to rebuild when they upgrade, not the end of the world at all. However
because many intermediate things depend on the Kafka producer you can cause
these weird situations where your app depends on two third party things
that use Kafka and each requires different, incompatible versions. We did
this a lot in earlier versions of Kafka and it was the cause of much angst
(and an ingrained general reluctance to upgrade) from our users.
I still think we may have to break things, i just don't think we should do
it for things like builders vs direct constructors which i think are kind
of a debatable matter of taste.
-Jay
On Mon, Apr 24, 2017 at 9:40 AM, Matthias J. Sax <ma...@confluent.io>
wrote:
> Hey Jay,
>
> I understand your concern, and for sure, we will need to keep the
> current constructors deprecated for a long time (ie, many years).
>
> But if we don't make the move, we will not be able to improve. And I
> think warnings about using deprecated APIs is an acceptable price to
> pay. And the API improvements will help new people who adopt Kafka to
> get started more easily.
>
> Otherwise Kafka might end up as many other enterprise software with a
> lots of old stuff that is kept forever because nobody has the guts to
> improve/change it.
>
> Of course, we can still improve the docs of the deprecated constructors,
> too.
>
> Just my two cents.
>
>
> -Matthias
>
> On 4/23/17 3:37 PM, Jay Kreps wrote:
> > Hey guys,
> >
> > I definitely think that the constructors could have been better designed,
> > but I think given that they're in heavy use I don't think this proposal
> > will improve things. Deprecating constructors just leaves everyone with
> > lots of warnings and crossed out things. We can't actually delete the
> > methods because lots of code needs to be usable across multiple Kafka
> > versions, right? So we aren't picking between the original approach
> (worse)
> > and the new approach (better); what we are proposing is a perpetual
> > mingling of the original style and the new style with a bunch of
> deprecated
> > stuff, which I think is worst of all.
> >
> > I'd vote for just documenting the meaning of null in the ProducerRecord
> > constructor.
> >
> > -Jay
> >
> > On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> > stephane@simplemachines.com.au> wrote:
> >
> >> Hi all,
> >>
> >> My first KIP, let me know your thoughts!
> >> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> >> 141+-+ProducerRecordBuilder+Interface
> >>
> >>
> >> Cheers,
> >> Stephane
> >>
> >
>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Michael Pearce <Mi...@ig.com>.
This is ref to this section that's still there
"
The ProducerRecord constructors will all be deprecated, except the main long explicit one:
"
Sent using OWA for iPhone
________________________________________
From: Michael Pearce <Mi...@ig.com>
Sent: Saturday, April 29, 2017 6:17:04 AM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Probably should remove the words in the kip the other constructors are going to be deprecated if they aren't any more with just the tactical change
Sent using OWA for iPhone
________________________________________
From: Michael Pearce <Mi...@ig.com>
Sent: Saturday, April 29, 2017 6:04:00 AM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
+1
N.b. it is a shame we don't work on the contention to work a better way forwards.
________________________________________
From: Stephane Maarek <st...@simplemachines.com.au>
Sent: Saturday, April 29, 2017 4:39:32 AM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Hi
Thanks for all your input. My initial intent was to just add a “timestamp” constructor for the ProducerRecord, and it seems greater changes are too contentious.
I’ve just rolled back the PR to add the missing constructors, in the way Ismael suggested.
Updated KIP here: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=69406838
Added constructors here: https://github.com/apache/kafka/pull/2894
Let me know your thoughts,
Regards,
Stephane
On 25/4/17, 2:58 am, "Michael Pearce" <Mi...@ig.com> wrote:
Why not simply make a cleaner client fluent API wrapper? The internals use and send via current api, but provide a cleaner more fluent api.
A good example here is HTTP compontents where they did this.
https://hc.apache.org/httpcomponents-client-ga/tutorial/html/fluent.html
On 24/04/2017, 17:40, "Matthias J. Sax" <ma...@confluent.io> wrote:
Hey Jay,
I understand your concern, and for sure, we will need to keep the
current constructors deprecated for a long time (ie, many years).
But if we don't make the move, we will not be able to improve. And I
think warnings about using deprecated APIs is an acceptable price to
pay. And the API improvements will help new people who adopt Kafka to
get started more easily.
Otherwise Kafka might end up as many other enterprise software with a
lots of old stuff that is kept forever because nobody has the guts to
improve/change it.
Of course, we can still improve the docs of the deprecated constructors,
too.
Just my two cents.
-Matthias
On 4/23/17 3:37 PM, Jay Kreps wrote:
> Hey guys,
>
> I definitely think that the constructors could have been better designed,
> but I think given that they're in heavy use I don't think this proposal
> will improve things. Deprecating constructors just leaves everyone with
> lots of warnings and crossed out things. We can't actually delete the
> methods because lots of code needs to be usable across multiple Kafka
> versions, right? So we aren't picking between the original approach (worse)
> and the new approach (better); what we are proposing is a perpetual
> mingling of the original style and the new style with a bunch of deprecated
> stuff, which I think is worst of all.
>
> I'd vote for just documenting the meaning of null in the ProducerRecord
> constructor.
>
> -Jay
>
> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> stephane@simplemachines.com.au> wrote:
>
>> Hi all,
>>
>> My first KIP, let me know your thoughts!
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
>> 141+-+ProducerRecordBuilder+Interface
>>
>>
>> Cheers,
>> Stephane
>>
>
The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Michael Pearce <Mi...@ig.com>.
Probably should remove the words in the kip the other constructors are going to be deprecated if they aren't any more with just the tactical change
Sent using OWA for iPhone
________________________________________
From: Michael Pearce <Mi...@ig.com>
Sent: Saturday, April 29, 2017 6:04:00 AM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
+1
N.b. it is a shame we don't work on the contention to work a better way forwards.
________________________________________
From: Stephane Maarek <st...@simplemachines.com.au>
Sent: Saturday, April 29, 2017 4:39:32 AM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Hi
Thanks for all your input. My initial intent was to just add a “timestamp” constructor for the ProducerRecord, and it seems greater changes are too contentious.
I’ve just rolled back the PR to add the missing constructors, in the way Ismael suggested.
Updated KIP here: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=69406838
Added constructors here: https://github.com/apache/kafka/pull/2894
Let me know your thoughts,
Regards,
Stephane
On 25/4/17, 2:58 am, "Michael Pearce" <Mi...@ig.com> wrote:
Why not simply make a cleaner client fluent API wrapper? The internals use and send via current api, but provide a cleaner more fluent api.
A good example here is HTTP compontents where they did this.
https://hc.apache.org/httpcomponents-client-ga/tutorial/html/fluent.html
On 24/04/2017, 17:40, "Matthias J. Sax" <ma...@confluent.io> wrote:
Hey Jay,
I understand your concern, and for sure, we will need to keep the
current constructors deprecated for a long time (ie, many years).
But if we don't make the move, we will not be able to improve. And I
think warnings about using deprecated APIs is an acceptable price to
pay. And the API improvements will help new people who adopt Kafka to
get started more easily.
Otherwise Kafka might end up as many other enterprise software with a
lots of old stuff that is kept forever because nobody has the guts to
improve/change it.
Of course, we can still improve the docs of the deprecated constructors,
too.
Just my two cents.
-Matthias
On 4/23/17 3:37 PM, Jay Kreps wrote:
> Hey guys,
>
> I definitely think that the constructors could have been better designed,
> but I think given that they're in heavy use I don't think this proposal
> will improve things. Deprecating constructors just leaves everyone with
> lots of warnings and crossed out things. We can't actually delete the
> methods because lots of code needs to be usable across multiple Kafka
> versions, right? So we aren't picking between the original approach (worse)
> and the new approach (better); what we are proposing is a perpetual
> mingling of the original style and the new style with a bunch of deprecated
> stuff, which I think is worst of all.
>
> I'd vote for just documenting the meaning of null in the ProducerRecord
> constructor.
>
> -Jay
>
> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> stephane@simplemachines.com.au> wrote:
>
>> Hi all,
>>
>> My first KIP, let me know your thoughts!
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
>> 141+-+ProducerRecordBuilder+Interface
>>
>>
>> Cheers,
>> Stephane
>>
>
The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Michael Pearce <Mi...@ig.com>.
+1
N.b. it is a shame we don't work on the contention to work a better way forwards.
________________________________________
From: Stephane Maarek <st...@simplemachines.com.au>
Sent: Saturday, April 29, 2017 4:39:32 AM
To: dev@kafka.apache.org
Subject: Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Hi
Thanks for all your input. My initial intent was to just add a “timestamp” constructor for the ProducerRecord, and it seems greater changes are too contentious.
I’ve just rolled back the PR to add the missing constructors, in the way Ismael suggested.
Updated KIP here: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=69406838
Added constructors here: https://github.com/apache/kafka/pull/2894
Let me know your thoughts,
Regards,
Stephane
On 25/4/17, 2:58 am, "Michael Pearce" <Mi...@ig.com> wrote:
Why not simply make a cleaner client fluent API wrapper? The internals use and send via current api, but provide a cleaner more fluent api.
A good example here is HTTP compontents where they did this.
https://hc.apache.org/httpcomponents-client-ga/tutorial/html/fluent.html
On 24/04/2017, 17:40, "Matthias J. Sax" <ma...@confluent.io> wrote:
Hey Jay,
I understand your concern, and for sure, we will need to keep the
current constructors deprecated for a long time (ie, many years).
But if we don't make the move, we will not be able to improve. And I
think warnings about using deprecated APIs is an acceptable price to
pay. And the API improvements will help new people who adopt Kafka to
get started more easily.
Otherwise Kafka might end up as many other enterprise software with a
lots of old stuff that is kept forever because nobody has the guts to
improve/change it.
Of course, we can still improve the docs of the deprecated constructors,
too.
Just my two cents.
-Matthias
On 4/23/17 3:37 PM, Jay Kreps wrote:
> Hey guys,
>
> I definitely think that the constructors could have been better designed,
> but I think given that they're in heavy use I don't think this proposal
> will improve things. Deprecating constructors just leaves everyone with
> lots of warnings and crossed out things. We can't actually delete the
> methods because lots of code needs to be usable across multiple Kafka
> versions, right? So we aren't picking between the original approach (worse)
> and the new approach (better); what we are proposing is a perpetual
> mingling of the original style and the new style with a bunch of deprecated
> stuff, which I think is worst of all.
>
> I'd vote for just documenting the meaning of null in the ProducerRecord
> constructor.
>
> -Jay
>
> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> stephane@simplemachines.com.au> wrote:
>
>> Hi all,
>>
>> My first KIP, let me know your thoughts!
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
>> 141+-+ProducerRecordBuilder+Interface
>>
>>
>> Cheers,
>> Stephane
>>
>
The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Stephane Maarek <st...@simplemachines.com.au>.
Hi
Thanks for all your input. My initial intent was to just add a “timestamp” constructor for the ProducerRecord, and it seems greater changes are too contentious.
I’ve just rolled back the PR to add the missing constructors, in the way Ismael suggested.
Updated KIP here: https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=69406838
Added constructors here: https://github.com/apache/kafka/pull/2894
Let me know your thoughts,
Regards,
Stephane
On 25/4/17, 2:58 am, "Michael Pearce" <Mi...@ig.com> wrote:
Why not simply make a cleaner client fluent API wrapper? The internals use and send via current api, but provide a cleaner more fluent api.
A good example here is HTTP compontents where they did this.
https://hc.apache.org/httpcomponents-client-ga/tutorial/html/fluent.html
On 24/04/2017, 17:40, "Matthias J. Sax" <ma...@confluent.io> wrote:
Hey Jay,
I understand your concern, and for sure, we will need to keep the
current constructors deprecated for a long time (ie, many years).
But if we don't make the move, we will not be able to improve. And I
think warnings about using deprecated APIs is an acceptable price to
pay. And the API improvements will help new people who adopt Kafka to
get started more easily.
Otherwise Kafka might end up as many other enterprise software with a
lots of old stuff that is kept forever because nobody has the guts to
improve/change it.
Of course, we can still improve the docs of the deprecated constructors,
too.
Just my two cents.
-Matthias
On 4/23/17 3:37 PM, Jay Kreps wrote:
> Hey guys,
>
> I definitely think that the constructors could have been better designed,
> but I think given that they're in heavy use I don't think this proposal
> will improve things. Deprecating constructors just leaves everyone with
> lots of warnings and crossed out things. We can't actually delete the
> methods because lots of code needs to be usable across multiple Kafka
> versions, right? So we aren't picking between the original approach (worse)
> and the new approach (better); what we are proposing is a perpetual
> mingling of the original style and the new style with a bunch of deprecated
> stuff, which I think is worst of all.
>
> I'd vote for just documenting the meaning of null in the ProducerRecord
> constructor.
>
> -Jay
>
> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> stephane@simplemachines.com.au> wrote:
>
>> Hi all,
>>
>> My first KIP, let me know your thoughts!
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
>> 141+-+ProducerRecordBuilder+Interface
>>
>>
>> Cheers,
>> Stephane
>>
>
The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Michael Pearce <Mi...@ig.com>.
Why not simply make a cleaner client fluent API wrapper? The internals use and send via current api, but provide a cleaner more fluent api.
A good example here is HTTP compontents where they did this.
https://hc.apache.org/httpcomponents-client-ga/tutorial/html/fluent.html
On 24/04/2017, 17:40, "Matthias J. Sax" <ma...@confluent.io> wrote:
Hey Jay,
I understand your concern, and for sure, we will need to keep the
current constructors deprecated for a long time (ie, many years).
But if we don't make the move, we will not be able to improve. And I
think warnings about using deprecated APIs is an acceptable price to
pay. And the API improvements will help new people who adopt Kafka to
get started more easily.
Otherwise Kafka might end up as many other enterprise software with a
lots of old stuff that is kept forever because nobody has the guts to
improve/change it.
Of course, we can still improve the docs of the deprecated constructors,
too.
Just my two cents.
-Matthias
On 4/23/17 3:37 PM, Jay Kreps wrote:
> Hey guys,
>
> I definitely think that the constructors could have been better designed,
> but I think given that they're in heavy use I don't think this proposal
> will improve things. Deprecating constructors just leaves everyone with
> lots of warnings and crossed out things. We can't actually delete the
> methods because lots of code needs to be usable across multiple Kafka
> versions, right? So we aren't picking between the original approach (worse)
> and the new approach (better); what we are proposing is a perpetual
> mingling of the original style and the new style with a bunch of deprecated
> stuff, which I think is worst of all.
>
> I'd vote for just documenting the meaning of null in the ProducerRecord
> constructor.
>
> -Jay
>
> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> stephane@simplemachines.com.au> wrote:
>
>> Hi all,
>>
>> My first KIP, let me know your thoughts!
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
>> 141+-+ProducerRecordBuilder+Interface
>>
>>
>> Cheers,
>> Stephane
>>
>
The information contained in this email is strictly confidential and for the use of the addressee only, unless otherwise indicated. If you are not the intended recipient, please do not read, copy, use or disclose to others this message or any attachment. Please also notify the sender by replying to this email or by telephone (+44(020 7896 0011) and then delete the email and any copies of it. Opinions, conclusion (etc) that do not relate to the official business of this company shall be understood as neither given nor endorsed by it. IG is a trading name of IG Markets Limited (a company registered in England and Wales, company number 04008957) and IG Index Limited (a company registered in England and Wales, company number 01190902). Registered address at Cannon Bridge House, 25 Dowgate Hill, London EC4R 2YA. Both IG Markets Limited (register number 195355) and IG Index Limited (register number 114059) are authorised and regulated by the Financial Conduct Authority.
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by "Matthias J. Sax" <ma...@confluent.io>.
Hey Jay,
I understand your concern, and for sure, we will need to keep the
current constructors deprecated for a long time (ie, many years).
But if we don't make the move, we will not be able to improve. And I
think warnings about using deprecated APIs is an acceptable price to
pay. And the API improvements will help new people who adopt Kafka to
get started more easily.
Otherwise Kafka might end up as many other enterprise software with a
lots of old stuff that is kept forever because nobody has the guts to
improve/change it.
Of course, we can still improve the docs of the deprecated constructors,
too.
Just my two cents.
-Matthias
On 4/23/17 3:37 PM, Jay Kreps wrote:
> Hey guys,
>
> I definitely think that the constructors could have been better designed,
> but I think given that they're in heavy use I don't think this proposal
> will improve things. Deprecating constructors just leaves everyone with
> lots of warnings and crossed out things. We can't actually delete the
> methods because lots of code needs to be usable across multiple Kafka
> versions, right? So we aren't picking between the original approach (worse)
> and the new approach (better); what we are proposing is a perpetual
> mingling of the original style and the new style with a bunch of deprecated
> stuff, which I think is worst of all.
>
> I'd vote for just documenting the meaning of null in the ProducerRecord
> constructor.
>
> -Jay
>
> On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
> stephane@simplemachines.com.au> wrote:
>
>> Hi all,
>>
>> My first KIP, let me know your thoughts!
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
>> 141+-+ProducerRecordBuilder+Interface
>>
>>
>> Cheers,
>> Stephane
>>
>
Re: [DISCUSS] KIP 141 - ProducerRecordBuilder Interface
Posted by Jay Kreps <ja...@confluent.io>.
Hey guys,
I definitely think that the constructors could have been better designed,
but I think given that they're in heavy use I don't think this proposal
will improve things. Deprecating constructors just leaves everyone with
lots of warnings and crossed out things. We can't actually delete the
methods because lots of code needs to be usable across multiple Kafka
versions, right? So we aren't picking between the original approach (worse)
and the new approach (better); what we are proposing is a perpetual
mingling of the original style and the new style with a bunch of deprecated
stuff, which I think is worst of all.
I'd vote for just documenting the meaning of null in the ProducerRecord
constructor.
-Jay
On Wed, Apr 19, 2017 at 3:34 PM, Stephane Maarek <
stephane@simplemachines.com.au> wrote:
> Hi all,
>
> My first KIP, let me know your thoughts!
> https://cwiki.apache.org/confluence/display/KAFKA/KIP+
> 141+-+ProducerRecordBuilder+Interface
>
>
> Cheers,
> Stephane
>