Skip to content

feat: support subscribeMode field on pulsar #3831

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

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

ericsyh
Copy link
Contributor

@ericsyh ericsyh commented May 22, 2025

Description

Please explain the changes you've made

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #3832

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

@ericsyh ericsyh requested review from a team as code owners May 22, 2025 15:37
@msfussell
Copy link
Member

msfussell commented May 24, 2025

@ericsyh - Is there an issue number for this PR?

@ericsyh
Copy link
Contributor Author

ericsyh commented May 24, 2025

@ericsyh - Is there an issue number for this PR?

@msfussell sorry i missed the issue create before, just created a new issue of #3832 and linked on this PR.

nelson-parente
nelson-parente previously approved these changes May 26, 2025
Copy link
Contributor

@nelson-parente nelson-parente left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@mikeee mikeee left a comment

Choose a reason for hiding this comment

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

Looking good to me, just a few nits

Co-authored-by: Mike Nguyen <[email protected]>
Signed-off-by: Eric Shen <[email protected]>
@ericsyh
Copy link
Contributor Author

ericsyh commented Jun 1, 2025

@nelson-parente @mikeee Could you help review again? cc @yaron2 @JoshVanL help review

mikeee
mikeee previously approved these changes Jun 11, 2025
Copy link
Member

@mikeee mikeee left a comment

Choose a reason for hiding this comment

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

Lgtm - running the tests now

@mikeee
Copy link
Member

mikeee commented Jun 11, 2025

Looks like a minor linter issue popped up if you could please address it :)

Signed-off-by: Eric Shen <[email protected]>
@ericsyh
Copy link
Contributor Author

ericsyh commented Jun 11, 2025

Looks like a minor linter issue popped up if you could please address it :)

@mikeee I just fixed the lint in the 14f449d, now it should be fine.

@ericsyh
Copy link
Contributor Author

ericsyh commented Jun 13, 2025

@nelson-parente @msfussell PTAL the updated PR thx.

@nelson-parente
Copy link
Contributor

@yaron2 I think this is good to merge

@yaron2 yaron2 added this pull request to the merge queue Jun 13, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 13, 2025
@ericsyh
Copy link
Contributor Author

ericsyh commented Jun 14, 2025

The lint and several comformance check fail in the merge-quege CI was not caused by my changes...

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.

[feat](pulsar): support the subscription mode in PubSub Pulsar fields.
6 participants