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

[Feature][Tool] Add connector check script for issue 6199 #6635

Merged
merged 5 commits into from
Apr 9, 2024

Conversation

dailai
Copy link
Contributor

@dailai dailai commented Apr 3, 2024

Purpose of this pull request

[Feature][Tool] Add connector check script #6199

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

@hailin0
Copy link
Contributor

hailin0 commented Apr 7, 2024

cc @Hisoka-X

@Hisoka-X Hisoka-X linked an issue Apr 7, 2024 that may be closed by this pull request
3 tasks
@hailin0
Copy link
Contributor

hailin0 commented Apr 7, 2024

good pr

@EricJoy2048
Copy link
Member

good pr

Copy link
Member

@Hisoka-X Hisoka-X left a comment

Choose a reason for hiding this comment

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

How about add some e2e test case? To check the value of System.out.println.

@Hisoka-X Hisoka-X added the feature New feature label Apr 7, 2024
@dailai dailai force-pushed the support-check-script-of-connector branch from 19cf30c to 63be624 Compare April 8, 2024 09:43
@dailai dailai requested a review from Hisoka-X April 8, 2024 09:44
@dailai dailai requested a review from Hisoka-X April 9, 2024 04:09
@dailai dailai force-pushed the support-check-script-of-connector branch from 8f4c783 to 89826f7 Compare April 9, 2024 04:15
@Hisoka-X
Copy link
Member

Hisoka-X commented Apr 9, 2024

image
Tested in local. Looking great!

@Hisoka-X
Copy link
Member

Hisoka-X commented Apr 9, 2024

@dailai could you retrigger the ci?

@dailai
Copy link
Contributor Author

dailai commented Apr 9, 2024

@dailai could you retrigger the ci?

Please wait a minute.

@dailai dailai force-pushed the support-check-script-of-connector branch from b7b8ab5 to b284da0 Compare April 9, 2024 04:33
@dailai dailai requested a review from Hisoka-X April 9, 2024 04:34
Copy link
Member

@Hisoka-X Hisoka-X left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +56 to +57
type = {EngineType.SPARK, EngineType.FLINK},
disabledReason = "Only support for seatunnel")
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is it tied to the execution engine? There seems to be no strong correlation

Copy link
Contributor Author

@dailai dailai Apr 9, 2024

Choose a reason for hiding this comment

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

Why is it tied to the execution engine? There seems to be no strong correlation

It really does not matter, but since it is to check the connector and transform of seatunnel, we only need to use the container of seatunnel to run the e2e test.

Copy link
Contributor

Choose a reason for hiding this comment

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

👌

Copy link
Contributor

@hailin0 hailin0 left a comment

Choose a reason for hiding this comment

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

LGTM

@hailin0 hailin0 merged commit 65aedf6 into apache:dev Apr 9, 2024
10 checks passed
@dailai dailai deleted the support-check-script-of-connector branch April 10, 2024 06:56
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.

[Feature][Tool] Add connector check script
4 participants