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 >
