-
Notifications
You must be signed in to change notification settings - Fork 13.8k
[FLINK-28765] Create a doc for protobuf format #20408
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
MartijnVisser
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.
@maosuhan Thanks a lot for the docs! Some minor fixes, we do need to add Protobuf to the sql_connectors_yml file to make sure it's downloadable/embedded properly.
Co-authored-by: MartijnVisser <martijn@2symbols.com>
Co-authored-by: MartijnVisser <martijn@2symbols.com>
Co-authored-by: MartijnVisser <martijn@2symbols.com>
Co-authored-by: MartijnVisser <martijn@2symbols.com>
Co-authored-by: MartijnVisser <martijn@2symbols.com>
Co-authored-by: MartijnVisser <martijn@2symbols.com>
Co-authored-by: MartijnVisser <martijn@2symbols.com>
Co-authored-by: MartijnVisser <martijn@2symbols.com>
f6af56b to
72ed994
Compare
MartijnVisser
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.
@maosuhan Thanks for the PR! I've added the needed data for displaying the Maven dependency and I've also copied the doc to the Chinese content directory. Anything to add from your end? Else I'll merge it.
|
Thanks @maosuhan for contributing this, and @MartijnVisser for reviewing. |
|
@libenchao I'm assuming we're talking about the |
|
@MartijnVisser @libenchao Thanks for your reviewing and I have updated the doc also I wrote more detailed information about how to use it. Anything I should change from your side? |
@MartijnVisser I mean the java classes compiled from the .desc file by protoc, we need users to do this step and provide the classes when submitting the flink job. |
Co-authored-by: Benchao Li <libenchao@gmail.com>
Co-authored-by: Benchao Li <libenchao@gmail.com>
|
@libenchao thanks for the suggestion. Changes have been committed. |
libenchao
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.
LGTM, merging
After this feature has been done https://issues.apache.org/jira/browse/FLINK-18202, we should write a doc to introduce how to use the protobuf format in SQL.