-
Notifications
You must be signed in to change notification settings - Fork 226
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
[improve]Transform the End-To-End(E2E) tasks on the assembly line #466
Conversation
ce59eb4
to
137af59
Compare
...s-connector/src/test/java/org/apache/doris/flink/autoci/container/DorisContainerService.java
Outdated
Show resolved
Hide resolved
...s-connector/src/test/java/org/apache/doris/flink/autoci/container/DorisContainerService.java
Outdated
Show resolved
Hide resolved
DROP TABLE IF EXISTS test_e2e_mysql.tbl1; | ||
|
||
CREATE TABLE test_e2e_mysql.tbl1 ( | ||
`name` varchar(256) primary key, | ||
`age` int | ||
); | ||
|
||
insert into test_e2e_mysql.tbl1 values ('doris_1',1); |
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.
The table structures are the same, only the table names are different. Do we need to extract them?
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.
This is the next plan. These cases will be transformed. Each table will have a different structure and will be more complex. Now these cases are too simple.
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.
For functional testing, can this table structure be reused? And for all types of tests, a single table can be written. I feel that there are too many identical table structures here, which doesn't seem to make much sense?
flink-doris-connector/src/test/java/org/apache/doris/flink/autoci/AbstractAutoCITestBase.java
Outdated
Show resolved
Hide resolved
...s-connector/src/test/java/org/apache/doris/flink/autoci/container/DorisContainerService.java
Outdated
Show resolved
Hide resolved
050b5ff
to
3558af3
Compare
31affcf
to
26613bf
Compare
e7193c4
to
2e76bc4
Compare
...-doris-connector/src/test/java/org/apache/doris/flink/container/instance/MySQLContainer.java
Outdated
Show resolved
Hide resolved
...k-doris-connector/src/test/java/org/apache/doris/flink/container/e2e/Mysql2DorisE2ECase.java
Outdated
Show resolved
Hide resolved
flink-doris-connector/src/test/java/org/apache/doris/flink/container/AbstractE2EService.java
Outdated
Show resolved
Hide resolved
public void cancelCurrentJob(String jobName) { | ||
String currentName = currentJobName.get(); | ||
Future<?> currentFuture = currentJob.get(); | ||
|
||
if (currentFuture != null && !currentFuture.isDone() && jobName.equals(currentName)) { | ||
LOG.info("Cancelling the current job. jobName={}", jobName); | ||
boolean cancelled = currentFuture.cancel(true); | ||
if (cancelled) { | ||
LOG.info("Job successfully cancelled. jobName={}", jobName); | ||
} else { | ||
LOG.info("Job cancellation failed. jobName={}", jobName); | ||
} | ||
} else { | ||
LOG.info("No matching job to cancel or job already completed. jobName={}", jobName); | ||
} | ||
} |
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.
Do we need to introduce a thread pool? Why not use Flink's built-in jobClient to cancel?
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.
Good idea.
8f834ea
to
e599f5e
Compare
e599f5e
to
71f37d7
Compare
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
Proposed changes
Issue Number: close #xxx
Problem Summary:
The overall code of our early End-To-End tasks was quite confusing. As a result, when we needed to add a simple case, we often needed to add the entire pipeline code. In this way, the readability and scalability of the code are reduced, so we made this transformation.
Checklist(Required)
Further comments
If this is a relatively large or complex change, kick off the discussion at [email protected] by explaining why you chose the solution you did and what alternatives you considered, etc...