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

[Bugfix][CDC Base] Fix NPE caused by adding a table for restore job #6145

Merged
merged 1 commit into from
Jan 9, 2024

Conversation

ic4y
Copy link
Contributor

@ic4y ic4y commented Jan 5, 2024

Purpose of this pull request

Fix NPE caused by adding a table for restore job
image

image

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

@ic4y ic4y requested a review from hailin0 January 5, 2024 13:29
@hailin0
Copy link
Member

hailin0 commented Jan 8, 2024

LGTM

Writing e2e is difficult, what do you think?
cc @Hisoka-X

@ic4y Can you write unit tests?

@Hisoka-X
Copy link
Member

Hisoka-X commented Jan 8, 2024

LGTM

Writing e2e is difficult, what do you think? cc @Hisoka-X

@ic4y Can you write unit tests?

I think this is caused by the lack of our CDC-related test cases. Without basic use cases, it is difficult to add use cases for specific scenarios. We need to gradually improve CDC-related test cases. E2E testing is difficult to achieve, but through fine control of the code flow, it should be possible to write use cases for special scenarios.

We can do it later.

@EricJoy2048
Copy link
Member

LGTM
Writing e2e is difficult, what do you think? cc @Hisoka-X
@ic4y Can you write unit tests?

I think this is caused by the lack of our CDC-related test cases. Without basic use cases, it is difficult to add use cases for specific scenarios. We need to gradually improve CDC-related test cases. E2E testing is difficult to achieve, but through fine control of the code flow, it should be possible to write use cases for special scenarios.

We can do it later.

Yes, The test cases for CDC are difficult, but we still need to find a way to add e2e, especially as our CDC supports more and more connectors. cc @Carl-Zhou-CN

@EricJoy2048 EricJoy2048 added this to the 2.3.4 milestone Jan 8, 2024
@hailin0
Copy link
Member

hailin0 commented Jan 9, 2024

LGTM
Writing e2e is difficult, what do you think? cc @Hisoka-X
@ic4y Can you write unit tests?

I think this is caused by the lack of our CDC-related test cases. Without basic use cases, it is difficult to add use cases for specific scenarios. We need to gradually improve CDC-related test cases. E2E testing is difficult to achieve, but through fine control of the code flow, it should be possible to write use cases for special scenarios.

We can do it later.

Yes, need to add basic restore/add/drop table e2e cases

@hailin0 hailin0 merged commit 8d3f8e4 into apache:dev Jan 9, 2024
4 checks passed
chaorongzhi pushed a commit to chaorongzhi/seatunnel that referenced this pull request Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants