-
Notifications
You must be signed in to change notification settings - Fork 46
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
[FLINK-32938] Replace pulsar admin calls #59
Conversation
Thanks for opening this pull request! Please check out our contributing guidelines. (https://flink.apache.org/contributing/how-to-contribute.html) |
Can someone help rerun the CI? On my local laptop, everything runs fine:
|
@nlu90 retriggered and passed. I'm reviewing this patch now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you!
We need to update the document a bit. And since we bump the minima Pulsar version, I'm afraid that we already need a new major version bump. And thus it's possible we remove the deprecated method.
@@ -126,9 +123,11 @@ public class PulsarSinkBuilder<IN> { | |||
* | |||
* @param adminUrl The url for the PulsarAdmin. | |||
* @return this PulsarSinkBuilder. | |||
* @deprecated this method will return builder directly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps add a link to this PR as background. But it's OK that we do it in a follow-up patch.
Merging... I'll prepare a follow-up patch for docs updates, which includes a response for FLINK-33111 |
Awesome work, congrats on your first merged pull request! |
Purpose of the change
Replace PulsarAdmin calls with PulsarClient or Consumer calls
Brief change log
ProducerRegister
.PulsarSinkContext
.MetadataListener
.Verifying this change
This change is already covered by existing tests, such as (please describe tests).
Significant changes
(Please check any boxes [x] if the answer is "yes". You can first publish the PR and check them afterwards, for
convenience.)
@Public(Evolving)
)