Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KAFKA-18836 Make ConsumerGroupMetadata an interface #18977

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

pszymczyk
Copy link

@pszymczyk pszymczyk commented Feb 20, 2025

Implementation for KAFKA-18836 improvement.
KIP-1136

Copy link
Contributor

@frankvicky frankvicky left a comment

Choose a reason for hiding this comment

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

Hi @pszymczyk
Thanks for the patch
According to the ticket, this change should have a KIP first since this will introduce a change to the public API.

Please refer to the following link for further information:
https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals

@pszymczyk
Copy link
Author

Hi @frankvicky
Sure I will provide a KIP this week, just need to read all the manuals and follow procedures as it is my first contribution into Kafka project.

@mjsax mjsax added kip Requires or implements a KIP and removed triage PRs from the community labels Feb 20, 2025
* Note: Any change to this class is considered public and requires a KIP.
*/
public class ConsumerGroupMetadata {
Copy link
Member

Choose a reason for hiding this comment

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

We cannot do it this way. For Kafka 4.1, we need to keep the class as-is, to maintain backward compatibility. We should only deprecate both constructors, and update the JavaDocs saying, that this class will become an interface in the future, ie, Apache Kafka 5.0.

The KIP should lay out this two step approach, and if the KIP gets accepted, we should file a follow up ticket for AK 5.0 release, that would do the actual code change from class to interface.

Copy link
Author

Choose a reason for hiding this comment

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

Sure, makes sense, I will provide a KIP and update the PR.

Copy link
Author

Choose a reason for hiding this comment

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

@pszymczyk pszymczyk reopened this Feb 21, 2025
@github-actions github-actions bot added the triage PRs from the community label Feb 21, 2025
@pszymczyk pszymczyk marked this pull request as ready for review February 21, 2025 14:11
@github-actions github-actions bot removed the triage PRs from the community label Feb 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clients consumer kip Requires or implements a KIP producer small Small PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants