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 >> >
