-
Notifications
You must be signed in to change notification settings - Fork 13.8k
[FLINK-38530] create Dynamic Kafka Source for pyflink #27409
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
Conversation
|
test failures are unrelated |
dianfu
left a comment
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.
@bowenli86 Thanks a lot for the work! 👍 Only a few minor comments.
| super().__init__(j_service) | ||
|
|
||
|
|
||
| class KafkaStreamSubscriber(object): |
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.
What about also introducing class StreamPatternSubscriber and KafkaStreamSetSubscriber ?
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.
added
| from pyflink.datastream.connectors.kafka import KafkaOffsetsInitializer | ||
| from pyflink.java_gateway import get_gateway | ||
|
|
||
| __all__ = [ |
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.
It would be great to also update the documentation here: https://nightlies.apache.org/flink/flink-docs-master/docs/connectors/datastream/dynamic-kafka/
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.
found out dynamic kafka doc has been moved to kafka connector repo https://github.com/apache/flink-connector-kafka/blob/main/docs/content/docs/connectors/datastream/dynamic-kafka.md
created a PR there apache/flink-connector-kafka#211
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.
(btw, the pyflink kafka connector migration to connector repo which has been left half way done for 2 years is better to be fixed, otherwise we'll be constantly facing the inconsistency among the 2 repos and jump back and forth for all kinds of issues. Do you have time or anyone in OSS or Alibaba can take on it?)
What is the purpose of the change
create Dynamic Kafka Source for pyflink
Brief change log
4.0.1-2.0so it picks up dynamic source java classes, which existing3.0.0connector does not haveVerifying this change
This change added tests and can be verified as follows:
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (no)Documentation