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 <cmcc...@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 <ism...@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.mai...@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 > > > > > > > > > > > > > > >
