Sounds good to me. Ismael
On Thu, Jul 27, 2023, 6:32 PM Colin McCabe <cmcc...@apache.org> wrote: > I would incline towards plural, but I honestly don't feel that strongly > about it. I will just change it to --bootstrap-controller for now, to match > --bootstrap-server. Perhaps we can discuss this further in a later KIP, if > people are inclined... > > Colin > > > On Thu, Jul 27, 2023, at 07:13, Ron Dagostino wrote: > >> support plural for both and the singular one is just an alias for > compatibility > > > > I like that approach. I can never remember what works, so I would > > prefer to just use what I think works and then have it work. > > > > Ron > > > > On Thu, Jul 27, 2023 at 8:33 AM Ismael Juma <m...@ismaeljuma.com> wrote: > >> > >> I think singular was used because that's the common case for the cli > tools. > >> To be honest, it's actually a bit more confusing to have both singular > and > >> plural for the cli tools and you have to remember the exact version for > >> each one. We should either support plural for both and the singular one > is > >> just an alias for compatibility or stick with singular for both. > >> > >> Ismael > >> > >> On Thu, Jul 27, 2023, 12:03 AM Colin McCabe <cmcc...@apache.org> wrote: > >> > >> > On Wed, Jul 26, 2023, at 07:09, Ron Dagostino wrote: > >> > > Thanks, Colin. +1 (binding) from me. > >> > > > >> > > I will note that Ziming mentioned in the DISCUSS thread that "There > is > >> > > a mistake that we use `--bootstrap-server` instead of > >> > > `--bootstrap-server(s)`, so should we also change the new argument > >> > > `--bootstrap-controller` (no s).". I agree that this is an > >> > > unfortunate historical artifact. Furthermore, the config property > is > >> > > bootstrap.servers, which does not match the command line argument > >> > > --bootstrap-server. I don't know what we should do here -- I am not > >> > > sure that we should propagate it. We could continue the same > mismatch > >> > > and use --bootstrap-controller, but I feel like using > >> > > --bootstrap-controllers makes more sense. Anyway, this is more an > >> > > ergonomic issue than anything else. I am still +1 binding, but we > >> > > should arrive at an explicit decision here. > >> > > >> > Hi Ron, > >> > > >> > Thanks for the review and vote. > >> > > >> > The current KIP-919 text has "--bootstrap-controllers" for > command-line > >> > and "bootstrap.controllers" for config. > >> > > >> > As far as I know, --bootstrap-server is singular rather than plural > just > >> > for historical reasons.... so I figured the new flag can just be > plural. As > >> > you mentioned, the configs have always been plural, so it's more > consistent > >> > for the flags to match the configs. > >> > > >> > (Though obviously that ship has sailed on --bootstrap-server, we're > not > >> > gonna change it now ;) > >> > > >> > > > >> > > Nits: > >> > > s/boostrap-servers/bootstrap-servers/ > >> > > s/DescribeClusterResquest/DescribeClusterRequest/ > >> > > s/ControllerRegistrationRecord/RegisterControllerRecord/g > >> > > > >> > > >> > Thanks. I fixed these typos and changed ControllerRegistrationRecord > -> > >> > RegisterControllerRecord as suggested. > >> > > >> > best, > >> > Colin > >> > > >> > > >> > > Ron > >> > > > >> > > On Wed, Jul 26, 2023 at 10:06 AM David Arthur > >> > > <david.art...@confluent.io.invalid> wrote: > >> > >> > >> > >> Thanks for driving this KIP, Colin! > >> > >> > >> > >> +1 binding > >> > >> > >> > >> -David > >> > >> > >> > >> On Wed, Jul 26, 2023 at 8:58 AM Divij Vaidya < > divijvaidy...@gmail.com> > >> > >> wrote: > >> > >> > >> > >> > +1 (binding) > >> > >> > > >> > >> > -- > >> > >> > Divij Vaidya > >> > >> > > >> > >> > > >> > >> > On Wed, Jul 26, 2023 at 2:56 PM ziming deng < > dengziming1...@gmail.com > >> > > > >> > >> > wrote: > >> > >> > > > >> > >> > > +1 (binding) from me. > >> > >> > > > >> > >> > > Thanks for the KIP! > >> > >> > > > >> > >> > > -- > >> > >> > > Ziming > >> > >> > > > >> > >> > > > On Jul 26, 2023, at 20:18, Luke Chen <show...@gmail.com> > wrote: > >> > >> > > > > >> > >> > > > +1 (binding) from me. > >> > >> > > > > >> > >> > > > Thanks for the KIP! > >> > >> > > > > >> > >> > > > Luke > >> > >> > > > > >> > >> > > > On Tue, Jul 25, 2023 at 1:24 AM Colin McCabe < > cmcc...@apache.org> > >> > >> > wrote: > >> > >> > > > > >> > >> > > >> Hi all, > >> > >> > > >> > >> > >> > > >> I'd like to start the vote for KIP-919: Allow AdminClient > to Talk > >> > >> > Directly > >> > >> > > >> with the KRaft Controller Quorum and add Controller > Registration. > >> > >> > > >> > >> > >> > > >> The KIP is here: > https://cwiki.apache.org/confluence/x/Owo0Dw > >> > >> > > >> > >> > >> > > >> Thanks to everyone who reviewed the proposal. > >> > >> > > >> > >> > >> > > >> best, > >> > >> > > >> Colin > >> > >> > > >> > >> > >> > > > >> > >> > > >> > >> > >> > >> > >> > >> -- > >> > >> -David > >> > >
