I agree. --execute is inconsistent with the way UNIX commands normally work. Normally, if you don't want something to be executed, you pass something like a --dry-run flag. Users expect a command to be run when the run it... that's the reason why this has created so much confusion for Kafka users.
I certainly don't think we should add an --execute option for any other commands. I was just suggesting that we could have a transitional period when running this particular command command without --execute would prompt the user, and then do the operation if they agreed. After a few releases, we can remove the prompt and make this consistent with the other commands. On Mon, Feb 26, 2018, at 14:44, Matthias J. Sax wrote: > I agree on consistency, too. > > However, I am not sure if we should introduce an explicit --execute > option. Anybody familiar with Linux tools will expect a command to > execute by default. > > Thus, I would suggest to remove --execute for all tools that use this > option atm. > > Btw: there is a related Jira: > https://issues.apache.org/jira/browse/KAFKA-1299 > > Furthermore, this also affect arguments like > > --bootstrap-servers > vs > --broker-list > > and maybe others. > > IMHO, all tools should use the same names. Thus, it's a larger change... > But totally worth doing it. Yeah. This is another case where we could have a transitional period. Support both the old and the new flag for a while, then transition to the new flag. best, Colin > > > -Matthias > > On 2/26/18 10:09 AM, Guozhang Wang wrote: > > Hi Jorge, > > > > I agree on being consistent across our tools. > > > > Besides the kafka-consumer-groups and kafka-streams-application-reset, a > > couple of other classes to consider adding the --execute options for the > > next major release: > > > > 1. kafka-preferred-replica-elections > > 2. kafka-reassign-partitions > > 3. kafka-delete-records > > 4. kafka-topics > > 5. kafka-acls > > 6. kafka-configs > > 7. kafka-delegation-tokens > > > > > > Guozhang > > > > On Mon, Feb 26, 2018 at 3:03 AM, Jorge Esteban Quilcate Otoya < > > quilcate.jo...@gmail.com> wrote: > > > >> Hi all, > >> > >> Thanks for the feedback. > >> > >> I have updated the "Compatibility, Deprecation, and Migration Plan" section > >> to document this to support the rollback. I probably should have handled > >> this change, as small as it looks, as a new KIP to avoid this issue. > >> > >> I like Colin's idea about asking for confirmation, although I'm not sure if > >> another tool has already this behavior and could create more confusion > >> (e.g. why this command ask for confirmation and others don't). Maybe we > >> will require a more broad looks at the CLI tools to agree on this? > >> > >> Jorge. > >> > >> El jue., 22 feb. 2018 a las 21:09, Guozhang Wang (<wangg...@gmail.com>) > >> escribió: > >> > >>> Yup, agreed. > >>> > >>> On Thu, Feb 22, 2018 at 11:46 AM, Ismael Juma <ism...@juma.me.uk> wrote: > >>> > >>>> Hi Guozhang, > >>>> > >>>> To clarify my comment: any change with a backwards compatibility impact > >>>> should be mentioned in the "Compatibility, Deprecation, and Migration > >>> Plan" > >>>> section (in addition to the deprecation period and only happening in a > >>>> major release as you said). > >>>> > >>>> Ismael > >>>> > >>>> On Thu, Feb 22, 2018 at 11:10 AM, Guozhang Wang <wangg...@gmail.com> > >>>> wrote: > >>>> > >>>>> Just to clarify, the KIP itself has mentioned about the change so the > >>> PR > >>>>> was not un-intentional: > >>>>> > >>>>> " > >>>>> > >>>>> 3. Keep execution parameters uniform between both tools: It will > >>> execute > >>>> by > >>>>> default, and have a `dry-run` parameter just show the results. This > >>> will > >>>>> involve change current `ConsumerGroupCommand` to change execution > >>>> options. > >>>>> > >>>>> " > >>>>> > >>>>> We were agreed that the proposed change is better than the current > >>>> status, > >>>>> since may people not using "--execute" on consumer reset tool were > >>>> actually > >>>>> surprised that nothing gets executed. What we were concerning as a > >>>>> hind-sight is that instead of doing such change in a minor release > >> like > >>>>> 1.1, we should consider only doing that in the next major release as > >> it > >>>>> breaks compatibility. In the past when we are going to remove / > >> replace > >>>>> certain option we would first add a going-to-be-deprecated warning in > >>> the > >>>>> previous releases until it was finally removed. So Jason's suggestion > >>> is > >>>> to > >>>>> do the same: we are not reverting this change forever, but trying to > >>>> delay > >>>>> it after 1.1. > >>>>> > >>>>> > >>>>> Guozhang > >>>>> > >>>>> > >>>>> On Thu, Feb 22, 2018 at 10:56 AM, Colin McCabe <cmcc...@apache.org> > >>>> wrote: > >>>>> > >>>>>> Perhaps, if the user doesn't pass the --execute flag, the tool > >> should > >>>>>> print a prompt like "would you like to perform this reset?" and > >> wait > >>>> for > >>>>> a > >>>>>> Y / N (or yes or no) input from the command-line. Then, if the > >>>> --execute > >>>>>> flag is passed, we skip this. That seems 99% compatible, and also > >>>>>> accomplishes the goal of making the tool less confusing. > >>>>>> > >>>>>> best, > >>>>>> Colin > >>>>>> > >>>>>> > >>>>>> On Thu, Feb 22, 2018, at 10:23, Ismael Juma wrote: > >>>>>>> Yes, let's revert the incompatible changes. There was no mention > >> of > >>>>>>> compatibility impact on the KIP and we should ensure that is the > >>> case > >>>>> for > >>>>>>> 1.1.0. > >>>>>>> > >>>>>>> Ismael > >>>>>>> > >>>>>>> On Thu, Feb 22, 2018 at 9:55 AM, Jason Gustafson < > >>> ja...@confluent.io > >>>>> > >>>>>> wrote: > >>>>>>> > >>>>>>>> I know it's a been a while since this vote passed, but I think > >> we > >>>>> need > >>>>>> to > >>>>>>>> reconsider the incompatible changes to the consumer reset tool. > >>>>>>>> Specifically, we have removed the --execute option without > >>>>> deprecating > >>>>>> it > >>>>>>>> first, and we have changed the default behavior to execute > >> rather > >>>>> than > >>>>>> do a > >>>>>>>> dry run. The latter in particular seems dangerous since users > >> who > >>>>> were > >>>>>>>> previously using the default behavior to view offsets will now > >>>>> suddenly > >>>>>>>> find the offsets already committed. As far as I can tell, this > >>>> change > >>>>>> was > >>>>>>>> done mostly for cosmetic reasons. Without a compelling reason, > >> I > >>>>> think > >>>>>> we > >>>>>>>> should err on the side of maintaining compatibility. At a > >>> minimum, > >>>> if > >>>>>> we > >>>>>>>> really want to break compatibility, we should wait for the next > >>>> major > >>>>>>>> release. > >>>>>>>> > >>>>>>>> Note that I have submitted a patch to revert this change here: > >>>>>>>> https://github.com/apache/kafka/pull/4611. > >>>>>>>> > >>>>>>>> Thoughts? > >>>>>>>> > >>>>>>>> Thanks, > >>>>>>>> Jason > >>>>>>>> > >>>>>>>> > >>>>>>>> > >>>>>>>> On Tue, Nov 14, 2017 at 3:26 AM, Jorge Esteban Quilcate Otoya < > >>>>>>>> quilcate.jo...@gmail.com> wrote: > >>>>>>>> > >>>>>>>>> Thanks to everyone for your feedback. > >>>>>>>>> > >>>>>>>>> KIP has been accepted and discussion is moved to PR. > >>>>>>>>> > >>>>>>>>> Cheers, > >>>>>>>>> Jorge. > >>>>>>>>> > >>>>>>>>> El lun., 6 nov. 2017 a las 17:31, Rajini Sivaram (< > >>>>>>>> rajinisiva...@gmail.com > >>>>>>>>>> ) > >>>>>>>>> escribió: > >>>>>>>>> > >>>>>>>>>> +1 (binding) > >>>>>>>>>> Thanks for the KIP, Jorge. > >>>>>>>>>> > >>>>>>>>>> Regards, > >>>>>>>>>> > >>>>>>>>>> Rajini > >>>>>>>>>> > >>>>>>>>>> On Tue, Oct 31, 2017 at 9:58 AM, Damian Guy < > >>>>> damian....@gmail.com> > >>>>>>>>> wrote: > >>>>>>>>>> > >>>>>>>>>>> Thanks for the KIP - +1 (binding) > >>>>>>>>>>> > >>>>>>>>>>> On Mon, 23 Oct 2017 at 18:39 Guozhang Wang < > >>>> wangg...@gmail.com > >>>>>> > >>>>>>>> wrote: > >>>>>>>>>>> > >>>>>>>>>>>> Thanks Jorge for driving this KIP! +1 (binding). > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> Guozhang > >>>>>>>>>>>> > >>>>>>>>>>>> On Mon, Oct 16, 2017 at 2:11 PM, Bill Bejeck < > >>>>>> bbej...@gmail.com> > >>>>>>>>>> wrote: > >>>>>>>>>>>> > >>>>>>>>>>>>> +1 > >>>>>>>>>>>>> > >>>>>>>>>>>>> Thanks, > >>>>>>>>>>>>> Bill > >>>>>>>>>>>>> > >>>>>>>>>>>>> On Fri, Oct 13, 2017 at 6:36 PM, Ted Yu < > >>>>> yuzhih...@gmail.com > >>>>>>> > >>>>>>>>> wrote: > >>>>>>>>>>>>> > >>>>>>>>>>>>>> +1 > >>>>>>>>>>>>>> > >>>>>>>>>>>>>> On Fri, Oct 13, 2017 at 3:32 PM, Matthias J. Sax < > >>>>>>>>>>>> matth...@confluent.io> > >>>>>>>>>>>>>> wrote: > >>>>>>>>>>>>>> > >>>>>>>>>>>>>>> +1 > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> On 9/11/17 3:04 PM, Jorge Esteban Quilcate Otoya > >>>> wrote: > >>>>>>>>>>>>>>>> Hi All, > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> It seems that there is no further concern with > >>> the > >>>>>> KIP-171. > >>>>>>>>>>>>>>>> At this point we would like to start the voting > >>>>>> process. > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> The KIP can be found here: > >>>>>>>>>>>>>>>> https://cwiki.apache.org/ > >>>>> confluence/display/KAFKA/KIP- > >>>>>>>>>>>>>>> > >>> 171+-+Extend+Consumer+Group+Reset+Offset+for+Stream+ > >>>>>>>>> Application > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>>> Thanks! > >>>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>>> > >>>>>>>>>>>>>> > >>>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> > >>>>>>>>>>>> -- > >>>>>>>>>>>> -- Guozhang > >>>>>>>>>>>> > >>>>>>>>>>> > >>>>>>>>>> > >>>>>>>>> > >>>>>>>> > >>>>>> > >>>>> > >>>>> > >>>>> > >>>>> -- > >>>>> -- Guozhang > >>>>> > >>>> > >>> > >>> > >>> > >>> -- > >>> -- Guozhang > >>> > >> > > > > > > > > Email had 1 attachment: > + signature.asc > 1k (application/pgp-signature)
