You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@seatunnel.apache.org by GitBox <gi...@apache.org> on 2022/09/12 08:24:18 UTC
[GitHub] [incubator-seatunnel] liugddx opened a new pull request, #2719: [Improve][e2e] add driver-jar to lib
liugddx opened a new pull request, #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719
<!--
Thank you for contributing to SeaTunnel! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
## Contribution Checklist
- Make sure that the pull request corresponds to a [GITHUB issue](https://github.com/apache/incubator-seatunnel/issues).
- Name the pull request in the form "[Feature] [component] Title of the pull request", where *Feature* can be replaced by `Hotfix`, `Bug`, etc.
- Minor fixes should be named following this pattern: `[hotfix] [docs] Fix typo in README.md doc`.
-->
## Purpose of this pull request
<!-- Describe the purpose of this pull request. For example: This pull request adds checkstyle plugin.-->
add third-party or dirver jar to engine env.
## Check list
* [ ] Code changed are covered with tests, or it does not need tests for reason:
* [ ] If any new Jar binary package adding in your PR, please add License Notice according
[New License Guide](https://github.com/apache/incubator-seatunnel/blob/dev/docs/en/contribution/new-license.md)
* [ ] If necessary, please update the documentation to describe the new feature. https://github.com/apache/incubator-seatunnel/tree/dev/docs
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
liugddx commented on PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#issuecomment-1246492246
pls review ,if there is no problem i will modify all test case and remove jdbc driver. @TyrantLucifer @CalvinKirs @laglangyue
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
liugddx commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r971492382
##########
seatunnel-plugin-discovery/src/main/java/org/apache/seatunnel/plugin/discovery/AbstractPluginDiscovery.java:
##########
@@ -107,15 +107,25 @@ public T createPluginInstance(PluginIdentifier pluginIdentifier) {
try {
// use current thread classloader to avoid different classloader load same class error.
this.addURLToClassLoader.accept(classLoader, pluginJarPath.get());
+ ClassLoader finalClassLoader = classLoader;
Review Comment:
ACK
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer merged pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
TyrantLucifer merged PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
liugddx commented on PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#issuecomment-1247465803
CI is timeout .
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
liugddx commented on PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#issuecomment-1249993262
@TyrantLucifer PTAL. thanks
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
laglangyue commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r968477728
##########
seatunnel-e2e/seatunnel-flink-connector-v2-e2e/connector-jdbc-flink-e2e/src/test/java/org/apache/seatunnel/e2e/flink/v2/jdbc/JdbcSourceToConsoleIT.java:
##########
@@ -99,4 +101,10 @@ public void closePostgreSqlContainer() {
psl.stop();
}
}
+
+ @Override
+ protected void executeExtraCommands(GenericContainer<?> container) throws IOException, InterruptedException {
Review Comment:
All of the jars in ${seatunnel_home}/plugins/${any string}/lib/*.jar will be added to the lib for spark/flink
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
liugddx commented on PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#issuecomment-1247466547
> CI is timeout . 
pls help me to rerun CI. @CalvinKirs .and pls review again @TyrantLucifer ,thx.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
TyrantLucifer commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r971452701
##########
seatunnel-plugin-discovery/src/main/java/org/apache/seatunnel/plugin/discovery/AbstractPluginDiscovery.java:
##########
@@ -107,15 +107,25 @@ public T createPluginInstance(PluginIdentifier pluginIdentifier) {
try {
// use current thread classloader to avoid different classloader load same class error.
this.addURLToClassLoader.accept(classLoader, pluginJarPath.get());
+ ClassLoader finalClassLoader = classLoader;
Review Comment:
This class only load jars about plugin, load extra jars is this pr #2639 had been done. Please update your codes and execute `mvn clean package` at your local mechine.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
liugddx commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r970580061
##########
seatunnel-e2e/seatunnel-e2e-common/src/test/java/org/apache/seatunnel/e2e/common/AbstractContainer.java:
##########
@@ -64,6 +64,12 @@ public AbstractContainer() {
protected abstract List<String> getExtraStartShellCommands();
+ protected abstract String getThirdPartyPluginsPath();
Review Comment:
i will remove this method
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
TyrantLucifer commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r969116315
##########
seatunnel-e2e/seatunnel-flink-connector-v2-e2e/connector-jdbc-flink-e2e/src/test/java/org/apache/seatunnel/e2e/flink/v2/jdbc/JdbcSourceToConsoleIT.java:
##########
@@ -99,4 +101,10 @@ public void closePostgreSqlContainer() {
psl.stop();
}
}
+
+ @Override
+ protected void executeExtraCommands(GenericContainer<?> container) throws IOException, InterruptedException {
Review Comment:
>
In my opinion, not only will the container download additional jar packages, but it may also execute other commands, such as creating files, etc....
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
TyrantLucifer commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r970565384
##########
seatunnel-e2e/seatunnel-e2e-common/src/test/java/org/apache/seatunnel/e2e/common/AbstractContainer.java:
##########
@@ -64,6 +64,12 @@ public AbstractContainer() {
protected abstract List<String> getExtraStartShellCommands();
+ protected abstract String getThirdPartyPluginsPath();
Review Comment:
What's the meaning of this? In e2e test cases, extra jars not update to github server.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
liugddx commented on PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#issuecomment-1247865718
pls review again ,thx. @TyrantLucifer @Hisoka-X .
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
liugddx commented on PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#issuecomment-1246498714
#2642
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
TyrantLucifer commented on PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#issuecomment-1247470709
> > CI is timeout . 
>
> pls help me to rerun CI. @CalvinKirs .and pls review again @TyrantLucifer ,thx.
CI has restarted. Let's waiting it.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
TyrantLucifer commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r971463162
##########
seatunnel-plugin-discovery/src/main/java/org/apache/seatunnel/plugin/discovery/AbstractPluginDiscovery.java:
##########
@@ -107,15 +107,25 @@ public T createPluginInstance(PluginIdentifier pluginIdentifier) {
try {
// use current thread classloader to avoid different classloader load same class error.
this.addURLToClassLoader.accept(classLoader, pluginJarPath.get());
+ ClassLoader finalClassLoader = classLoader;
Review Comment:
It comes to changes to the core logic, I need to verify this PR so that it is currently pending.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
laglangyue commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r968477728
##########
seatunnel-e2e/seatunnel-flink-connector-v2-e2e/connector-jdbc-flink-e2e/src/test/java/org/apache/seatunnel/e2e/flink/v2/jdbc/JdbcSourceToConsoleIT.java:
##########
@@ -99,4 +101,10 @@ public void closePostgreSqlContainer() {
psl.stop();
}
}
+
+ @Override
+ protected void executeExtraCommands(GenericContainer<?> container) throws IOException, InterruptedException {
Review Comment:
All of the jars in ${seatunnel_home}/plugins/${any string}/*.jar will be added to the lib for spark/flink
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
laglangyue commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r968477728
##########
seatunnel-e2e/seatunnel-flink-connector-v2-e2e/connector-jdbc-flink-e2e/src/test/java/org/apache/seatunnel/e2e/flink/v2/jdbc/JdbcSourceToConsoleIT.java:
##########
@@ -99,4 +101,10 @@ public void closePostgreSqlContainer() {
psl.stop();
}
}
+
+ @Override
+ protected void executeExtraCommands(GenericContainer<?> container) throws IOException, InterruptedException {
Review Comment:
All of the jars in ${seatunnel_home}/plugins/*/*.jar will be added to the lib for spark/flink
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
laglangyue commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r968477728
##########
seatunnel-e2e/seatunnel-flink-connector-v2-e2e/connector-jdbc-flink-e2e/src/test/java/org/apache/seatunnel/e2e/flink/v2/jdbc/JdbcSourceToConsoleIT.java:
##########
@@ -99,4 +101,10 @@ public void closePostgreSqlContainer() {
psl.stop();
}
}
+
+ @Override
+ protected void executeExtraCommands(GenericContainer<?> container) throws IOException, InterruptedException {
Review Comment:
All of the jars in plugins/*/*.jar will be added to the lib for spark/flink
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
liugddx commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r969508798
##########
seatunnel-e2e/seatunnel-flink-connector-v2-e2e/connector-jdbc-flink-e2e/src/test/java/org/apache/seatunnel/e2e/flink/v2/jdbc/JdbcSourceToConsoleIT.java:
##########
@@ -99,4 +101,10 @@ public void closePostgreSqlContainer() {
psl.stop();
}
}
+
+ @Override
+ protected void executeExtraCommands(GenericContainer<?> container) throws IOException, InterruptedException {
Review Comment:
@TyrantLucifer pls review again
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
TyrantLucifer commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r971457396
##########
seatunnel-plugin-discovery/src/main/java/org/apache/seatunnel/plugin/discovery/AbstractPluginDiscovery.java:
##########
@@ -107,15 +107,25 @@ public T createPluginInstance(PluginIdentifier pluginIdentifier) {
try {
// use current thread classloader to avoid different classloader load same class error.
this.addURLToClassLoader.accept(classLoader, pluginJarPath.get());
+ ClassLoader finalClassLoader = classLoader;
Review Comment:
@CalvinKirs @Hisoka-X @EricJoy2048 @hailin0 PTAL.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] Hisoka-X commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
Hisoka-X commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r971485223
##########
seatunnel-plugin-discovery/src/main/java/org/apache/seatunnel/plugin/discovery/AbstractPluginDiscovery.java:
##########
@@ -107,15 +107,25 @@ public T createPluginInstance(PluginIdentifier pluginIdentifier) {
try {
// use current thread classloader to avoid different classloader load same class error.
this.addURLToClassLoader.accept(classLoader, pluginJarPath.get());
+ ClassLoader finalClassLoader = classLoader;
Review Comment:
Do it in `registerPlugin` method maybe better. At now. `registerPlugin` only register to flink environment, but not load it into client classpath. You can call `addURLToClassLoader ` logic in there. And call `registerPlugin` to load third party jar first, then call Source/SinkExecuteProcessor to load connector jar.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] Hisoka-X commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
Hisoka-X commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r971555663
##########
seatunnel-common/src/main/java/org/apache/seatunnel/common/config/Common.java:
##########
@@ -47,6 +52,21 @@ private Common() {
private static boolean STARTER = false;
+ /**
Review Comment:
Please don't add it in here, this `ADD_URL_TO_CLASSLOADER` only work for flink, spark use default `ADD_URL_TO_CLASSLOADER` in `AbstractPluginDiscovery`. So this method not 'common'
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
liugddx commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r971454579
##########
seatunnel-plugin-discovery/src/main/java/org/apache/seatunnel/plugin/discovery/AbstractPluginDiscovery.java:
##########
@@ -107,15 +107,25 @@ public T createPluginInstance(PluginIdentifier pluginIdentifier) {
try {
// use current thread classloader to avoid different classloader load same class error.
this.addURLToClassLoader.accept(classLoader, pluginJarPath.get());
+ ClassLoader finalClassLoader = classLoader;
Review Comment:
> This class only load jars about plugin, load extra jars is this pr #2639 had been done. Please update your codes and execute `mvn clean package` at your local mechine.
I know that.and i just use this method.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] Hisoka-X commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
Hisoka-X commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r971483016
##########
seatunnel-plugin-discovery/src/main/java/org/apache/seatunnel/plugin/discovery/AbstractPluginDiscovery.java:
##########
@@ -107,15 +107,25 @@ public T createPluginInstance(PluginIdentifier pluginIdentifier) {
try {
// use current thread classloader to avoid different classloader load same class error.
this.addURLToClassLoader.accept(classLoader, pluginJarPath.get());
+ ClassLoader finalClassLoader = classLoader;
Review Comment:
You can use `addURLToClassLoader.accept` to load third party jar to classloader, but not here. `AbstractPluginDiscovery` used for load connector jar. But it use logic like `addURLToClassLoader` to load connector jar into current classloader. If you also want use logic like `addURLToClassLoader`, I think you should create new class like `ThirdPartyJarLoadService`, use same parameter to receive `addURLToClassLoader`.
PS: `addURLToClassLoader` logic shouldn't repeat, please extract public code.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
TyrantLucifer commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r971490959
##########
seatunnel-plugin-discovery/src/main/java/org/apache/seatunnel/plugin/discovery/AbstractPluginDiscovery.java:
##########
@@ -107,15 +107,25 @@ public T createPluginInstance(PluginIdentifier pluginIdentifier) {
try {
// use current thread classloader to avoid different classloader load same class error.
this.addURLToClassLoader.accept(classLoader, pluginJarPath.get());
+ ClassLoader finalClassLoader = classLoader;
Review Comment:
> Do it in `registerPlugin` method maybe better. At now. `registerPlugin` only register to flink environment, but not load it into client classpath. You can call `addURLToClassLoader ` logic in there. And call `registerPlugin` to load third party jar first, then call Source/SinkExecuteProcessor to load connector jar.
Aggre with you.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
liugddx commented on PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#issuecomment-1249943477
@ashulin @CalvinKirs PTAL.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
laglangyue commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r968475293
##########
seatunnel-e2e/seatunnel-flink-connector-v2-e2e/connector-jdbc-flink-e2e/src/test/java/org/apache/seatunnel/e2e/flink/v2/jdbc/JdbcSourceToConsoleIT.java:
##########
@@ -99,4 +101,10 @@ public void closePostgreSqlContainer() {
psl.stop();
}
}
+
+ @Override
+ protected void executeExtraCommands(GenericContainer<?> container) throws IOException, InterruptedException {
Review Comment:
For subclasses, you don't need to consider if you write this command, as long as you provide http-link,
and it's may be` static List<String> ` in subclasses.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] laglangyue commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
laglangyue commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r968475293
##########
seatunnel-e2e/seatunnel-flink-connector-v2-e2e/connector-jdbc-flink-e2e/src/test/java/org/apache/seatunnel/e2e/flink/v2/jdbc/JdbcSourceToConsoleIT.java:
##########
@@ -99,4 +101,10 @@ public void closePostgreSqlContainer() {
psl.stop();
}
}
+
+ @Override
+ protected void executeExtraCommands(GenericContainer<?> container) throws IOException, InterruptedException {
Review Comment:
For subclasses, it not need to consider how to write this command, just need to provide http-link,
and it's may be` static List<String> ` in subclasses.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
TyrantLucifer commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r971453418
##########
seatunnel-plugin-discovery/src/main/java/org/apache/seatunnel/plugin/discovery/AbstractPluginDiscovery.java:
##########
@@ -107,15 +107,25 @@ public T createPluginInstance(PluginIdentifier pluginIdentifier) {
try {
// use current thread classloader to avoid different classloader load same class error.
this.addURLToClassLoader.accept(classLoader, pluginJarPath.get());
+ ClassLoader finalClassLoader = classLoader;
Review Comment:
BTW, do you change any codes in flink module? I can not see you changed any codes in it.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
liugddx commented on PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#issuecomment-1247479402
i also fix some error.
- when i run e2e cases in another docker environment not in my computer, that way will report an error,so i replace the host and use `GenericContainer.getHost`
- the spark container use [Rootless mode)](https://docs.docker.com/engine/security/rootless/).so i must use `root` and exe `mkdir` etc.
- when i run e2e cases, it will error,because the driver jar not in classloader,so i must add it .`addURLToClassLoader.accept`
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] liugddx commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
liugddx commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r971456530
##########
seatunnel-plugin-discovery/src/main/java/org/apache/seatunnel/plugin/discovery/AbstractPluginDiscovery.java:
##########
@@ -107,15 +107,25 @@ public T createPluginInstance(PluginIdentifier pluginIdentifier) {
try {
// use current thread classloader to avoid different classloader load same class error.
this.addURLToClassLoader.accept(classLoader, pluginJarPath.get());
+ ClassLoader finalClassLoader = classLoader;
Review Comment:
step1 will use dirver class ,and i Transfer the position step1 to step2.it will report error again.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] TyrantLucifer commented on a diff in pull request #2719: [Improve][e2e] add driver-jar to lib
Posted by GitBox <gi...@apache.org>.
TyrantLucifer commented on code in PR #2719:
URL: https://github.com/apache/incubator-seatunnel/pull/2719#discussion_r971490850
##########
seatunnel-plugin-discovery/src/main/java/org/apache/seatunnel/plugin/discovery/AbstractPluginDiscovery.java:
##########
@@ -107,15 +107,25 @@ public T createPluginInstance(PluginIdentifier pluginIdentifier) {
try {
// use current thread classloader to avoid different classloader load same class error.
this.addURLToClassLoader.accept(classLoader, pluginJarPath.get());
+ ClassLoader finalClassLoader = classLoader;
Review Comment:
>
Aggre with you.
--
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.
To unsubscribe, e-mail: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org