You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Almog Gavra <al...@confluent.io> on 2019/05/01 18:24:06 UTC
[VOTE] KIP-464: Defaults for AdminClient#createTopic
Hello Everyone!
Kicking off the voting for
https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
You can see discussion thread here (please respond with suggestions on that
thread):
https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
Cheers,
Almog
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Randall Hauch <rh...@gmail.com>.
+! (binding) on the current KIP with the builder, based on the fact that
the builder simplifies usage vs adding other constructors and is more
easily extended over time.
Randall
On Thu, May 9, 2019 at 5:38 PM Almog Gavra <al...@confluent.io> wrote:
> Thanks Colin! Since the discussion around the builder is here I'll copy
> over my comment from the discuss thread:
>
> If we want the flexibility that the builder provides we would need to add
> three constructors:
> - no partitions/replicas
> - just partitions
> - just replicas
>
> I see good use cases for the first two - the third (just replicas) seems
> less necessary but complicates the API a bit (you have to differentiate
> NewTopic(int) with NewTopic(short) or something like that). If we're happy
> with a KIP that covers just the first two then I can remove the builder to
> simplify things. Otherwise, I think the builder is an important addition.
>
> Thoughts?
>
> On Thu, May 9, 2019 at 2:50 PM Colin McCabe <cm...@apache.org> wrote:
>
> > +1 (binding).
> >
> > Re: the builder discussion. I don't feel strongly either way-- the
> > builder sketched out in the KIP looks reasonable, but I can also
> understand
> > Ismael's argument for keeping the KIP minimal.
> >
> > best,
> > Colin
> >
> >
> > On Thu, May 9, 2019, at 08:09, Randall Hauch wrote:
> > > I'm fine with simplifying the KIP by removing the Builder (which seems
> > > ancillary), or keeping the KIP as-is. I'll wait to vote until Almog
> says
> > > which way he'd like to proceed.
> > >
> > > On Thu, May 9, 2019 at 9:45 AM Ismael Juma <is...@juma.me.uk> wrote:
> > >
> > > > Hi Almog,
> > > >
> > > > Adding a Builder seems unrelated to this change. Do we need it? Given
> > the
> > > > imminent KIP deadline, I'd keep it simple and just have the
> constructor
> > > > with just the name parameter.
> > > >
> > > > Ismael
> > > >
> > > > On Thu, May 2, 2019 at 1:58 AM Mickael Maison <
> > mickael.maison@gmail.com>
> > > > wrote:
> > > >
> > > > > I was planning to write a KIP for the exact same feature!
> > > > > +1 (non binding)
> > > > >
> > > > > Thanks for the KIP
> > > > >
> > > > > On Wed, May 1, 2019 at 7:24 PM Almog Gavra <al...@confluent.io>
> > wrote:
> > > > > >
> > > > > > Hello Everyone!
> > > > > >
> > > > > > Kicking off the voting for
> > > > > >
> > > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
> > > > > >
> > > > > >
> > > > > > You can see discussion thread here (please respond with
> > suggestions on
> > > > > that
> > > > > > thread):
> > > > > >
> > > > >
> > > >
> >
> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
> > > > > >
> > > > > >
> > > > > > Cheers,
> > > > > > Almog
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Patrik Kleindl <pk...@gmail.com>.
Good idea, this will prevent a couple of headaches.
Regards
Patrik
> Am 13.06.2019 um 00:24 schrieb Matthias J. Sax <ma...@confluent.io>:
>
> We want to make a small additional change and piggy-back it to this KIP.
>
> To exploit the new feature of AdmintClient in KafkaStreams, we want to
> update the default value of Streams configuration parameter
> `replication.factor` from `1` to `-1`.
>
> This config change will of course only go into 2.4 release.
>
> I updated the KIP accordingly and created
> https://issues.apache.org/jira/browse/KAFKA-8531
>
> Let us know if there are any concerns. I don't think that we need to
> revote base on this tiny change.
>
>
> -Matthias
>
>> On 5/28/19 8:50 AM, Almog Gavra wrote:
>> Hello everyone - the PR is out and ready to review!
>> https://github.com/apache/kafka/pull/6728/
>>
>>> On Fri, May 10, 2019 at 11:57 AM Almog Gavra <al...@confluent.io> wrote:
>>>
>>> Thanks everyone for the comments and discussion! Closing the voting out
>>> for this KIP:
>>>
>>> * 4 Binding (Randall, Manikumar, Colin, Gwen)
>>> * 2 Non-Binding (Ryanne, Mickael)
>>>
>>> Cheers,
>>> Almog
>>>
>>>
>>>> On Fri, May 10, 2019 at 11:55 AM Gwen Shapira <gw...@confluent.io> wrote:
>>>>
>>>> +1 (binding)
>>>>
>>>>> On Fri, May 10, 2019 at 9:32 AM Almog Gavra <al...@confluent.io> wrote:
>>>>>
>>>>> I'm happy pulling it out into a separate KIP to target the discussion.
>>>> This
>>>>> one can just introduce the "default" constructor for no partitions or
>>>>> replicas since we'll need that one whether or not we add the builder.
>>>>>
>>>>> Updated the KIP moving the builder to a section in "Rejected
>>>> Alternatives -
>>>>> Follow Up KIP" - @Randall since your +1 was for the KIP with builder I
>>>> want
>>>>> to make sure that is okay with you.
>>>>>
>>>>> On Fri, May 10, 2019 at 9:14 AM Colin McCabe <cm...@apache.org>
>>>> wrote:
>>>>>
>>>>>> Given that there are still some open questions about the builder,
>>>> maybe
>>>>> we
>>>>>> should put it in a separate KIP?
>>>>>>
>>>>>> best,
>>>>>> Colin
>>>>>>
>>>>>>
>>>>>>> On Fri, May 10, 2019, at 09:00, Ryanne Dolan wrote:
>>>>>>> +1 (non-binding) for the core feature, but I could take or leave the
>>>>>>> builder.
>>>>>>>
>>>>>>> Ryanne
>>>>>>>
>>>>>>> On Fri, May 10, 2019 at 10:43 AM Almog Gavra <al...@confluent.io>
>>>>> wrote:
>>>>>>>
>>>>>>>> @Ismael - I agree that the methods are a little random. They were
>>>>> just
>>>>>>>> ported from what's currently in the connect builder. I think a
>>>> better
>>>>>>>> option might be to keep the connect builder around and have extend
>>>>> from
>>>>>>>> this builder, and make this builder only implement the "critical"
>>>>>> methods
>>>>>>>> (e.g. replicas/partitions/config). (cc Randall)
>>>>>>>>
>>>>>>>> Is passing optionals in the constructor something that's common in
>>>>> AK?
>>>>>> I
>>>>>>>> think my preference would be toward the builder so that it's
>>>> easier
>>>>> to
>>>>>>>> extend as Randall mentioned.
>>>>>>>>
>>>>>>>> Almog
>>>>>>>>
>>>>>>>> On Fri, May 10, 2019 at 8:17 AM Ismael Juma <is...@juma.me.uk>
>>>>> wrote:
>>>>>>>>
>>>>>>>>> The current builder includes random methods like
>>>>>> uncleanLeaderElection.
>>>>>>>>> That doesn't make sense to me since it's a topic config (and we
>>>>> don't
>>>>>>>>> include methods for other topic configs). Also, I'm not sure
>>>> about
>>>>>> the
>>>>>>>>> naming convention, should we have a `with` prefix? It would be
>>>> good
>>>>>> to
>>>>>>>>> check existing builders in `clients` if any exist for what
>>>> they're
>>>>>> doing.
>>>>>>>>>
>>>>>>>>> If we didn't add a builder, another option would be a single new
>>>>>>>>> constructor:
>>>>>>>>>
>>>>>>>>> public NewTopic(String name, Optional<Integer> numPartitions,
>>>>>>>>> Optional<Short> replicationFactor)
>>>>>>>>>
>>>>>>>>> Ismael
>>>>>>>>>
>>>>>>>>> On Thu, May 9, 2019 at 3:38 PM Almog Gavra <al...@confluent.io>
>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> Thanks Colin! Since the discussion around the builder is here
>>>>> I'll
>>>>>> copy
>>>>>>>>>> over my comment from the discuss thread:
>>>>>>>>>>
>>>>>>>>>> If we want the flexibility that the builder provides we would
>>>>> need
>>>>>> to
>>>>>>>> add
>>>>>>>>>> three constructors:
>>>>>>>>>> - no partitions/replicas
>>>>>>>>>> - just partitions
>>>>>>>>>> - just replicas
>>>>>>>>>>
>>>>>>>>>> I see good use cases for the first two - the third (just
>>>>> replicas)
>>>>>>>> seems
>>>>>>>>>> less necessary but complicates the API a bit (you have to
>>>>>> differentiate
>>>>>>>>>> NewTopic(int) with NewTopic(short) or something like that). If
>>>>>> we're
>>>>>>>>> happy
>>>>>>>>>> with a KIP that covers just the first two then I can remove
>>>> the
>>>>>> builder
>>>>>>>>> to
>>>>>>>>>> simplify things. Otherwise, I think the builder is an
>>>> important
>>>>>>>> addition.
>>>>>>>>>>
>>>>>>>>>> Thoughts?
>>>>>>>>>>
>>>>>>>>>> On Thu, May 9, 2019 at 2:50 PM Colin McCabe <
>>>> cmccabe@apache.org>
>>>>>>>> wrote:
>>>>>>>>>>
>>>>>>>>>>> +1 (binding).
>>>>>>>>>>>
>>>>>>>>>>> Re: the builder discussion. I don't feel strongly either
>>>> way--
>>>>>> the
>>>>>>>>>>> builder sketched out in the KIP looks reasonable, but I can
>>>>> also
>>>>>>>>>> understand
>>>>>>>>>>> Ismael's argument for keeping the KIP minimal.
>>>>>>>>>>>
>>>>>>>>>>> best,
>>>>>>>>>>> Colin
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> On Thu, May 9, 2019, at 08:09, Randall Hauch wrote:
>>>>>>>>>>>> I'm fine with simplifying the KIP by removing the Builder
>>>>>> (which
>>>>>>>>> seems
>>>>>>>>>>>> ancillary), or keeping the KIP as-is. I'll wait to vote
>>>> until
>>>>>> Almog
>>>>>>>>>> says
>>>>>>>>>>>> which way he'd like to proceed.
>>>>>>>>>>>>
>>>>>>>>>>>> On Thu, May 9, 2019 at 9:45 AM Ismael Juma <
>>>>> ismael@juma.me.uk>
>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Almog,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Adding a Builder seems unrelated to this change. Do we
>>>> need
>>>>>> it?
>>>>>>>>> Given
>>>>>>>>>>> the
>>>>>>>>>>>>> imminent KIP deadline, I'd keep it simple and just have
>>>> the
>>>>>>>>>> constructor
>>>>>>>>>>>>> with just the name parameter.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Ismael
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Thu, May 2, 2019 at 1:58 AM Mickael Maison <
>>>>>>>>>>> mickael.maison@gmail.com>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>
>>>>>>>>>>>>>> I was planning to write a KIP for the exact same
>>>> feature!
>>>>>>>>>>>>>> +1 (non binding)
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Thanks for the KIP
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On Wed, May 1, 2019 at 7:24 PM Almog Gavra <
>>>>>> almog@confluent.io
>>>>>>>>>
>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hello Everyone!
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Kicking off the voting for
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> You can see discussion thread here (please respond
>>>> with
>>>>>>>>>>> suggestions on
>>>>>>>>>>>>>> that
>>>>>>>>>>>>>>> thread):
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>
>>>> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>>> Almog
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> *Gwen Shapira*
>>>> Product Manager | Confluent
>>>> 650.450.2760 | @gwenshap
>>>> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
>>>> <http://www.confluent.io/blog>
>>>>
>>>
>>
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by "Matthias J. Sax" <ma...@confluent.io>.
We want to make a small additional change and piggy-back it to this KIP.
To exploit the new feature of AdmintClient in KafkaStreams, we want to
update the default value of Streams configuration parameter
`replication.factor` from `1` to `-1`.
This config change will of course only go into 2.4 release.
I updated the KIP accordingly and created
https://issues.apache.org/jira/browse/KAFKA-8531
Let us know if there are any concerns. I don't think that we need to
revote base on this tiny change.
-Matthias
On 5/28/19 8:50 AM, Almog Gavra wrote:
> Hello everyone - the PR is out and ready to review!
> https://github.com/apache/kafka/pull/6728/
>
> On Fri, May 10, 2019 at 11:57 AM Almog Gavra <al...@confluent.io> wrote:
>
>> Thanks everyone for the comments and discussion! Closing the voting out
>> for this KIP:
>>
>> * 4 Binding (Randall, Manikumar, Colin, Gwen)
>> * 2 Non-Binding (Ryanne, Mickael)
>>
>> Cheers,
>> Almog
>>
>>
>> On Fri, May 10, 2019 at 11:55 AM Gwen Shapira <gw...@confluent.io> wrote:
>>
>>> +1 (binding)
>>>
>>> On Fri, May 10, 2019 at 9:32 AM Almog Gavra <al...@confluent.io> wrote:
>>>
>>>> I'm happy pulling it out into a separate KIP to target the discussion.
>>> This
>>>> one can just introduce the "default" constructor for no partitions or
>>>> replicas since we'll need that one whether or not we add the builder.
>>>>
>>>> Updated the KIP moving the builder to a section in "Rejected
>>> Alternatives -
>>>> Follow Up KIP" - @Randall since your +1 was for the KIP with builder I
>>> want
>>>> to make sure that is okay with you.
>>>>
>>>> On Fri, May 10, 2019 at 9:14 AM Colin McCabe <cm...@apache.org>
>>> wrote:
>>>>
>>>>> Given that there are still some open questions about the builder,
>>> maybe
>>>> we
>>>>> should put it in a separate KIP?
>>>>>
>>>>> best,
>>>>> Colin
>>>>>
>>>>>
>>>>> On Fri, May 10, 2019, at 09:00, Ryanne Dolan wrote:
>>>>>> +1 (non-binding) for the core feature, but I could take or leave the
>>>>>> builder.
>>>>>>
>>>>>> Ryanne
>>>>>>
>>>>>> On Fri, May 10, 2019 at 10:43 AM Almog Gavra <al...@confluent.io>
>>>> wrote:
>>>>>>
>>>>>>> @Ismael - I agree that the methods are a little random. They were
>>>> just
>>>>>>> ported from what's currently in the connect builder. I think a
>>> better
>>>>>>> option might be to keep the connect builder around and have extend
>>>> from
>>>>>>> this builder, and make this builder only implement the "critical"
>>>>> methods
>>>>>>> (e.g. replicas/partitions/config). (cc Randall)
>>>>>>>
>>>>>>> Is passing optionals in the constructor something that's common in
>>>> AK?
>>>>> I
>>>>>>> think my preference would be toward the builder so that it's
>>> easier
>>>> to
>>>>>>> extend as Randall mentioned.
>>>>>>>
>>>>>>> Almog
>>>>>>>
>>>>>>> On Fri, May 10, 2019 at 8:17 AM Ismael Juma <is...@juma.me.uk>
>>>> wrote:
>>>>>>>
>>>>>>>> The current builder includes random methods like
>>>>> uncleanLeaderElection.
>>>>>>>> That doesn't make sense to me since it's a topic config (and we
>>>> don't
>>>>>>>> include methods for other topic configs). Also, I'm not sure
>>> about
>>>>> the
>>>>>>>> naming convention, should we have a `with` prefix? It would be
>>> good
>>>>> to
>>>>>>>> check existing builders in `clients` if any exist for what
>>> they're
>>>>> doing.
>>>>>>>>
>>>>>>>> If we didn't add a builder, another option would be a single new
>>>>>>>> constructor:
>>>>>>>>
>>>>>>>> public NewTopic(String name, Optional<Integer> numPartitions,
>>>>>>>> Optional<Short> replicationFactor)
>>>>>>>>
>>>>>>>> Ismael
>>>>>>>>
>>>>>>>> On Thu, May 9, 2019 at 3:38 PM Almog Gavra <al...@confluent.io>
>>>>> wrote:
>>>>>>>>
>>>>>>>>> Thanks Colin! Since the discussion around the builder is here
>>>> I'll
>>>>> copy
>>>>>>>>> over my comment from the discuss thread:
>>>>>>>>>
>>>>>>>>> If we want the flexibility that the builder provides we would
>>>> need
>>>>> to
>>>>>>> add
>>>>>>>>> three constructors:
>>>>>>>>> - no partitions/replicas
>>>>>>>>> - just partitions
>>>>>>>>> - just replicas
>>>>>>>>>
>>>>>>>>> I see good use cases for the first two - the third (just
>>>> replicas)
>>>>>>> seems
>>>>>>>>> less necessary but complicates the API a bit (you have to
>>>>> differentiate
>>>>>>>>> NewTopic(int) with NewTopic(short) or something like that). If
>>>>> we're
>>>>>>>> happy
>>>>>>>>> with a KIP that covers just the first two then I can remove
>>> the
>>>>> builder
>>>>>>>> to
>>>>>>>>> simplify things. Otherwise, I think the builder is an
>>> important
>>>>>>> addition.
>>>>>>>>>
>>>>>>>>> Thoughts?
>>>>>>>>>
>>>>>>>>> On Thu, May 9, 2019 at 2:50 PM Colin McCabe <
>>> cmccabe@apache.org>
>>>>>>> wrote:
>>>>>>>>>
>>>>>>>>>> +1 (binding).
>>>>>>>>>>
>>>>>>>>>> Re: the builder discussion. I don't feel strongly either
>>> way--
>>>>> the
>>>>>>>>>> builder sketched out in the KIP looks reasonable, but I can
>>>> also
>>>>>>>>> understand
>>>>>>>>>> Ismael's argument for keeping the KIP minimal.
>>>>>>>>>>
>>>>>>>>>> best,
>>>>>>>>>> Colin
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On Thu, May 9, 2019, at 08:09, Randall Hauch wrote:
>>>>>>>>>>> I'm fine with simplifying the KIP by removing the Builder
>>>>> (which
>>>>>>>> seems
>>>>>>>>>>> ancillary), or keeping the KIP as-is. I'll wait to vote
>>> until
>>>>> Almog
>>>>>>>>> says
>>>>>>>>>>> which way he'd like to proceed.
>>>>>>>>>>>
>>>>>>>>>>> On Thu, May 9, 2019 at 9:45 AM Ismael Juma <
>>>> ismael@juma.me.uk>
>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>>> Hi Almog,
>>>>>>>>>>>>
>>>>>>>>>>>> Adding a Builder seems unrelated to this change. Do we
>>> need
>>>>> it?
>>>>>>>> Given
>>>>>>>>>> the
>>>>>>>>>>>> imminent KIP deadline, I'd keep it simple and just have
>>> the
>>>>>>>>> constructor
>>>>>>>>>>>> with just the name parameter.
>>>>>>>>>>>>
>>>>>>>>>>>> Ismael
>>>>>>>>>>>>
>>>>>>>>>>>> On Thu, May 2, 2019 at 1:58 AM Mickael Maison <
>>>>>>>>>> mickael.maison@gmail.com>
>>>>>>>>>>>> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> I was planning to write a KIP for the exact same
>>> feature!
>>>>>>>>>>>>> +1 (non binding)
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks for the KIP
>>>>>>>>>>>>>
>>>>>>>>>>>>> On Wed, May 1, 2019 at 7:24 PM Almog Gavra <
>>>>> almog@confluent.io
>>>>>>>>
>>>>>>>>>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Hello Everyone!
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Kicking off the voting for
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>>
>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> You can see discussion thread here (please respond
>>> with
>>>>>>>>>> suggestions on
>>>>>>>>>>>>> that
>>>>>>>>>>>>>> thread):
>>>>>>>>>>>>>>
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>
>>>>
>>> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Cheers,
>>>>>>>>>>>>>> Almog
>>>>>>>>>>>>>
>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>>
>>> --
>>> *Gwen Shapira*
>>> Product Manager | Confluent
>>> 650.450.2760 | @gwenshap
>>> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
>>> <http://www.confluent.io/blog>
>>>
>>
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Almog Gavra <al...@confluent.io>.
Hello everyone - the PR is out and ready to review!
https://github.com/apache/kafka/pull/6728/
On Fri, May 10, 2019 at 11:57 AM Almog Gavra <al...@confluent.io> wrote:
> Thanks everyone for the comments and discussion! Closing the voting out
> for this KIP:
>
> * 4 Binding (Randall, Manikumar, Colin, Gwen)
> * 2 Non-Binding (Ryanne, Mickael)
>
> Cheers,
> Almog
>
>
> On Fri, May 10, 2019 at 11:55 AM Gwen Shapira <gw...@confluent.io> wrote:
>
>> +1 (binding)
>>
>> On Fri, May 10, 2019 at 9:32 AM Almog Gavra <al...@confluent.io> wrote:
>>
>> > I'm happy pulling it out into a separate KIP to target the discussion.
>> This
>> > one can just introduce the "default" constructor for no partitions or
>> > replicas since we'll need that one whether or not we add the builder.
>> >
>> > Updated the KIP moving the builder to a section in "Rejected
>> Alternatives -
>> > Follow Up KIP" - @Randall since your +1 was for the KIP with builder I
>> want
>> > to make sure that is okay with you.
>> >
>> > On Fri, May 10, 2019 at 9:14 AM Colin McCabe <cm...@apache.org>
>> wrote:
>> >
>> > > Given that there are still some open questions about the builder,
>> maybe
>> > we
>> > > should put it in a separate KIP?
>> > >
>> > > best,
>> > > Colin
>> > >
>> > >
>> > > On Fri, May 10, 2019, at 09:00, Ryanne Dolan wrote:
>> > > > +1 (non-binding) for the core feature, but I could take or leave the
>> > > > builder.
>> > > >
>> > > > Ryanne
>> > > >
>> > > > On Fri, May 10, 2019 at 10:43 AM Almog Gavra <al...@confluent.io>
>> > wrote:
>> > > >
>> > > > > @Ismael - I agree that the methods are a little random. They were
>> > just
>> > > > > ported from what's currently in the connect builder. I think a
>> better
>> > > > > option might be to keep the connect builder around and have extend
>> > from
>> > > > > this builder, and make this builder only implement the "critical"
>> > > methods
>> > > > > (e.g. replicas/partitions/config). (cc Randall)
>> > > > >
>> > > > > Is passing optionals in the constructor something that's common in
>> > AK?
>> > > I
>> > > > > think my preference would be toward the builder so that it's
>> easier
>> > to
>> > > > > extend as Randall mentioned.
>> > > > >
>> > > > > Almog
>> > > > >
>> > > > > On Fri, May 10, 2019 at 8:17 AM Ismael Juma <is...@juma.me.uk>
>> > wrote:
>> > > > >
>> > > > > > The current builder includes random methods like
>> > > uncleanLeaderElection.
>> > > > > > That doesn't make sense to me since it's a topic config (and we
>> > don't
>> > > > > > include methods for other topic configs). Also, I'm not sure
>> about
>> > > the
>> > > > > > naming convention, should we have a `with` prefix? It would be
>> good
>> > > to
>> > > > > > check existing builders in `clients` if any exist for what
>> they're
>> > > doing.
>> > > > > >
>> > > > > > If we didn't add a builder, another option would be a single new
>> > > > > > constructor:
>> > > > > >
>> > > > > > public NewTopic(String name, Optional<Integer> numPartitions,
>> > > > > > Optional<Short> replicationFactor)
>> > > > > >
>> > > > > > Ismael
>> > > > > >
>> > > > > > On Thu, May 9, 2019 at 3:38 PM Almog Gavra <al...@confluent.io>
>> > > wrote:
>> > > > > >
>> > > > > > > Thanks Colin! Since the discussion around the builder is here
>> > I'll
>> > > copy
>> > > > > > > over my comment from the discuss thread:
>> > > > > > >
>> > > > > > > If we want the flexibility that the builder provides we would
>> > need
>> > > to
>> > > > > add
>> > > > > > > three constructors:
>> > > > > > > - no partitions/replicas
>> > > > > > > - just partitions
>> > > > > > > - just replicas
>> > > > > > >
>> > > > > > > I see good use cases for the first two - the third (just
>> > replicas)
>> > > > > seems
>> > > > > > > less necessary but complicates the API a bit (you have to
>> > > differentiate
>> > > > > > > NewTopic(int) with NewTopic(short) or something like that). If
>> > > we're
>> > > > > > happy
>> > > > > > > with a KIP that covers just the first two then I can remove
>> the
>> > > builder
>> > > > > > to
>> > > > > > > simplify things. Otherwise, I think the builder is an
>> important
>> > > > > addition.
>> > > > > > >
>> > > > > > > Thoughts?
>> > > > > > >
>> > > > > > > On Thu, May 9, 2019 at 2:50 PM Colin McCabe <
>> cmccabe@apache.org>
>> > > > > wrote:
>> > > > > > >
>> > > > > > > > +1 (binding).
>> > > > > > > >
>> > > > > > > > Re: the builder discussion. I don't feel strongly either
>> way--
>> > > the
>> > > > > > > > builder sketched out in the KIP looks reasonable, but I can
>> > also
>> > > > > > > understand
>> > > > > > > > Ismael's argument for keeping the KIP minimal.
>> > > > > > > >
>> > > > > > > > best,
>> > > > > > > > Colin
>> > > > > > > >
>> > > > > > > >
>> > > > > > > > On Thu, May 9, 2019, at 08:09, Randall Hauch wrote:
>> > > > > > > > > I'm fine with simplifying the KIP by removing the Builder
>> > > (which
>> > > > > > seems
>> > > > > > > > > ancillary), or keeping the KIP as-is. I'll wait to vote
>> until
>> > > Almog
>> > > > > > > says
>> > > > > > > > > which way he'd like to proceed.
>> > > > > > > > >
>> > > > > > > > > On Thu, May 9, 2019 at 9:45 AM Ismael Juma <
>> > ismael@juma.me.uk>
>> > > > > > wrote:
>> > > > > > > > >
>> > > > > > > > > > Hi Almog,
>> > > > > > > > > >
>> > > > > > > > > > Adding a Builder seems unrelated to this change. Do we
>> need
>> > > it?
>> > > > > > Given
>> > > > > > > > the
>> > > > > > > > > > imminent KIP deadline, I'd keep it simple and just have
>> the
>> > > > > > > constructor
>> > > > > > > > > > with just the name parameter.
>> > > > > > > > > >
>> > > > > > > > > > Ismael
>> > > > > > > > > >
>> > > > > > > > > > On Thu, May 2, 2019 at 1:58 AM Mickael Maison <
>> > > > > > > > mickael.maison@gmail.com>
>> > > > > > > > > > wrote:
>> > > > > > > > > >
>> > > > > > > > > > > I was planning to write a KIP for the exact same
>> feature!
>> > > > > > > > > > > +1 (non binding)
>> > > > > > > > > > >
>> > > > > > > > > > > Thanks for the KIP
>> > > > > > > > > > >
>> > > > > > > > > > > On Wed, May 1, 2019 at 7:24 PM Almog Gavra <
>> > > almog@confluent.io
>> > > > > >
>> > > > > > > > wrote:
>> > > > > > > > > > > >
>> > > > > > > > > > > > Hello Everyone!
>> > > > > > > > > > > >
>> > > > > > > > > > > > Kicking off the voting for
>> > > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
>> > > > > > > > > > > >
>> > > > > > > > > > > >
>> > > > > > > > > > > > You can see discussion thread here (please respond
>> with
>> > > > > > > > suggestions on
>> > > > > > > > > > > that
>> > > > > > > > > > > > thread):
>> > > > > > > > > > > >
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > >
>> >
>> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
>> > > > > > > > > > > >
>> > > > > > > > > > > >
>> > > > > > > > > > > > Cheers,
>> > > > > > > > > > > > Almog
>> > > > > > > > > > >
>> > > > > > > > > >
>> > > > > > > > >
>> > > > > > > >
>> > > > > > >
>> > > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>>
>> --
>> *Gwen Shapira*
>> Product Manager | Confluent
>> 650.450.2760 | @gwenshap
>> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
>> <http://www.confluent.io/blog>
>>
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Almog Gavra <al...@confluent.io>.
Thanks everyone for the comments and discussion! Closing the voting out for
this KIP:
* 4 Binding (Randall, Manikumar, Colin, Gwen)
* 2 Non-Binding (Ryanne, Mickael)
Cheers,
Almog
On Fri, May 10, 2019 at 11:55 AM Gwen Shapira <gw...@confluent.io> wrote:
> +1 (binding)
>
> On Fri, May 10, 2019 at 9:32 AM Almog Gavra <al...@confluent.io> wrote:
>
> > I'm happy pulling it out into a separate KIP to target the discussion.
> This
> > one can just introduce the "default" constructor for no partitions or
> > replicas since we'll need that one whether or not we add the builder.
> >
> > Updated the KIP moving the builder to a section in "Rejected
> Alternatives -
> > Follow Up KIP" - @Randall since your +1 was for the KIP with builder I
> want
> > to make sure that is okay with you.
> >
> > On Fri, May 10, 2019 at 9:14 AM Colin McCabe <cm...@apache.org> wrote:
> >
> > > Given that there are still some open questions about the builder, maybe
> > we
> > > should put it in a separate KIP?
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Fri, May 10, 2019, at 09:00, Ryanne Dolan wrote:
> > > > +1 (non-binding) for the core feature, but I could take or leave the
> > > > builder.
> > > >
> > > > Ryanne
> > > >
> > > > On Fri, May 10, 2019 at 10:43 AM Almog Gavra <al...@confluent.io>
> > wrote:
> > > >
> > > > > @Ismael - I agree that the methods are a little random. They were
> > just
> > > > > ported from what's currently in the connect builder. I think a
> better
> > > > > option might be to keep the connect builder around and have extend
> > from
> > > > > this builder, and make this builder only implement the "critical"
> > > methods
> > > > > (e.g. replicas/partitions/config). (cc Randall)
> > > > >
> > > > > Is passing optionals in the constructor something that's common in
> > AK?
> > > I
> > > > > think my preference would be toward the builder so that it's easier
> > to
> > > > > extend as Randall mentioned.
> > > > >
> > > > > Almog
> > > > >
> > > > > On Fri, May 10, 2019 at 8:17 AM Ismael Juma <is...@juma.me.uk>
> > wrote:
> > > > >
> > > > > > The current builder includes random methods like
> > > uncleanLeaderElection.
> > > > > > That doesn't make sense to me since it's a topic config (and we
> > don't
> > > > > > include methods for other topic configs). Also, I'm not sure
> about
> > > the
> > > > > > naming convention, should we have a `with` prefix? It would be
> good
> > > to
> > > > > > check existing builders in `clients` if any exist for what
> they're
> > > doing.
> > > > > >
> > > > > > If we didn't add a builder, another option would be a single new
> > > > > > constructor:
> > > > > >
> > > > > > public NewTopic(String name, Optional<Integer> numPartitions,
> > > > > > Optional<Short> replicationFactor)
> > > > > >
> > > > > > Ismael
> > > > > >
> > > > > > On Thu, May 9, 2019 at 3:38 PM Almog Gavra <al...@confluent.io>
> > > wrote:
> > > > > >
> > > > > > > Thanks Colin! Since the discussion around the builder is here
> > I'll
> > > copy
> > > > > > > over my comment from the discuss thread:
> > > > > > >
> > > > > > > If we want the flexibility that the builder provides we would
> > need
> > > to
> > > > > add
> > > > > > > three constructors:
> > > > > > > - no partitions/replicas
> > > > > > > - just partitions
> > > > > > > - just replicas
> > > > > > >
> > > > > > > I see good use cases for the first two - the third (just
> > replicas)
> > > > > seems
> > > > > > > less necessary but complicates the API a bit (you have to
> > > differentiate
> > > > > > > NewTopic(int) with NewTopic(short) or something like that). If
> > > we're
> > > > > > happy
> > > > > > > with a KIP that covers just the first two then I can remove the
> > > builder
> > > > > > to
> > > > > > > simplify things. Otherwise, I think the builder is an important
> > > > > addition.
> > > > > > >
> > > > > > > Thoughts?
> > > > > > >
> > > > > > > On Thu, May 9, 2019 at 2:50 PM Colin McCabe <
> cmccabe@apache.org>
> > > > > wrote:
> > > > > > >
> > > > > > > > +1 (binding).
> > > > > > > >
> > > > > > > > Re: the builder discussion. I don't feel strongly either
> way--
> > > the
> > > > > > > > builder sketched out in the KIP looks reasonable, but I can
> > also
> > > > > > > understand
> > > > > > > > Ismael's argument for keeping the KIP minimal.
> > > > > > > >
> > > > > > > > best,
> > > > > > > > Colin
> > > > > > > >
> > > > > > > >
> > > > > > > > On Thu, May 9, 2019, at 08:09, Randall Hauch wrote:
> > > > > > > > > I'm fine with simplifying the KIP by removing the Builder
> > > (which
> > > > > > seems
> > > > > > > > > ancillary), or keeping the KIP as-is. I'll wait to vote
> until
> > > Almog
> > > > > > > says
> > > > > > > > > which way he'd like to proceed.
> > > > > > > > >
> > > > > > > > > On Thu, May 9, 2019 at 9:45 AM Ismael Juma <
> > ismael@juma.me.uk>
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi Almog,
> > > > > > > > > >
> > > > > > > > > > Adding a Builder seems unrelated to this change. Do we
> need
> > > it?
> > > > > > Given
> > > > > > > > the
> > > > > > > > > > imminent KIP deadline, I'd keep it simple and just have
> the
> > > > > > > constructor
> > > > > > > > > > with just the name parameter.
> > > > > > > > > >
> > > > > > > > > > Ismael
> > > > > > > > > >
> > > > > > > > > > On Thu, May 2, 2019 at 1:58 AM Mickael Maison <
> > > > > > > > mickael.maison@gmail.com>
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > I was planning to write a KIP for the exact same
> feature!
> > > > > > > > > > > +1 (non binding)
> > > > > > > > > > >
> > > > > > > > > > > Thanks for the KIP
> > > > > > > > > > >
> > > > > > > > > > > On Wed, May 1, 2019 at 7:24 PM Almog Gavra <
> > > almog@confluent.io
> > > > > >
> > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hello Everyone!
> > > > > > > > > > > >
> > > > > > > > > > > > Kicking off the voting for
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > You can see discussion thread here (please respond
> with
> > > > > > > > suggestions on
> > > > > > > > > > > that
> > > > > > > > > > > > thread):
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > >
> >
> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > Cheers,
> > > > > > > > > > > > Almog
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
>
> --
> *Gwen Shapira*
> Product Manager | Confluent
> 650.450.2760 | @gwenshap
> Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
> <http://www.confluent.io/blog>
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Gwen Shapira <gw...@confluent.io>.
+1 (binding)
On Fri, May 10, 2019 at 9:32 AM Almog Gavra <al...@confluent.io> wrote:
> I'm happy pulling it out into a separate KIP to target the discussion. This
> one can just introduce the "default" constructor for no partitions or
> replicas since we'll need that one whether or not we add the builder.
>
> Updated the KIP moving the builder to a section in "Rejected Alternatives -
> Follow Up KIP" - @Randall since your +1 was for the KIP with builder I want
> to make sure that is okay with you.
>
> On Fri, May 10, 2019 at 9:14 AM Colin McCabe <cm...@apache.org> wrote:
>
> > Given that there are still some open questions about the builder, maybe
> we
> > should put it in a separate KIP?
> >
> > best,
> > Colin
> >
> >
> > On Fri, May 10, 2019, at 09:00, Ryanne Dolan wrote:
> > > +1 (non-binding) for the core feature, but I could take or leave the
> > > builder.
> > >
> > > Ryanne
> > >
> > > On Fri, May 10, 2019 at 10:43 AM Almog Gavra <al...@confluent.io>
> wrote:
> > >
> > > > @Ismael - I agree that the methods are a little random. They were
> just
> > > > ported from what's currently in the connect builder. I think a better
> > > > option might be to keep the connect builder around and have extend
> from
> > > > this builder, and make this builder only implement the "critical"
> > methods
> > > > (e.g. replicas/partitions/config). (cc Randall)
> > > >
> > > > Is passing optionals in the constructor something that's common in
> AK?
> > I
> > > > think my preference would be toward the builder so that it's easier
> to
> > > > extend as Randall mentioned.
> > > >
> > > > Almog
> > > >
> > > > On Fri, May 10, 2019 at 8:17 AM Ismael Juma <is...@juma.me.uk>
> wrote:
> > > >
> > > > > The current builder includes random methods like
> > uncleanLeaderElection.
> > > > > That doesn't make sense to me since it's a topic config (and we
> don't
> > > > > include methods for other topic configs). Also, I'm not sure about
> > the
> > > > > naming convention, should we have a `with` prefix? It would be good
> > to
> > > > > check existing builders in `clients` if any exist for what they're
> > doing.
> > > > >
> > > > > If we didn't add a builder, another option would be a single new
> > > > > constructor:
> > > > >
> > > > > public NewTopic(String name, Optional<Integer> numPartitions,
> > > > > Optional<Short> replicationFactor)
> > > > >
> > > > > Ismael
> > > > >
> > > > > On Thu, May 9, 2019 at 3:38 PM Almog Gavra <al...@confluent.io>
> > wrote:
> > > > >
> > > > > > Thanks Colin! Since the discussion around the builder is here
> I'll
> > copy
> > > > > > over my comment from the discuss thread:
> > > > > >
> > > > > > If we want the flexibility that the builder provides we would
> need
> > to
> > > > add
> > > > > > three constructors:
> > > > > > - no partitions/replicas
> > > > > > - just partitions
> > > > > > - just replicas
> > > > > >
> > > > > > I see good use cases for the first two - the third (just
> replicas)
> > > > seems
> > > > > > less necessary but complicates the API a bit (you have to
> > differentiate
> > > > > > NewTopic(int) with NewTopic(short) or something like that). If
> > we're
> > > > > happy
> > > > > > with a KIP that covers just the first two then I can remove the
> > builder
> > > > > to
> > > > > > simplify things. Otherwise, I think the builder is an important
> > > > addition.
> > > > > >
> > > > > > Thoughts?
> > > > > >
> > > > > > On Thu, May 9, 2019 at 2:50 PM Colin McCabe <cm...@apache.org>
> > > > wrote:
> > > > > >
> > > > > > > +1 (binding).
> > > > > > >
> > > > > > > Re: the builder discussion. I don't feel strongly either way--
> > the
> > > > > > > builder sketched out in the KIP looks reasonable, but I can
> also
> > > > > > understand
> > > > > > > Ismael's argument for keeping the KIP minimal.
> > > > > > >
> > > > > > > best,
> > > > > > > Colin
> > > > > > >
> > > > > > >
> > > > > > > On Thu, May 9, 2019, at 08:09, Randall Hauch wrote:
> > > > > > > > I'm fine with simplifying the KIP by removing the Builder
> > (which
> > > > > seems
> > > > > > > > ancillary), or keeping the KIP as-is. I'll wait to vote until
> > Almog
> > > > > > says
> > > > > > > > which way he'd like to proceed.
> > > > > > > >
> > > > > > > > On Thu, May 9, 2019 at 9:45 AM Ismael Juma <
> ismael@juma.me.uk>
> > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi Almog,
> > > > > > > > >
> > > > > > > > > Adding a Builder seems unrelated to this change. Do we need
> > it?
> > > > > Given
> > > > > > > the
> > > > > > > > > imminent KIP deadline, I'd keep it simple and just have the
> > > > > > constructor
> > > > > > > > > with just the name parameter.
> > > > > > > > >
> > > > > > > > > Ismael
> > > > > > > > >
> > > > > > > > > On Thu, May 2, 2019 at 1:58 AM Mickael Maison <
> > > > > > > mickael.maison@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > I was planning to write a KIP for the exact same feature!
> > > > > > > > > > +1 (non binding)
> > > > > > > > > >
> > > > > > > > > > Thanks for the KIP
> > > > > > > > > >
> > > > > > > > > > On Wed, May 1, 2019 at 7:24 PM Almog Gavra <
> > almog@confluent.io
> > > > >
> > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hello Everyone!
> > > > > > > > > > >
> > > > > > > > > > > Kicking off the voting for
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > You can see discussion thread here (please respond with
> > > > > > > suggestions on
> > > > > > > > > > that
> > > > > > > > > > > thread):
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Cheers,
> > > > > > > > > > > Almog
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
--
*Gwen Shapira*
Product Manager | Confluent
650.450.2760 | @gwenshap
Follow us: Twitter <https://twitter.com/ConfluentInc> | blog
<http://www.confluent.io/blog>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Randall Hauch <rh...@gmail.com>.
Thanks, Almog.
+1 (binding) for this simpler KIP.
On Fri, May 10, 2019 at 11:37 AM Manikumar <ma...@gmail.com>
wrote:
> Hi Almog,
>
> +1 (binding), Thanks for the KIP.
>
> Thanks,
> Manikumar
>
> On Fri, May 10, 2019 at 10:02 PM Almog Gavra <al...@confluent.io> wrote:
>
> > I'm happy pulling it out into a separate KIP to target the discussion.
> This
> > one can just introduce the "default" constructor for no partitions or
> > replicas since we'll need that one whether or not we add the builder.
> >
> > Updated the KIP moving the builder to a section in "Rejected
> Alternatives -
> > Follow Up KIP" - @Randall since your +1 was for the KIP with builder I
> want
> > to make sure that is okay with you.
> >
> > On Fri, May 10, 2019 at 9:14 AM Colin McCabe <cm...@apache.org> wrote:
> >
> > > Given that there are still some open questions about the builder, maybe
> > we
> > > should put it in a separate KIP?
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Fri, May 10, 2019, at 09:00, Ryanne Dolan wrote:
> > > > +1 (non-binding) for the core feature, but I could take or leave the
> > > > builder.
> > > >
> > > > Ryanne
> > > >
> > > > On Fri, May 10, 2019 at 10:43 AM Almog Gavra <al...@confluent.io>
> > wrote:
> > > >
> > > > > @Ismael - I agree that the methods are a little random. They were
> > just
> > > > > ported from what's currently in the connect builder. I think a
> better
> > > > > option might be to keep the connect builder around and have extend
> > from
> > > > > this builder, and make this builder only implement the "critical"
> > > methods
> > > > > (e.g. replicas/partitions/config). (cc Randall)
> > > > >
> > > > > Is passing optionals in the constructor something that's common in
> > AK?
> > > I
> > > > > think my preference would be toward the builder so that it's easier
> > to
> > > > > extend as Randall mentioned.
> > > > >
> > > > > Almog
> > > > >
> > > > > On Fri, May 10, 2019 at 8:17 AM Ismael Juma <is...@juma.me.uk>
> > wrote:
> > > > >
> > > > > > The current builder includes random methods like
> > > uncleanLeaderElection.
> > > > > > That doesn't make sense to me since it's a topic config (and we
> > don't
> > > > > > include methods for other topic configs). Also, I'm not sure
> about
> > > the
> > > > > > naming convention, should we have a `with` prefix? It would be
> good
> > > to
> > > > > > check existing builders in `clients` if any exist for what
> they're
> > > doing.
> > > > > >
> > > > > > If we didn't add a builder, another option would be a single new
> > > > > > constructor:
> > > > > >
> > > > > > public NewTopic(String name, Optional<Integer> numPartitions,
> > > > > > Optional<Short> replicationFactor)
> > > > > >
> > > > > > Ismael
> > > > > >
> > > > > > On Thu, May 9, 2019 at 3:38 PM Almog Gavra <al...@confluent.io>
> > > wrote:
> > > > > >
> > > > > > > Thanks Colin! Since the discussion around the builder is here
> > I'll
> > > copy
> > > > > > > over my comment from the discuss thread:
> > > > > > >
> > > > > > > If we want the flexibility that the builder provides we would
> > need
> > > to
> > > > > add
> > > > > > > three constructors:
> > > > > > > - no partitions/replicas
> > > > > > > - just partitions
> > > > > > > - just replicas
> > > > > > >
> > > > > > > I see good use cases for the first two - the third (just
> > replicas)
> > > > > seems
> > > > > > > less necessary but complicates the API a bit (you have to
> > > differentiate
> > > > > > > NewTopic(int) with NewTopic(short) or something like that). If
> > > we're
> > > > > > happy
> > > > > > > with a KIP that covers just the first two then I can remove the
> > > builder
> > > > > > to
> > > > > > > simplify things. Otherwise, I think the builder is an important
> > > > > addition.
> > > > > > >
> > > > > > > Thoughts?
> > > > > > >
> > > > > > > On Thu, May 9, 2019 at 2:50 PM Colin McCabe <
> cmccabe@apache.org>
> > > > > wrote:
> > > > > > >
> > > > > > > > +1 (binding).
> > > > > > > >
> > > > > > > > Re: the builder discussion. I don't feel strongly either
> way--
> > > the
> > > > > > > > builder sketched out in the KIP looks reasonable, but I can
> > also
> > > > > > > understand
> > > > > > > > Ismael's argument for keeping the KIP minimal.
> > > > > > > >
> > > > > > > > best,
> > > > > > > > Colin
> > > > > > > >
> > > > > > > >
> > > > > > > > On Thu, May 9, 2019, at 08:09, Randall Hauch wrote:
> > > > > > > > > I'm fine with simplifying the KIP by removing the Builder
> > > (which
> > > > > > seems
> > > > > > > > > ancillary), or keeping the KIP as-is. I'll wait to vote
> until
> > > Almog
> > > > > > > says
> > > > > > > > > which way he'd like to proceed.
> > > > > > > > >
> > > > > > > > > On Thu, May 9, 2019 at 9:45 AM Ismael Juma <
> > ismael@juma.me.uk>
> > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Hi Almog,
> > > > > > > > > >
> > > > > > > > > > Adding a Builder seems unrelated to this change. Do we
> need
> > > it?
> > > > > > Given
> > > > > > > > the
> > > > > > > > > > imminent KIP deadline, I'd keep it simple and just have
> the
> > > > > > > constructor
> > > > > > > > > > with just the name parameter.
> > > > > > > > > >
> > > > > > > > > > Ismael
> > > > > > > > > >
> > > > > > > > > > On Thu, May 2, 2019 at 1:58 AM Mickael Maison <
> > > > > > > > mickael.maison@gmail.com>
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > I was planning to write a KIP for the exact same
> feature!
> > > > > > > > > > > +1 (non binding)
> > > > > > > > > > >
> > > > > > > > > > > Thanks for the KIP
> > > > > > > > > > >
> > > > > > > > > > > On Wed, May 1, 2019 at 7:24 PM Almog Gavra <
> > > almog@confluent.io
> > > > > >
> > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hello Everyone!
> > > > > > > > > > > >
> > > > > > > > > > > > Kicking off the voting for
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > You can see discussion thread here (please respond
> with
> > > > > > > > suggestions on
> > > > > > > > > > > that
> > > > > > > > > > > > thread):
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > >
> >
> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > Cheers,
> > > > > > > > > > > > Almog
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Manikumar <ma...@gmail.com>.
Hi Almog,
+1 (binding), Thanks for the KIP.
Thanks,
Manikumar
On Fri, May 10, 2019 at 10:02 PM Almog Gavra <al...@confluent.io> wrote:
> I'm happy pulling it out into a separate KIP to target the discussion. This
> one can just introduce the "default" constructor for no partitions or
> replicas since we'll need that one whether or not we add the builder.
>
> Updated the KIP moving the builder to a section in "Rejected Alternatives -
> Follow Up KIP" - @Randall since your +1 was for the KIP with builder I want
> to make sure that is okay with you.
>
> On Fri, May 10, 2019 at 9:14 AM Colin McCabe <cm...@apache.org> wrote:
>
> > Given that there are still some open questions about the builder, maybe
> we
> > should put it in a separate KIP?
> >
> > best,
> > Colin
> >
> >
> > On Fri, May 10, 2019, at 09:00, Ryanne Dolan wrote:
> > > +1 (non-binding) for the core feature, but I could take or leave the
> > > builder.
> > >
> > > Ryanne
> > >
> > > On Fri, May 10, 2019 at 10:43 AM Almog Gavra <al...@confluent.io>
> wrote:
> > >
> > > > @Ismael - I agree that the methods are a little random. They were
> just
> > > > ported from what's currently in the connect builder. I think a better
> > > > option might be to keep the connect builder around and have extend
> from
> > > > this builder, and make this builder only implement the "critical"
> > methods
> > > > (e.g. replicas/partitions/config). (cc Randall)
> > > >
> > > > Is passing optionals in the constructor something that's common in
> AK?
> > I
> > > > think my preference would be toward the builder so that it's easier
> to
> > > > extend as Randall mentioned.
> > > >
> > > > Almog
> > > >
> > > > On Fri, May 10, 2019 at 8:17 AM Ismael Juma <is...@juma.me.uk>
> wrote:
> > > >
> > > > > The current builder includes random methods like
> > uncleanLeaderElection.
> > > > > That doesn't make sense to me since it's a topic config (and we
> don't
> > > > > include methods for other topic configs). Also, I'm not sure about
> > the
> > > > > naming convention, should we have a `with` prefix? It would be good
> > to
> > > > > check existing builders in `clients` if any exist for what they're
> > doing.
> > > > >
> > > > > If we didn't add a builder, another option would be a single new
> > > > > constructor:
> > > > >
> > > > > public NewTopic(String name, Optional<Integer> numPartitions,
> > > > > Optional<Short> replicationFactor)
> > > > >
> > > > > Ismael
> > > > >
> > > > > On Thu, May 9, 2019 at 3:38 PM Almog Gavra <al...@confluent.io>
> > wrote:
> > > > >
> > > > > > Thanks Colin! Since the discussion around the builder is here
> I'll
> > copy
> > > > > > over my comment from the discuss thread:
> > > > > >
> > > > > > If we want the flexibility that the builder provides we would
> need
> > to
> > > > add
> > > > > > three constructors:
> > > > > > - no partitions/replicas
> > > > > > - just partitions
> > > > > > - just replicas
> > > > > >
> > > > > > I see good use cases for the first two - the third (just
> replicas)
> > > > seems
> > > > > > less necessary but complicates the API a bit (you have to
> > differentiate
> > > > > > NewTopic(int) with NewTopic(short) or something like that). If
> > we're
> > > > > happy
> > > > > > with a KIP that covers just the first two then I can remove the
> > builder
> > > > > to
> > > > > > simplify things. Otherwise, I think the builder is an important
> > > > addition.
> > > > > >
> > > > > > Thoughts?
> > > > > >
> > > > > > On Thu, May 9, 2019 at 2:50 PM Colin McCabe <cm...@apache.org>
> > > > wrote:
> > > > > >
> > > > > > > +1 (binding).
> > > > > > >
> > > > > > > Re: the builder discussion. I don't feel strongly either way--
> > the
> > > > > > > builder sketched out in the KIP looks reasonable, but I can
> also
> > > > > > understand
> > > > > > > Ismael's argument for keeping the KIP minimal.
> > > > > > >
> > > > > > > best,
> > > > > > > Colin
> > > > > > >
> > > > > > >
> > > > > > > On Thu, May 9, 2019, at 08:09, Randall Hauch wrote:
> > > > > > > > I'm fine with simplifying the KIP by removing the Builder
> > (which
> > > > > seems
> > > > > > > > ancillary), or keeping the KIP as-is. I'll wait to vote until
> > Almog
> > > > > > says
> > > > > > > > which way he'd like to proceed.
> > > > > > > >
> > > > > > > > On Thu, May 9, 2019 at 9:45 AM Ismael Juma <
> ismael@juma.me.uk>
> > > > > wrote:
> > > > > > > >
> > > > > > > > > Hi Almog,
> > > > > > > > >
> > > > > > > > > Adding a Builder seems unrelated to this change. Do we need
> > it?
> > > > > Given
> > > > > > > the
> > > > > > > > > imminent KIP deadline, I'd keep it simple and just have the
> > > > > > constructor
> > > > > > > > > with just the name parameter.
> > > > > > > > >
> > > > > > > > > Ismael
> > > > > > > > >
> > > > > > > > > On Thu, May 2, 2019 at 1:58 AM Mickael Maison <
> > > > > > > mickael.maison@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > I was planning to write a KIP for the exact same feature!
> > > > > > > > > > +1 (non binding)
> > > > > > > > > >
> > > > > > > > > > Thanks for the KIP
> > > > > > > > > >
> > > > > > > > > > On Wed, May 1, 2019 at 7:24 PM Almog Gavra <
> > almog@confluent.io
> > > > >
> > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hello Everyone!
> > > > > > > > > > >
> > > > > > > > > > > Kicking off the voting for
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > You can see discussion thread here (please respond with
> > > > > > > suggestions on
> > > > > > > > > > that
> > > > > > > > > > > thread):
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > Cheers,
> > > > > > > > > > > Almog
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Almog Gavra <al...@confluent.io>.
I'm happy pulling it out into a separate KIP to target the discussion. This
one can just introduce the "default" constructor for no partitions or
replicas since we'll need that one whether or not we add the builder.
Updated the KIP moving the builder to a section in "Rejected Alternatives -
Follow Up KIP" - @Randall since your +1 was for the KIP with builder I want
to make sure that is okay with you.
On Fri, May 10, 2019 at 9:14 AM Colin McCabe <cm...@apache.org> wrote:
> Given that there are still some open questions about the builder, maybe we
> should put it in a separate KIP?
>
> best,
> Colin
>
>
> On Fri, May 10, 2019, at 09:00, Ryanne Dolan wrote:
> > +1 (non-binding) for the core feature, but I could take or leave the
> > builder.
> >
> > Ryanne
> >
> > On Fri, May 10, 2019 at 10:43 AM Almog Gavra <al...@confluent.io> wrote:
> >
> > > @Ismael - I agree that the methods are a little random. They were just
> > > ported from what's currently in the connect builder. I think a better
> > > option might be to keep the connect builder around and have extend from
> > > this builder, and make this builder only implement the "critical"
> methods
> > > (e.g. replicas/partitions/config). (cc Randall)
> > >
> > > Is passing optionals in the constructor something that's common in AK?
> I
> > > think my preference would be toward the builder so that it's easier to
> > > extend as Randall mentioned.
> > >
> > > Almog
> > >
> > > On Fri, May 10, 2019 at 8:17 AM Ismael Juma <is...@juma.me.uk> wrote:
> > >
> > > > The current builder includes random methods like
> uncleanLeaderElection.
> > > > That doesn't make sense to me since it's a topic config (and we don't
> > > > include methods for other topic configs). Also, I'm not sure about
> the
> > > > naming convention, should we have a `with` prefix? It would be good
> to
> > > > check existing builders in `clients` if any exist for what they're
> doing.
> > > >
> > > > If we didn't add a builder, another option would be a single new
> > > > constructor:
> > > >
> > > > public NewTopic(String name, Optional<Integer> numPartitions,
> > > > Optional<Short> replicationFactor)
> > > >
> > > > Ismael
> > > >
> > > > On Thu, May 9, 2019 at 3:38 PM Almog Gavra <al...@confluent.io>
> wrote:
> > > >
> > > > > Thanks Colin! Since the discussion around the builder is here I'll
> copy
> > > > > over my comment from the discuss thread:
> > > > >
> > > > > If we want the flexibility that the builder provides we would need
> to
> > > add
> > > > > three constructors:
> > > > > - no partitions/replicas
> > > > > - just partitions
> > > > > - just replicas
> > > > >
> > > > > I see good use cases for the first two - the third (just replicas)
> > > seems
> > > > > less necessary but complicates the API a bit (you have to
> differentiate
> > > > > NewTopic(int) with NewTopic(short) or something like that). If
> we're
> > > > happy
> > > > > with a KIP that covers just the first two then I can remove the
> builder
> > > > to
> > > > > simplify things. Otherwise, I think the builder is an important
> > > addition.
> > > > >
> > > > > Thoughts?
> > > > >
> > > > > On Thu, May 9, 2019 at 2:50 PM Colin McCabe <cm...@apache.org>
> > > wrote:
> > > > >
> > > > > > +1 (binding).
> > > > > >
> > > > > > Re: the builder discussion. I don't feel strongly either way--
> the
> > > > > > builder sketched out in the KIP looks reasonable, but I can also
> > > > > understand
> > > > > > Ismael's argument for keeping the KIP minimal.
> > > > > >
> > > > > > best,
> > > > > > Colin
> > > > > >
> > > > > >
> > > > > > On Thu, May 9, 2019, at 08:09, Randall Hauch wrote:
> > > > > > > I'm fine with simplifying the KIP by removing the Builder
> (which
> > > > seems
> > > > > > > ancillary), or keeping the KIP as-is. I'll wait to vote until
> Almog
> > > > > says
> > > > > > > which way he'd like to proceed.
> > > > > > >
> > > > > > > On Thu, May 9, 2019 at 9:45 AM Ismael Juma <is...@juma.me.uk>
> > > > wrote:
> > > > > > >
> > > > > > > > Hi Almog,
> > > > > > > >
> > > > > > > > Adding a Builder seems unrelated to this change. Do we need
> it?
> > > > Given
> > > > > > the
> > > > > > > > imminent KIP deadline, I'd keep it simple and just have the
> > > > > constructor
> > > > > > > > with just the name parameter.
> > > > > > > >
> > > > > > > > Ismael
> > > > > > > >
> > > > > > > > On Thu, May 2, 2019 at 1:58 AM Mickael Maison <
> > > > > > mickael.maison@gmail.com>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > I was planning to write a KIP for the exact same feature!
> > > > > > > > > +1 (non binding)
> > > > > > > > >
> > > > > > > > > Thanks for the KIP
> > > > > > > > >
> > > > > > > > > On Wed, May 1, 2019 at 7:24 PM Almog Gavra <
> almog@confluent.io
> > > >
> > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hello Everyone!
> > > > > > > > > >
> > > > > > > > > > Kicking off the voting for
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > You can see discussion thread here (please respond with
> > > > > > suggestions on
> > > > > > > > > that
> > > > > > > > > > thread):
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Cheers,
> > > > > > > > > > Almog
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Colin McCabe <cm...@apache.org>.
Given that there are still some open questions about the builder, maybe we should put it in a separate KIP?
best,
Colin
On Fri, May 10, 2019, at 09:00, Ryanne Dolan wrote:
> +1 (non-binding) for the core feature, but I could take or leave the
> builder.
>
> Ryanne
>
> On Fri, May 10, 2019 at 10:43 AM Almog Gavra <al...@confluent.io> wrote:
>
> > @Ismael - I agree that the methods are a little random. They were just
> > ported from what's currently in the connect builder. I think a better
> > option might be to keep the connect builder around and have extend from
> > this builder, and make this builder only implement the "critical" methods
> > (e.g. replicas/partitions/config). (cc Randall)
> >
> > Is passing optionals in the constructor something that's common in AK? I
> > think my preference would be toward the builder so that it's easier to
> > extend as Randall mentioned.
> >
> > Almog
> >
> > On Fri, May 10, 2019 at 8:17 AM Ismael Juma <is...@juma.me.uk> wrote:
> >
> > > The current builder includes random methods like uncleanLeaderElection.
> > > That doesn't make sense to me since it's a topic config (and we don't
> > > include methods for other topic configs). Also, I'm not sure about the
> > > naming convention, should we have a `with` prefix? It would be good to
> > > check existing builders in `clients` if any exist for what they're doing.
> > >
> > > If we didn't add a builder, another option would be a single new
> > > constructor:
> > >
> > > public NewTopic(String name, Optional<Integer> numPartitions,
> > > Optional<Short> replicationFactor)
> > >
> > > Ismael
> > >
> > > On Thu, May 9, 2019 at 3:38 PM Almog Gavra <al...@confluent.io> wrote:
> > >
> > > > Thanks Colin! Since the discussion around the builder is here I'll copy
> > > > over my comment from the discuss thread:
> > > >
> > > > If we want the flexibility that the builder provides we would need to
> > add
> > > > three constructors:
> > > > - no partitions/replicas
> > > > - just partitions
> > > > - just replicas
> > > >
> > > > I see good use cases for the first two - the third (just replicas)
> > seems
> > > > less necessary but complicates the API a bit (you have to differentiate
> > > > NewTopic(int) with NewTopic(short) or something like that). If we're
> > > happy
> > > > with a KIP that covers just the first two then I can remove the builder
> > > to
> > > > simplify things. Otherwise, I think the builder is an important
> > addition.
> > > >
> > > > Thoughts?
> > > >
> > > > On Thu, May 9, 2019 at 2:50 PM Colin McCabe <cm...@apache.org>
> > wrote:
> > > >
> > > > > +1 (binding).
> > > > >
> > > > > Re: the builder discussion. I don't feel strongly either way-- the
> > > > > builder sketched out in the KIP looks reasonable, but I can also
> > > > understand
> > > > > Ismael's argument for keeping the KIP minimal.
> > > > >
> > > > > best,
> > > > > Colin
> > > > >
> > > > >
> > > > > On Thu, May 9, 2019, at 08:09, Randall Hauch wrote:
> > > > > > I'm fine with simplifying the KIP by removing the Builder (which
> > > seems
> > > > > > ancillary), or keeping the KIP as-is. I'll wait to vote until Almog
> > > > says
> > > > > > which way he'd like to proceed.
> > > > > >
> > > > > > On Thu, May 9, 2019 at 9:45 AM Ismael Juma <is...@juma.me.uk>
> > > wrote:
> > > > > >
> > > > > > > Hi Almog,
> > > > > > >
> > > > > > > Adding a Builder seems unrelated to this change. Do we need it?
> > > Given
> > > > > the
> > > > > > > imminent KIP deadline, I'd keep it simple and just have the
> > > > constructor
> > > > > > > with just the name parameter.
> > > > > > >
> > > > > > > Ismael
> > > > > > >
> > > > > > > On Thu, May 2, 2019 at 1:58 AM Mickael Maison <
> > > > > mickael.maison@gmail.com>
> > > > > > > wrote:
> > > > > > >
> > > > > > > > I was planning to write a KIP for the exact same feature!
> > > > > > > > +1 (non binding)
> > > > > > > >
> > > > > > > > Thanks for the KIP
> > > > > > > >
> > > > > > > > On Wed, May 1, 2019 at 7:24 PM Almog Gavra <almog@confluent.io
> > >
> > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hello Everyone!
> > > > > > > > >
> > > > > > > > > Kicking off the voting for
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > >
> > > >
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > You can see discussion thread here (please respond with
> > > > > suggestions on
> > > > > > > > that
> > > > > > > > > thread):
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > >
> > > >
> > >
> > https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Cheers,
> > > > > > > > > Almog
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Ryanne Dolan <ry...@gmail.com>.
+1 (non-binding) for the core feature, but I could take or leave the
builder.
Ryanne
On Fri, May 10, 2019 at 10:43 AM Almog Gavra <al...@confluent.io> wrote:
> @Ismael - I agree that the methods are a little random. They were just
> ported from what's currently in the connect builder. I think a better
> option might be to keep the connect builder around and have extend from
> this builder, and make this builder only implement the "critical" methods
> (e.g. replicas/partitions/config). (cc Randall)
>
> Is passing optionals in the constructor something that's common in AK? I
> think my preference would be toward the builder so that it's easier to
> extend as Randall mentioned.
>
> Almog
>
> On Fri, May 10, 2019 at 8:17 AM Ismael Juma <is...@juma.me.uk> wrote:
>
> > The current builder includes random methods like uncleanLeaderElection.
> > That doesn't make sense to me since it's a topic config (and we don't
> > include methods for other topic configs). Also, I'm not sure about the
> > naming convention, should we have a `with` prefix? It would be good to
> > check existing builders in `clients` if any exist for what they're doing.
> >
> > If we didn't add a builder, another option would be a single new
> > constructor:
> >
> > public NewTopic(String name, Optional<Integer> numPartitions,
> > Optional<Short> replicationFactor)
> >
> > Ismael
> >
> > On Thu, May 9, 2019 at 3:38 PM Almog Gavra <al...@confluent.io> wrote:
> >
> > > Thanks Colin! Since the discussion around the builder is here I'll copy
> > > over my comment from the discuss thread:
> > >
> > > If we want the flexibility that the builder provides we would need to
> add
> > > three constructors:
> > > - no partitions/replicas
> > > - just partitions
> > > - just replicas
> > >
> > > I see good use cases for the first two - the third (just replicas)
> seems
> > > less necessary but complicates the API a bit (you have to differentiate
> > > NewTopic(int) with NewTopic(short) or something like that). If we're
> > happy
> > > with a KIP that covers just the first two then I can remove the builder
> > to
> > > simplify things. Otherwise, I think the builder is an important
> addition.
> > >
> > > Thoughts?
> > >
> > > On Thu, May 9, 2019 at 2:50 PM Colin McCabe <cm...@apache.org>
> wrote:
> > >
> > > > +1 (binding).
> > > >
> > > > Re: the builder discussion. I don't feel strongly either way-- the
> > > > builder sketched out in the KIP looks reasonable, but I can also
> > > understand
> > > > Ismael's argument for keeping the KIP minimal.
> > > >
> > > > best,
> > > > Colin
> > > >
> > > >
> > > > On Thu, May 9, 2019, at 08:09, Randall Hauch wrote:
> > > > > I'm fine with simplifying the KIP by removing the Builder (which
> > seems
> > > > > ancillary), or keeping the KIP as-is. I'll wait to vote until Almog
> > > says
> > > > > which way he'd like to proceed.
> > > > >
> > > > > On Thu, May 9, 2019 at 9:45 AM Ismael Juma <is...@juma.me.uk>
> > wrote:
> > > > >
> > > > > > Hi Almog,
> > > > > >
> > > > > > Adding a Builder seems unrelated to this change. Do we need it?
> > Given
> > > > the
> > > > > > imminent KIP deadline, I'd keep it simple and just have the
> > > constructor
> > > > > > with just the name parameter.
> > > > > >
> > > > > > Ismael
> > > > > >
> > > > > > On Thu, May 2, 2019 at 1:58 AM Mickael Maison <
> > > > mickael.maison@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > I was planning to write a KIP for the exact same feature!
> > > > > > > +1 (non binding)
> > > > > > >
> > > > > > > Thanks for the KIP
> > > > > > >
> > > > > > > On Wed, May 1, 2019 at 7:24 PM Almog Gavra <almog@confluent.io
> >
> > > > wrote:
> > > > > > > >
> > > > > > > > Hello Everyone!
> > > > > > > >
> > > > > > > > Kicking off the voting for
> > > > > > > >
> > > > > > >
> > > > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
> > > > > > > >
> > > > > > > >
> > > > > > > > You can see discussion thread here (please respond with
> > > > suggestions on
> > > > > > > that
> > > > > > > > thread):
> > > > > > > >
> > > > > > >
> > > > > >
> > > >
> > >
> >
> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
> > > > > > > >
> > > > > > > >
> > > > > > > > Cheers,
> > > > > > > > Almog
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Almog Gavra <al...@confluent.io>.
@Ismael - I agree that the methods are a little random. They were just
ported from what's currently in the connect builder. I think a better
option might be to keep the connect builder around and have extend from
this builder, and make this builder only implement the "critical" methods
(e.g. replicas/partitions/config). (cc Randall)
Is passing optionals in the constructor something that's common in AK? I
think my preference would be toward the builder so that it's easier to
extend as Randall mentioned.
Almog
On Fri, May 10, 2019 at 8:17 AM Ismael Juma <is...@juma.me.uk> wrote:
> The current builder includes random methods like uncleanLeaderElection.
> That doesn't make sense to me since it's a topic config (and we don't
> include methods for other topic configs). Also, I'm not sure about the
> naming convention, should we have a `with` prefix? It would be good to
> check existing builders in `clients` if any exist for what they're doing.
>
> If we didn't add a builder, another option would be a single new
> constructor:
>
> public NewTopic(String name, Optional<Integer> numPartitions,
> Optional<Short> replicationFactor)
>
> Ismael
>
> On Thu, May 9, 2019 at 3:38 PM Almog Gavra <al...@confluent.io> wrote:
>
> > Thanks Colin! Since the discussion around the builder is here I'll copy
> > over my comment from the discuss thread:
> >
> > If we want the flexibility that the builder provides we would need to add
> > three constructors:
> > - no partitions/replicas
> > - just partitions
> > - just replicas
> >
> > I see good use cases for the first two - the third (just replicas) seems
> > less necessary but complicates the API a bit (you have to differentiate
> > NewTopic(int) with NewTopic(short) or something like that). If we're
> happy
> > with a KIP that covers just the first two then I can remove the builder
> to
> > simplify things. Otherwise, I think the builder is an important addition.
> >
> > Thoughts?
> >
> > On Thu, May 9, 2019 at 2:50 PM Colin McCabe <cm...@apache.org> wrote:
> >
> > > +1 (binding).
> > >
> > > Re: the builder discussion. I don't feel strongly either way-- the
> > > builder sketched out in the KIP looks reasonable, but I can also
> > understand
> > > Ismael's argument for keeping the KIP minimal.
> > >
> > > best,
> > > Colin
> > >
> > >
> > > On Thu, May 9, 2019, at 08:09, Randall Hauch wrote:
> > > > I'm fine with simplifying the KIP by removing the Builder (which
> seems
> > > > ancillary), or keeping the KIP as-is. I'll wait to vote until Almog
> > says
> > > > which way he'd like to proceed.
> > > >
> > > > On Thu, May 9, 2019 at 9:45 AM Ismael Juma <is...@juma.me.uk>
> wrote:
> > > >
> > > > > Hi Almog,
> > > > >
> > > > > Adding a Builder seems unrelated to this change. Do we need it?
> Given
> > > the
> > > > > imminent KIP deadline, I'd keep it simple and just have the
> > constructor
> > > > > with just the name parameter.
> > > > >
> > > > > Ismael
> > > > >
> > > > > On Thu, May 2, 2019 at 1:58 AM Mickael Maison <
> > > mickael.maison@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > I was planning to write a KIP for the exact same feature!
> > > > > > +1 (non binding)
> > > > > >
> > > > > > Thanks for the KIP
> > > > > >
> > > > > > On Wed, May 1, 2019 at 7:24 PM Almog Gavra <al...@confluent.io>
> > > wrote:
> > > > > > >
> > > > > > > Hello Everyone!
> > > > > > >
> > > > > > > Kicking off the voting for
> > > > > > >
> > > > > >
> > > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
> > > > > > >
> > > > > > >
> > > > > > > You can see discussion thread here (please respond with
> > > suggestions on
> > > > > > that
> > > > > > > thread):
> > > > > > >
> > > > > >
> > > > >
> > >
> >
> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
> > > > > > >
> > > > > > >
> > > > > > > Cheers,
> > > > > > > Almog
> > > > > >
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Ismael Juma <is...@juma.me.uk>.
The current builder includes random methods like uncleanLeaderElection.
That doesn't make sense to me since it's a topic config (and we don't
include methods for other topic configs). Also, I'm not sure about the
naming convention, should we have a `with` prefix? It would be good to
check existing builders in `clients` if any exist for what they're doing.
If we didn't add a builder, another option would be a single new
constructor:
public NewTopic(String name, Optional<Integer> numPartitions,
Optional<Short> replicationFactor)
Ismael
On Thu, May 9, 2019 at 3:38 PM Almog Gavra <al...@confluent.io> wrote:
> Thanks Colin! Since the discussion around the builder is here I'll copy
> over my comment from the discuss thread:
>
> If we want the flexibility that the builder provides we would need to add
> three constructors:
> - no partitions/replicas
> - just partitions
> - just replicas
>
> I see good use cases for the first two - the third (just replicas) seems
> less necessary but complicates the API a bit (you have to differentiate
> NewTopic(int) with NewTopic(short) or something like that). If we're happy
> with a KIP that covers just the first two then I can remove the builder to
> simplify things. Otherwise, I think the builder is an important addition.
>
> Thoughts?
>
> On Thu, May 9, 2019 at 2:50 PM Colin McCabe <cm...@apache.org> wrote:
>
> > +1 (binding).
> >
> > Re: the builder discussion. I don't feel strongly either way-- the
> > builder sketched out in the KIP looks reasonable, but I can also
> understand
> > Ismael's argument for keeping the KIP minimal.
> >
> > best,
> > Colin
> >
> >
> > On Thu, May 9, 2019, at 08:09, Randall Hauch wrote:
> > > I'm fine with simplifying the KIP by removing the Builder (which seems
> > > ancillary), or keeping the KIP as-is. I'll wait to vote until Almog
> says
> > > which way he'd like to proceed.
> > >
> > > On Thu, May 9, 2019 at 9:45 AM Ismael Juma <is...@juma.me.uk> wrote:
> > >
> > > > Hi Almog,
> > > >
> > > > Adding a Builder seems unrelated to this change. Do we need it? Given
> > the
> > > > imminent KIP deadline, I'd keep it simple and just have the
> constructor
> > > > with just the name parameter.
> > > >
> > > > Ismael
> > > >
> > > > On Thu, May 2, 2019 at 1:58 AM Mickael Maison <
> > mickael.maison@gmail.com>
> > > > wrote:
> > > >
> > > > > I was planning to write a KIP for the exact same feature!
> > > > > +1 (non binding)
> > > > >
> > > > > Thanks for the KIP
> > > > >
> > > > > On Wed, May 1, 2019 at 7:24 PM Almog Gavra <al...@confluent.io>
> > wrote:
> > > > > >
> > > > > > Hello Everyone!
> > > > > >
> > > > > > Kicking off the voting for
> > > > > >
> > > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
> > > > > >
> > > > > >
> > > > > > You can see discussion thread here (please respond with
> > suggestions on
> > > > > that
> > > > > > thread):
> > > > > >
> > > > >
> > > >
> >
> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
> > > > > >
> > > > > >
> > > > > > Cheers,
> > > > > > Almog
> > > > >
> > > >
> > >
> >
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Almog Gavra <al...@confluent.io>.
Thanks Colin! Since the discussion around the builder is here I'll copy
over my comment from the discuss thread:
If we want the flexibility that the builder provides we would need to add
three constructors:
- no partitions/replicas
- just partitions
- just replicas
I see good use cases for the first two - the third (just replicas) seems
less necessary but complicates the API a bit (you have to differentiate
NewTopic(int) with NewTopic(short) or something like that). If we're happy
with a KIP that covers just the first two then I can remove the builder to
simplify things. Otherwise, I think the builder is an important addition.
Thoughts?
On Thu, May 9, 2019 at 2:50 PM Colin McCabe <cm...@apache.org> wrote:
> +1 (binding).
>
> Re: the builder discussion. I don't feel strongly either way-- the
> builder sketched out in the KIP looks reasonable, but I can also understand
> Ismael's argument for keeping the KIP minimal.
>
> best,
> Colin
>
>
> On Thu, May 9, 2019, at 08:09, Randall Hauch wrote:
> > I'm fine with simplifying the KIP by removing the Builder (which seems
> > ancillary), or keeping the KIP as-is. I'll wait to vote until Almog says
> > which way he'd like to proceed.
> >
> > On Thu, May 9, 2019 at 9:45 AM Ismael Juma <is...@juma.me.uk> wrote:
> >
> > > Hi Almog,
> > >
> > > Adding a Builder seems unrelated to this change. Do we need it? Given
> the
> > > imminent KIP deadline, I'd keep it simple and just have the constructor
> > > with just the name parameter.
> > >
> > > Ismael
> > >
> > > On Thu, May 2, 2019 at 1:58 AM Mickael Maison <
> mickael.maison@gmail.com>
> > > wrote:
> > >
> > > > I was planning to write a KIP for the exact same feature!
> > > > +1 (non binding)
> > > >
> > > > Thanks for the KIP
> > > >
> > > > On Wed, May 1, 2019 at 7:24 PM Almog Gavra <al...@confluent.io>
> wrote:
> > > > >
> > > > > Hello Everyone!
> > > > >
> > > > > Kicking off the voting for
> > > > >
> > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
> > > > >
> > > > >
> > > > > You can see discussion thread here (please respond with
> suggestions on
> > > > that
> > > > > thread):
> > > > >
> > > >
> > >
> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
> > > > >
> > > > >
> > > > > Cheers,
> > > > > Almog
> > > >
> > >
> >
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Colin McCabe <cm...@apache.org>.
+1 (binding).
Re: the builder discussion. I don't feel strongly either way-- the builder sketched out in the KIP looks reasonable, but I can also understand Ismael's argument for keeping the KIP minimal.
best,
Colin
On Thu, May 9, 2019, at 08:09, Randall Hauch wrote:
> I'm fine with simplifying the KIP by removing the Builder (which seems
> ancillary), or keeping the KIP as-is. I'll wait to vote until Almog says
> which way he'd like to proceed.
>
> On Thu, May 9, 2019 at 9:45 AM Ismael Juma <is...@juma.me.uk> wrote:
>
> > Hi Almog,
> >
> > Adding a Builder seems unrelated to this change. Do we need it? Given the
> > imminent KIP deadline, I'd keep it simple and just have the constructor
> > with just the name parameter.
> >
> > Ismael
> >
> > On Thu, May 2, 2019 at 1:58 AM Mickael Maison <mi...@gmail.com>
> > wrote:
> >
> > > I was planning to write a KIP for the exact same feature!
> > > +1 (non binding)
> > >
> > > Thanks for the KIP
> > >
> > > On Wed, May 1, 2019 at 7:24 PM Almog Gavra <al...@confluent.io> wrote:
> > > >
> > > > Hello Everyone!
> > > >
> > > > Kicking off the voting for
> > > >
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
> > > >
> > > >
> > > > You can see discussion thread here (please respond with suggestions on
> > > that
> > > > thread):
> > > >
> > >
> > https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
> > > >
> > > >
> > > > Cheers,
> > > > Almog
> > >
> >
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Randall Hauch <rh...@gmail.com>.
I'm fine with simplifying the KIP by removing the Builder (which seems
ancillary), or keeping the KIP as-is. I'll wait to vote until Almog says
which way he'd like to proceed.
On Thu, May 9, 2019 at 9:45 AM Ismael Juma <is...@juma.me.uk> wrote:
> Hi Almog,
>
> Adding a Builder seems unrelated to this change. Do we need it? Given the
> imminent KIP deadline, I'd keep it simple and just have the constructor
> with just the name parameter.
>
> Ismael
>
> On Thu, May 2, 2019 at 1:58 AM Mickael Maison <mi...@gmail.com>
> wrote:
>
> > I was planning to write a KIP for the exact same feature!
> > +1 (non binding)
> >
> > Thanks for the KIP
> >
> > On Wed, May 1, 2019 at 7:24 PM Almog Gavra <al...@confluent.io> wrote:
> > >
> > > Hello Everyone!
> > >
> > > Kicking off the voting for
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
> > >
> > >
> > > You can see discussion thread here (please respond with suggestions on
> > that
> > > thread):
> > >
> >
> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
> > >
> > >
> > > Cheers,
> > > Almog
> >
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Ismael Juma <is...@juma.me.uk>.
Hi Almog,
Adding a Builder seems unrelated to this change. Do we need it? Given the
imminent KIP deadline, I'd keep it simple and just have the constructor
with just the name parameter.
Ismael
On Thu, May 2, 2019 at 1:58 AM Mickael Maison <mi...@gmail.com>
wrote:
> I was planning to write a KIP for the exact same feature!
> +1 (non binding)
>
> Thanks for the KIP
>
> On Wed, May 1, 2019 at 7:24 PM Almog Gavra <al...@confluent.io> wrote:
> >
> > Hello Everyone!
> >
> > Kicking off the voting for
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
> >
> >
> > You can see discussion thread here (please respond with suggestions on
> that
> > thread):
> >
> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
> >
> >
> > Cheers,
> > Almog
>
Re: [VOTE] KIP-464: Defaults for AdminClient#createTopic
Posted by Mickael Maison <mi...@gmail.com>.
I was planning to write a KIP for the exact same feature!
+1 (non binding)
Thanks for the KIP
On Wed, May 1, 2019 at 7:24 PM Almog Gavra <al...@confluent.io> wrote:
>
> Hello Everyone!
>
> Kicking off the voting for
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-464%3A+Defaults+for+AdminClient%23createTopic
>
>
> You can see discussion thread here (please respond with suggestions on that
> thread):
> https://lists.apache.org/thread.html/c0adfd2457e5984be7471fe6ade8a94d52c647356c81c039445d6b34@%3Cdev.kafka.apache.org%3E
>
>
> Cheers,
> Almog