Skip to content

KAFKA-20385 [3/N]: Null-check for listener regn on AsyncKafkaConsumer - #23101

Open
adikou wants to merge 5 commits into
apache:trunkfrom
adikou:akousik/KAFKA-20385-fix
Open

KAFKA-20385 [3/N]: Null-check for listener regn on AsyncKafkaConsumer#23101
adikou wants to merge 5 commits into
apache:trunkfrom
adikou:akousik/KAFKA-20385-fix

Conversation

@adikou

@adikou adikou commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Added missing null check on listener registration for regex subscription. The bug would have unregistered any previously set listener in the consumer.

Reviewers: Chia-Ping Tsai chia7712@gmail.com, Lianet Magrans lmagrans@confluent.io

@adikou

adikou commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

cc @chia7712 @lianetm

@github-actions github-actions Bot added triage PRs from the community consumer clients small Small PRs labels Aug 6, 2026

@chia7712 chia7712 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@adikou It seems you missed the PR description :)

@adikou

adikou commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

LGTM

@adikou It seems you missed the PR description :)

I thought it was too small a change for that. Is it a required field? I can add some context if needed

@chia7712

chia7712 commented Aug 6, 2026

Copy link
Copy Markdown
Member

Is it a required field? I can add some context if needed

No patch should be for nothing :)

@adikou

adikou commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Is it a required field? I can add some context if needed

No patch should be for nothing :)

Fair enough.
Is there a way to trigger failed CI builds for a PR? Seems to be unrelated for this one

@chia7712

chia7712 commented Aug 6, 2026

Copy link
Copy Markdown
Member

@adikou you could rebase code to trigger CI :)

acquireAndEnsureOpen();
subscriptions.setRebalanceListener(listener, this);
if (listener != null)
subscriptions.setRebalanceListener(listener, this);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

shoudl we move this after the throwIfGroupIdNotDefined and pattern checks below? (would be weird to set the listener and then fail)

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.

sgtm. Fixed and pushed

@lianetm lianetm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks! LGTM, let's just wait for the build

@adikou

adikou commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Thanks! LGTM, let's just wait for the build

Thanks for the approval. Not sure why the build is failing. CI Action is not too clear

@adikou

adikou commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Thanks! LGTM, let's just wait for the build

Thanks for the approval. Not sure why the build is failing. CI Action is not too clear

Ah. https://x.com/githubstatus/status/2085464502028398833?s=46

@github-actions github-actions Bot removed the triage PRs from the community label Aug 7, 2026
@adikou

adikou commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

Last run was a success

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants