Re: [PR] [FLINK-35149][cdc-composer] Fix DataSinkTranslator#sinkTo ignoring pre-write topology if not TwoPhaseCommittingSink [flink-cdc]

2024-05-13 Thread via GitHub
loserwang1024 commented on PR #3233: URL: https://github.com/apache/flink-cdc/pull/3233#issuecomment-2106754276 @PatrickRen , CC, Would you like to help me review this PR? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and

Re: [PR] [FLINK-35149][cdc-composer] Fix DataSinkTranslator#sinkTo ignoring pre-write topology if not TwoPhaseCommittingSink [flink-cdc]

2024-05-08 Thread via GitHub
loserwang1024 commented on code in PR #3233: URL: https://github.com/apache/flink-cdc/pull/3233#discussion_r1594884935 ## flink-cdc-composer/src/main/java/org/apache/flink/cdc/composer/flink/translator/DataSinkTranslator.java: ## @@ -71,7 +72,9 @@ public void translate(

Re: [PR] [FLINK-35149][cdc-composer] Fix DataSinkTranslator#sinkTo ignoring pre-write topology if not TwoPhaseCommittingSink [flink-cdc]

2024-05-08 Thread via GitHub
loserwang1024 commented on code in PR #3233: URL: https://github.com/apache/flink-cdc/pull/3233#discussion_r1594878296 ## flink-cdc-composer/src/test/java/org/apache/flink/cdc/composer/flink/translator/DataSinkTranslatorTest.java: ## @@ -0,0 +1,85 @@ +/* + * Licensed to the

Re: [PR] [FLINK-35149][cdc-composer] Fix DataSinkTranslator#sinkTo ignoring pre-write topology if not TwoPhaseCommittingSink [flink-cdc]

2024-05-08 Thread via GitHub
pvary commented on code in PR #3233: URL: https://github.com/apache/flink-cdc/pull/3233#discussion_r1593954989 ## flink-cdc-composer/src/test/java/org/apache/flink/cdc/composer/flink/translator/DataSinkTranslatorTest.java: ## @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache

Re: [PR] [FLINK-35149][cdc-composer] Fix DataSinkTranslator#sinkTo ignoring pre-write topology if not TwoPhaseCommittingSink [flink-cdc]

2024-05-08 Thread via GitHub
pvary commented on code in PR #3233: URL: https://github.com/apache/flink-cdc/pull/3233#discussion_r1593952991 ## flink-cdc-composer/src/test/java/org/apache/flink/cdc/composer/flink/translator/DataSinkTranslatorTest.java: ## @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache

Re: [PR] [FLINK-35149][cdc-composer] Fix DataSinkTranslator#sinkTo ignoring pre-write topology if not TwoPhaseCommittingSink [flink-cdc]

2024-05-08 Thread via GitHub
pvary commented on code in PR #3233: URL: https://github.com/apache/flink-cdc/pull/3233#discussion_r1593951427 ## flink-cdc-composer/src/main/java/org/apache/flink/cdc/composer/flink/translator/DataSinkTranslator.java: ## @@ -71,7 +72,9 @@ public void translate( }

Re: [PR] [FLINK-35149][cdc-composer] Fix DataSinkTranslator#sinkTo ignoring pre-write topology if not TwoPhaseCommittingSink [flink-cdc]

2024-05-08 Thread via GitHub
loserwang1024 commented on PR #3233: URL: https://github.com/apache/flink-cdc/pull/3233#issuecomment-2099986993 > Could you define your own one in the test itself? Then you have free hands what it does, and does not... Done it. -- This is an automated message from the Apache

Re: [PR] [FLINK-35149][cdc-composer] Fix DataSinkTranslator#sinkTo ignoring pre-write topology if not TwoPhaseCommittingSink [flink-cdc]

2024-05-02 Thread via GitHub
pvary commented on PR #3233: URL: https://github.com/apache/flink-cdc/pull/3233#issuecomment-2090855121 > > Could you please add a test case to prevent later code changes to revert this fix? > > I'd like to, but it seems no pipeline sink which is WithPreWriteTopology but not

Re: [PR] [FLINK-35149][cdc-composer] Fix DataSinkTranslator#sinkTo ignoring pre-write topology if not TwoPhaseCommittingSink [flink-cdc]

2024-05-02 Thread via GitHub
loserwang1024 commented on PR #3233: URL: https://github.com/apache/flink-cdc/pull/3233#issuecomment-2089709636 > Could you please add a test case to prevent later code changes to revert this fix? I'd like to, but it seems no pipeline sink which is WithPreWriteTopology but not

Re: [PR] [FLINK-35149][cdc-composer] Fix DataSinkTranslator#sinkTo ignoring pre-write topology if not TwoPhaseCommittingSink [flink-cdc]

2024-04-18 Thread via GitHub
pvary commented on PR #3233: URL: https://github.com/apache/flink-cdc/pull/3233#issuecomment-2065143602 Good catch @loserwang1024! Could you please add a test case to prevent later code changes to revert this fix? -- This is an automated message from the Apache Git Service. To

Re: [PR] [FLINK-35149][cdc-composer] Fix DataSinkTranslator#sinkTo ignoring pre-write topology if not TwoPhaseCommittingSink [flink-cdc]

2024-04-17 Thread via GitHub
loserwang1024 commented on PR #3233: URL: https://github.com/apache/flink-cdc/pull/3233#issuecomment-2062883633 @PatrickRen , CC -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific

[PR] [FLINK-35149][cdc-composer] Fix DataSinkTranslator#sinkTo ignoring pre-write topology if not TwoPhaseCommittingSink [flink-cdc]

2024-04-17 Thread via GitHub
loserwang1024 opened a new pull request, #3233: URL: https://github.com/apache/flink-cdc/pull/3233 Current , when sink is not instanceof TwoPhaseCommittingSink, use input.transform rather than stream. It means that pre-write topology will be ignored. ``` private void sinkTo(