wuchong commented on a change in pull request #16013:
URL: https://github.com/apache/flink/pull/16013#discussion_r643720204
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/context/SessionContext.java
##########
@@ -284,6 +284,20 @@ public void addJar(String jarPath) {
executionContext = new ExecutionContext(sessionConfiguration,
classLoader, sessionState);
}
+ public void removeJar(String jarPath) {
+ URL jarURL = getURLFromPath(jarPath);
+ if (!dependencies.contains(jarURL)) {
Review comment:
Would be better to print a warning or message that can't remove the
specified jar because the jar path is not found in session classloader.
##########
File path:
flink-table/flink-sql-client/src/test/java/org/apache/flink/table/client/gateway/context/SessionContextTest.java
##########
@@ -273,6 +298,12 @@ private void validateAddJar(String jarPath) throws
IOException {
getConfiguration().get(JARS));
}
+ private void validateRemoveJar(String jarPath) {
+ sessionContext.removeJar(jarPath);
Review comment:
We should assert the jarpath is in session context, and then remove it.
Otherwise, we don't know whether the remove action is successful or not.
##########
File path:
flink-table/flink-sql-client/src/main/java/org/apache/flink/table/client/gateway/context/SessionContext.java
##########
@@ -353,7 +367,7 @@ private URL getURLFromPath(String jarPath) {
Path path = new Path(jarPath);
String scheme = path.toUri().getScheme();
if (scheme != null && !scheme.equals("file")) {
- throw new SqlExecutionException("SQL Client only supports to add
local jars.");
+ throw new SqlExecutionException("SQL client only supports the
local jar path.");
Review comment:
Would be better to still keep action name.
--
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 comment.
For queries about this service, please contact Infrastructure at:
[email protected]
