Skip to content

KAFKA-5520: KIP-171 - Extend Consumer Group Reset Offset for Stream Application#3831

Closed
jeqo wants to merge 13 commits intoapache:trunkfrom
jeqo:features/kip-171
Closed

KAFKA-5520: KIP-171 - Extend Consumer Group Reset Offset for Stream Application#3831
jeqo wants to merge 13 commits intoapache:trunkfrom
jeqo:features/kip-171

Conversation

@jeqo
Copy link
Copy Markdown
Contributor

@jeqo jeqo commented Sep 11, 2017

.describedAs("id")
.required();
bootstrapServerOption = optionParser.accepts("bootstrap-servers", "Comma-separated list of broker urls with format: HOST1:PORT1,HOST2:PORT2")
bootstrapServerOption = optionParser.accepts("bootstrap-server", "Comma-separated list of broker urls with format: HOST1:PORT1,HOST2:PORT2")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why removing the trailing 's' ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Forgot to mention it on the KIP, was thinking about aligning options. In Consumer Group command, and other, the option is bootstrap-server. Should I rollback this change?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it is not in KIP, better roll back.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeap, pushing changes.

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Sep 12, 2017

@jeqo Thanks for the PR. The KIP is not accepted yet, and there is no vote started. As a vote need to be open for 72h and KIPs must be accepted by tomorrow to be considered for 1.0 release, this will not make it anymore and we need to move it for 1.1 release.

\cc @guozhangwang Please correct me if I am wrong.

Thus, I would first try to get the KIP voted, and we can review this afterwards.

@guozhangwang
Copy link
Copy Markdown
Contributor

There is a VOTE thread started already by @jeqo . I think implementation in parallel while waiting for the KIP to be accepted is ok.

@jeqo
Copy link
Copy Markdown
Contributor Author

jeqo commented Oct 30, 2017

Closing this PR to merge with KIP-198: #4159

@jeqo jeqo closed this Oct 30, 2017
@guozhangwang
Copy link
Copy Markdown
Contributor

I'm not sure if we want to do these two KIPs in the same PR, but rather KIP-171 would depend on KIP-198 being done along which we will add the listTopics in the admin client. But that's just my understanding. @mjsax could you clarify?

@mjsax
Copy link
Copy Markdown
Member

mjsax commented Nov 1, 2017

I agree. We should have 2 PR per KIP/JIRA.

@jeqo
Copy link
Copy Markdown
Contributor Author

jeqo commented Nov 2, 2017

But KIP-198 is already merged https://cwiki.apache.org/confluence/display/KAFKA/KIP-198%3A+Remove+ZK+dependency+from+Streams+Reset+Tool :)
What I mean is that I have updated the implementation according to KIP-198, that overlaps with KIP-171. I will update the issue title to avoid confusion.

asfgit pushed a commit that referenced this pull request Dec 6, 2017
…plication

KIP: https://cwiki.apache.org/confluence/display/KAFKA/KIP-171+-+Extend+Consumer+Group+Reset+Offset+for+Stream+Application

Merge changes from KIP-198

Ref: #3831

Author: Jorge Quilcate Otoya <quilcate.jorge@gmail.com>
Author: Ismael Juma <ismael@juma.me.uk>
Author: Matthias J. Sax <matthias@confluent.io>
Author: Manikumar Reddy <manikumar.reddy@gmail.com>
Author: Guozhang Wang <wangguoz@gmail.com>
Author: Apurva Mehta <apurva@confluent.io>
Author: Rajini Sivaram <rajinisivaram@googlemail.com>
Author: Jason Gustafson <jason@confluent.io>
Author: Vahid Hashemian <vahidhashemian@us.ibm.com>
Author: Bill Bejeck <bill@confluent.io>
Author: Dong Lin <lindong28@gmail.com>
Author: Soenke Liebau <soenke.liebau@opencore.com>
Author: Colin P. Mccabe <cmccabe@confluent.io>
Author: Damian Guy <damian.guy@gmail.com>
Author: Xavier Léauté <xl+github@xvrl.net>
Author: Maytee Chinavanichkit <maytee.chinavanichkit@linecorp.com>
Author: Joel Hamill <git config --global user.email>
Author: Paolo Patierno <ppatierno@live.com>
Author: siva santhalingam <siva.santhalingam@gmail.com>
Author: Tommy Becker <tobecker@tivo.com>
Author: Mickael Maison <mickael.maison@gmail.com>
Author: Onur Karaman <okaraman@linkedin.com>
Author: tedyu <yuzhihong@gmail.com>
Author: Xin Li <Xin.Li@trivago.com>
Author: Magnus Edenhill <magnus@edenhill.se>
Author: Manjula K <manjula@kafka-summit.org>
Author: Hugo Louro <hmclouro@gmail.com>
Author: Jeff Widman <jeff@jeffwidman.com>
Author: bartdevylder <bartdevylder@gmail.com>
Author: Ewen Cheslack-Postava <me@ewencp.org>
Author: Jacek Laskowski <jacek@japila.pl>
Author: Tom Bentley <tbentley@redhat.com>
Author: Konstantine Karantasis <konstantine@confluent.io>

Reviewers: Matthias J. Sax <matthias@confluent.io>, Guozhang Wang <wangguoz@gmail.com>

Closes #4159 from jeqo/feature/kip-171
@jeqo jeqo deleted the features/kip-171 branch August 8, 2020 09:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants