Thanks for clarification Colin! I like the proposal.
-Matthias On 3/3/18 2:53 PM, Colin McCabe wrote: > 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)
signature.asc
Description: OpenPGP digital signature
