Skip to content
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][cdc-connector][mysql] Expose newly added tables capture in mysql pipeline connector. #3411

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

qg-lin
Copy link
Contributor

@qg-lin qg-lin commented Jun 12, 2024

@github-actions github-actions bot added docs Improvements or additions to documentation mysql-pipeline-connector labels Jun 12, 2024
import static org.assertj.core.api.Assertions.assertThat;

/** IT tests to cover various newly added tables during capture process in pipeline mode. */
public class MysqlPipelineNewlyAddedTableITCase extends MySqlSourceTestBase {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @qg-lin for the PR!

Could you please use Junit5 in the IT test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Another PR in progress for Junit5.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great, thanks!

@ruanhang1993
Copy link
Contributor

@qg-lin Thanks for your PR.
Please rebase the master branch and resolve the conflicts. Then let's run the CI.

If you need helps, please be free to notice me. Thanks.

@qg-lin
Copy link
Contributor Author

qg-lin commented Jun 20, 2024

@qg-lin Thanks for your PR. Please rebase the master branch and resolve the conflicts. Then let's run the CI.

If you need helps, please be free to notice me. Thanks.

Thanks for your review, I've resolved it.

Copy link
Contributor

@morazow morazow left a 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.");
Copy link
Contributor

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.

…line-connector-mysql/src/main/java/org/apache/flink/cdc/connectors/mysql/source/MySqlDataSourceOptions.java

Co-authored-by: Muhammet Orazov <[email protected]>
@morazow
Copy link
Contributor

morazow commented Jun 20, 2024

Got it 👍 I missed that there is doc for the flag, the doc version was already understandable.

Looks good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to documentation mysql-pipeline-connector
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants