-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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-35143][pipeline-connector][mysql] Expose newly added tables capture in mysql pipeline connector. #3411
[FLINK-35143][pipeline-connector][mysql] Expose newly added tables capture in mysql pipeline connector. #3411
Conversation
… in mysql pipeline connector.
...st/java/org/apache/flink/cdc/connectors/mysql/source/MysqlPipelineNewlyAddedTableITCase.java
Show resolved
Hide resolved
@qg-lin Thanks for your PR. If you need helps, please be free to notice me. Thanks. |
Thanks for your review, I've resolved it. |
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.
Thanks @qg-lin 👍
Minor wording suggestion from my side, otherwise looks good!
.booleanType() | ||
.defaultValue(false) | ||
.withDescription( | ||
"Whether capture the scan the newly added tables or not, by default is false. This option is only useful when we start the job from a savepoint/checkpoint."); |
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.
Could we drop the capture
here?
For example:
Whether to scan the newly added ...
With both capture
and scan
it seems hard to understand what the flag enables.
...mysql/src/main/java/org/apache/flink/cdc/connectors/mysql/source/MySqlDataSourceOptions.java
Outdated
Show resolved
Hide resolved
…line-connector-mysql/src/main/java/org/apache/flink/cdc/connectors/mysql/source/MySqlDataSourceOptions.java Co-authored-by: Muhammet Orazov <[email protected]>
Got it 👍 I missed that there is doc for the flag, the doc version was already understandable. Looks good! |
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
@qg-lin Please rebase the master branch and let's pass the CI. |
done |
@qg-lin It seems that some tests failed. Please take a look at them. Thanks. |
@ruanhang1993 Done, please trigger again. |
…pture in mysql pipeline connector. (apache#3411) Co-authored-by: Muhammet Orazov <[email protected]> Co-authored-by: north.lin <[email protected]>
https://issues.apache.org/jira/browse/FLINK-35143