You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/04/25 03:13:00 UTC
[GitHub] [incubator-kyuubi] iodone opened a new pull request, #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
iodone opened a new pull request, #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475
<!--
Thanks for sending a pull request!
Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->
### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
1. If you add a feature, you can talk about the use case of it.
2. If you fix a bug, you can clarify why it is a bug.
-->
In scala code mode implementation, we need to call `repl.addUrlsToClassPath` to set the classpath of the current thread to repl before entering `repl`
### _How was this patch tested?_
- [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [ ] [Run test](https://kyuubi.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1124712342
thanks, merging to master
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1113894497
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2475?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#2475](https://codecov.io/gh/apache/incubator-kyuubi/pull/2475?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6c31f2e) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/cdfae8d879194a355fdaa03dc33f23578d897402?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (cdfae8d) will **decrease** coverage by `0.03%`.
> The diff coverage is `76.92%`.
> :exclamation: Current head 6c31f2e differs from pull request most recent head 434f298. Consider uploading reports for the commit 434f298 to get more accurate results
```diff
@@ Coverage Diff @@
## master #2475 +/- ##
============================================
- Coverage 63.63% 63.59% -0.04%
Complexity 41 41
============================================
Files 373 373
Lines 17829 17849 +20
Branches 2378 2382 +4
============================================
+ Hits 11345 11351 +6
- Misses 5430 5440 +10
- Partials 1054 1058 +4
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2475?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...e/kyuubi/engine/spark/operation/ExecuteScala.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2475/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vRXhlY3V0ZVNjYWxhLnNjYWxh) | `81.81% <76.92%> (-2.06%)` | :arrow_down: |
| [...rg/apache/kyuubi/engine/trino/TrinoSqlEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2475/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3FsRW5naW5lLnNjYWxh) | `52.63% <0.00%> (-7.02%)` | :arrow_down: |
| [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2475/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uTWFuYWdlci5zY2FsYQ==) | `83.60% <0.00%> (-1.64%)` | :arrow_down: |
| [...in/java/org/apache/hive/beeline/KyuubiBeeLine.java](https://codecov.io/gh/apache/incubator-kyuubi/pull/2475/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhpdmUtYmVlbGluZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvaGl2ZS9iZWVsaW5lL0t5dXViaUJlZUxpbmUuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [.../org/apache/kyuubi/operation/KyuubiOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2475/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vS3l1dWJpT3BlcmF0aW9uLnNjYWxh) | `69.33% <0.00%> (+1.33%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2475?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2475?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [cdfae8d...434f298](https://codecov.io/gh/apache/incubator-kyuubi/pull/2475?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1120186298
cc @pan3793
Do you know the difference of k8s integration test?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1114005343
It seems that we add test resource `test-functions.jar` for four times.
Is it possible that other modules depends kyuubi-common:tests: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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1109362541
more simple way
```scala
val jars = spark.sparkContext.listJars().map { jar =>
spark.sharedState.jarClassLoader.getResource(new File(jar).getName)
}
repl.addUrlsToClassPath(jars: _*)
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1120096803
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] iodone commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
iodone commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1108047887
> Can you verify using spark.sharedState.jarClassLoader at
>
> https://github.com/apache/incubator-kyuubi/blob/c29c4430aa0fb15de8984a24fa61490ec2cf5bdc/externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/repl/KyuubiSparkILoop.scala#L47
OK， i will try 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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1120097155
```
- scala code with loading external package at runtime *** FAILED ***
org.apache.kyuubi.shade.org.apache.hive.service.cli.HiveSQLException: Error operating EXECUTE_STATEMENT: java.lang.NullPointerException
at scala.tools.nsc.Global.assoc$1(Global.scala:885)
at scala.tools.nsc.Global.$anonfun$invalidateClassPathEntries$2(Global.scala:900)
at scala.collection.TraversableLike.$anonfun$flatMap$1(TraversableLike.scala:293)
at scala.collection.mutable.ResizableArray.foreach(ResizableArray.scala:62)
at scala.collection.mutable.ResizableArray.foreach$(ResizableArray.scala:55)
at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:49)
at scala.collection.TraversableLike.flatMap(TraversableLike.scala:293)
at scala.collection.TraversableLike.flatMap$(TraversableLike.scala:290)
at scala.collection.AbstractTraversable.flatMap(Traversable.scala:108)
at scala.tools.nsc.Global.invalidateClassPathEntries(Global.scala:900)
at scala.tools.nsc.Global.extendCompilerClassPath(Global.scala:835)
at scala.tools.nsc.interpreter.IMain.addUrlsToClassPath(IMain.scala:254)
at org.apache.kyuubi.engine.spark.operation.ExecuteScala.$anonfun$runInternal$1(ExecuteScala.scala:86)
at scala.runtime.java8.JFunction0$mcV$sp.apply(JFunction0$mcV$sp.java:23)
at org.apache.kyuubi.engine.spark.operation.SparkOperation.withLocalProperties(SparkOperation.scala:88)
at org.apache.kyuubi.engine.spark.operation.ExecuteScala.runInternal(ExecuteScala.scala:60)
at org.apache.kyuubi.operation.AbstractOperation.run(AbstractOperation.scala:157)
at org.apache.kyuubi.session.AbstractSession.runOperation(AbstractSession.scala:94)
at org.apache.kyuubi.engine.spark.session.SparkSessionImpl.runOperation(SparkSessionImpl.scala:65)
at org.apache.kyuubi.session.AbstractSession.$anonfun$executeStatement$1(AbstractSession.scala:123)
at org.apache.kyuubi.session.AbstractSession.withAcquireRelease(AbstractSession.scala:75)
at org.apache.kyuubi.session.AbstractSession.executeStatement(AbstractSession.scala:120)
at org.apache.kyuubi.service.AbstractBackendService.executeStatement(AbstractBackendService.scala:66)
at org.apache.kyuubi.service.TFrontendService.ExecuteStatement(TFrontendService.scala:233)
at org.apache.kyuubi.shade.org.apache.hive.service.rpc.thrift.TCLIService$Processor$ExecuteStatement.getResult(TCLIService.java:1557)
at org.apache.kyuubi.shade.org.apache.hive.service.rpc.thrift.TCLIService$Processor$ExecuteStatement.getResult(TCLIService.java:1542)
at org.apache.kyuubi.shade.org.apache.thrift.ProcessFunction.process(ProcessFunction.java:39)
at org.apache.kyuubi.shade.org.apache.thrift.TBaseProcessor.process(TBaseProcessor.java:39)
at org.apache.kyuubi.service.authentication.TSetIpAddressProcessor.process(TSetIpAddressProcessor.scala:36)
at org.apache.kyuubi.shade.org.apache.thrift.server.TThreadPoolServer$WorkerProcess.run(TThreadPoolServer.java:286)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
at java.base/java.lang.Thread.run(Unknown Source)
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1120587226
So the blocker is: the new added test depends on a local jar but the k8s-it failed since the pod can not aware of the jar location ?
We can add the test in `SparkOperationSuite` first. In real world, the location of added jar should always at remote.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1108216202
> @yaooqinn, Using `spark.sharedState.jarClassLoader` does not work, because this setting is in the process of initialization and will only be set once?
yes. looks we can get added jars from sc.listJars and resolve them accordingly？
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] iodone commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
iodone commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1109214435
>
> > @yaooqinn, Using `spark.sharedState.jarClassLoader` does not work, because this setting is in the process of initialization and will only be set once?
> yes. looks we can get added jars from sc.listJars and resolve them accordingly？
Can you explain further? I cannot get to your point, here is my understanding:
When `spark.sql(add jar)` is executed, the added jar of`session.sharedState.jarClassLoader` is available and the classpath of the current thread is set. However, the classpath does not exist in the repl's runtime, and the path of the add jar needs to be added via `repl.addUrlsToClassPath` after each execution of the statement.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] iodone commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
iodone commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1120698211
> So the blocker is: the new added test depends on a local jar but the k8s-it failed since the pod can not aware of the jar location ?
>
> We can add the test in `SparkOperationSuite` first. In real world, the location of added jar should always at remote.
Yes, I think this problem is causing IT blocker. I'll try it to follow your suggestion, 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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1109316014
```scala
val jars = spark.sparkContext.listJars().map { jar =>
val jarStr = SparkFiles.get(new File(jar).getName)
val jarUri = new Path(jarStr).toUri
if (jarUri.getScheme == null) {
new File(jarStr).toURI.toURL
} else {
jarUri.toURL
}
}
repl.addUrlsToClassPath(jars: _*)
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] iodone commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
iodone commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1120564814
> It seems that for integration test, the `test-functions.jar` is an inner resource of kyuubi-common:tests jar.
>
> So the UT will fail.
Checked out the IT process and found some differences from UT.
This latest commit is tested by loading a jar package from external in K8s IT, similar to local file loading and OnYarn from HDFS loading. The jar file generated on the Client side needs a shared storage similar to HDFS to ensure that the Spark Engine Pod can access it.
Above I think this is the reason why IT test can not pass，@turboFei WDYT?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#discussion_r867338670
##########
kyuubi-common/src/test/scala/org/apache/kyuubi/operation/SparkQueryTests.scala:
##########
@@ -608,6 +610,34 @@ trait SparkQueryTests extends HiveJDBCTestHelper {
}
}
+ test("scala code with loading external package at runtime ") {
+ val jarDir = Utils.createTempDir().toFile
+
+ withJdbcStatement() { statement =>
+ statement.execute("SET kyuubi.operation.language=scala")
+
+ val jarFileInputStream =
+ Thread.currentThread().getContextClassLoader.getResourceAsStream("test-function.jar")
Review Comment:
It seems that the `test-function.jar` must be a File instead of resource.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1120187032
It seems that for integration test, the `test-functions.jar` is an inner resource of kyuubi-common:tests jar.
So the UT will fail.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] iodone commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
iodone commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1114105299
> It seems that we add test resource `test-functions.jar` for four times.
>
> Is it possible that other modules depends kyuubi-common:tests:jar?
I have spent a lot of time solving this problem recently, and have tried many methods, but since each module is tested separately, putting it in another module will result in not finding the `test-function.jar` path. And this does not let the k8s integration test pass. I need some help, can you give me some advice?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1114129810
> > It seems that we add test resource `test-functions.jar` for four times.
> > Is it possible that other modules depends kyuubi-common:tests:jar?
>
> I have spent a lot of time solving this problem recently, and have tried many methods, but since each module is tested separately, putting it in another module will result in not finding the `test-function.jar` path. And this does not let the k8s integration test pass. I need some help, can you give me some advice?
Got it. It seems that spark does not support the jar with schema 'jar:fille:'.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#discussion_r867339000
##########
kyuubi-common/src/test/scala/org/apache/kyuubi/operation/SparkQueryTests.scala:
##########
@@ -608,6 +610,34 @@ trait SparkQueryTests extends HiveJDBCTestHelper {
}
}
+ test("scala code with loading external package at runtime ") {
+ val jarDir = Utils.createTempDir().toFile
+
+ withJdbcStatement() { statement =>
+ statement.execute("SET kyuubi.operation.language=scala")
+
+ val jarFileInputStream =
+ Thread.currentThread().getContextClassLoader.getResourceAsStream("test-function.jar")
Review Comment:
let me raise a PR based on yours and have a try.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] iodone commented on a diff in pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
iodone commented on code in PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#discussion_r871112614
##########
integration-tests/kyuubi-kubernetes-it/pom.xml:
##########
@@ -80,6 +80,18 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>commons-io</groupId>
+ <artifactId>commons-io</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.scala-lang</groupId>
+ <artifactId>scala-compiler</artifactId>
+ <scope>test</scope>
+ </dependency>
Review Comment:
yes, IT tests extend from `SparkQueryTests`. `SparkQueryTests` depends on the above two packages.
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#discussion_r867280894
##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/ExecuteScala.scala:
##########
@@ -64,6 +68,27 @@ class ExecuteScala(
if (legacyOutput.nonEmpty) {
warn(s"Clearing legacy output from last interpreting:\n $legacyOutput")
}
+
+ val jars = spark.sparkContext.listJars().flatMap { jar =>
+ val jarFile = new File(jar)
+ spark.sharedState.jarClassLoader.getURLs.groupBy(_.getFile.split("/").last).get(
+ jarFile.getName) match {
Review Comment:
got 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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#discussion_r867342046
##########
kyuubi-common/src/test/scala/org/apache/kyuubi/operation/SparkQueryTests.scala:
##########
@@ -608,6 +613,73 @@ trait SparkQueryTests extends HiveJDBCTestHelper {
}
}
+ test("scala code with loading external package at runtime ") {
+ val jarDir = Utils.createTempDir().toFile
+
+ withJdbcStatement() { statement =>
+ statement.execute("SET kyuubi.operation.language=scala")
+ val udfCode =
Review Comment:
or add
```
// scalastyle:off
val udfCode =
"""
|package test.utils
|
|object Math {
| def add(x: Int, y: Int): Int = x + y
|
| def main(args: Array[String]): Unit = {
| println(add(1, 2))
| }
|}
|
|""".stripMargin
// scalastyle:on
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#discussion_r871048436
##########
integration-tests/kyuubi-kubernetes-it/pom.xml:
##########
@@ -80,6 +80,18 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>commons-io</groupId>
+ <artifactId>commons-io</artifactId>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.scala-lang</groupId>
+ <artifactId>scala-compiler</artifactId>
+ <scope>test</scope>
+ </dependency>
Review Comment:
unnecessary ?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] ulysses-you closed pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
ulysses-you closed pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
URL: https://github.com/apache/incubator-kyuubi/pull/2475
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] iodone commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
iodone commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1108117169
@yaooqinn, Using `spark.sharedState.jarClassLoader` does not work, because this setting is in the process of initialization and will only be set once?
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#discussion_r862421705
##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/ExecuteScala.scala:
##########
@@ -64,6 +68,27 @@ class ExecuteScala(
if (legacyOutput.nonEmpty) {
warn(s"Clearing legacy output from last interpreting:\n $legacyOutput")
}
+
+ val jars = spark.sparkContext.listJars().flatMap { jar =>
+ val jarFile = new File(jar)
+ spark.sharedState.jarClassLoader.getURLs.groupBy(_.getFile.split("/").last).get(
+ jarFile.getName) match {
Review Comment:
Should we use URL?
```
val jarName = new URL(jar).getFile.split("/").last
spark.sharedState.jarClassLoader.getURLs.groupBy(_.getFile.split("/").last).get(
jarName) match
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on a diff in pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
turboFei commented on code in PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#discussion_r867342046
##########
kyuubi-common/src/test/scala/org/apache/kyuubi/operation/SparkQueryTests.scala:
##########
@@ -608,6 +613,73 @@ trait SparkQueryTests extends HiveJDBCTestHelper {
}
}
+ test("scala code with loading external package at runtime ") {
+ val jarDir = Utils.createTempDir().toFile
+
+ withJdbcStatement() { statement =>
+ statement.execute("SET kyuubi.operation.language=scala")
+ val udfCode =
Review Comment:
```
val udfCode =
"""package test.utils
|object Math {
| def add(x: Int, y: Int): Int = x + y
| def main(args: Array[String]): Unit = {
| println(add(1, 2))
| }
|}
|""".stripMargin
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] turboFei commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
turboFei commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1120192927
```
error file=/home/runner/work/incubator-kyuubi/incubator-kyuubi/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/SparkQueryTests.scala message=Line contains a tab line=626 column=11
error file=/home/runner/work/incubator-kyuubi/incubator-kyuubi/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/SparkQueryTests.scala message=Line contains a tab line=628 column=11
error file=/home/runner/work/incubator-kyuubi/incubator-kyuubi/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/SparkQueryTests.scala message=Line contains a tab line=629 column=11
error file=/home/runner/work/incubator-kyuubi/incubator-kyuubi/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/SparkQueryTests.scala message=Line contains a tab line=630 column=11
```
You can check scala style with below command:
```
build/mvn scalastyle:check
```
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] iodone commented on a diff in pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
iodone commented on code in PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#discussion_r866430975
##########
externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/ExecuteScala.scala:
##########
@@ -64,6 +68,27 @@ class ExecuteScala(
if (legacyOutput.nonEmpty) {
warn(s"Clearing legacy output from last interpreting:\n $legacyOutput")
}
+
+ val jars = spark.sparkContext.listJars().flatMap { jar =>
+ val jarFile = new File(jar)
+ spark.sharedState.jarClassLoader.getURLs.groupBy(_.getFile.split("/").last).get(
+ jarFile.getName) match {
Review Comment:
`SparkContext.listJars` are of type `spark://` scheme, URL that does not support this type
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1124718904
Late + 1
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #2475: [KYUUBI #2471] Fix the bug of dynamically loading external packages
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #2475:
URL: https://github.com/apache/incubator-kyuubi/pull/2475#issuecomment-1108045179
Can you verify using spark.sharedState.jarClassLoader at https://github.com/apache/incubator-kyuubi/blob/c29c4430aa0fb15de8984a24fa61490ec2cf5bdc/externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/repl/KyuubiSparkILoop.scala#L47
--
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: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org