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