-
Notifications
You must be signed in to change notification settings - Fork 13.3k
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-28617][SQL Gateway] Support stop job statement in SqlGatewayService #21292
Conversation
The CI failed due to an unrelated Kafka connector test. We may take a look at the codes first. cc @fsk119 |
@flinkbot run azure |
d0d1f7d
to
e12294a
Compare
Rebased on the recent master to see if the test failure still exists. |
256b7ff
to
26da341
Compare
26da341
to
911b28d
Compare
ping @fsk119 |
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.
Sorry for the late response. I left some comments.
...ateway/src/main/java/org/apache/flink/table/gateway/service/operation/OperationExecutor.java
Outdated
Show resolved
Hide resolved
...ql-gateway/src/test/java/org/apache/flink/table/gateway/service/SqlGatewayServiceITCase.java
Outdated
Show resolved
Hide resolved
...ql-gateway/src/test/java/org/apache/flink/table/gateway/service/SqlGatewayServiceITCase.java
Outdated
Show resolved
Hide resolved
...ql-gateway/src/test/java/org/apache/flink/table/gateway/service/SqlGatewayServiceITCase.java
Outdated
Show resolved
Hide resolved
@flinkbot run azure |
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.
Thanks for your update.
...k-test-utils-parent/flink-test-utils/src/main/java/org/apache/flink/test/util/TestUtils.java
Outdated
Show resolved
Hide resolved
...ateway/src/main/java/org/apache/flink/table/gateway/service/operation/OperationExecutor.java
Outdated
Show resolved
Hide resolved
...ql-gateway/src/test/java/org/apache/flink/table/gateway/service/SqlGatewayServiceITCase.java
Outdated
Show resolved
Hide resolved
Updated as suggested. CI turns green. cc @fsk119 |
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
} catch (Exception e) { | ||
throw new FlinkException( | ||
"Could not stop job " | ||
+ stopJobOperation.getJobId() | ||
+ " in session " | ||
+ operationHandle.getIdentifier() | ||
+ ".", | ||
e); | ||
} |
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.
nit: It's a little trivial to catch the same exception twice with a similar msg. In the next PR, we can remove this.
What is the purpose of the change
Support stop job statement in SqlGatewayService. Subtask of FLIP-222.
Brief change log
Verifying this change
Please make sure both new and modified tests in this PR follows the conventions defined in our code quality guide: https://flink.apache.org/contributing/code-style-and-quality-common.html#testing
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
@Public(Evolving)
: noDocumentation