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/22 09:23:43 UTC
[GitHub] [incubator-kyuubi] deadwind4 opened a new pull request, #2446: [KYUUBI #2433] Hive default load required jars
deadwind4 opened a new pull request, #2446:
URL: https://github.com/apache/incubator-kyuubi/pull/2446
<!--
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.
-->
We submit a hive job to yarn, when we don't set `kyuubi.engine.hive.extra.classpath`, HiveSQLEngine will throw an error.
My improvement is load required jars when users don't set this option.
### _How was this patch tested?_
- [ ] Add some test cases that check the changes thoroughly including negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [x] [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] yaooqinn commented on pull request #2446: [KYUUBI #2433] HiveSQLEngine load required jars from HIVE_HADOOP_CLASSPATH
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #2446:
URL: https://github.com/apache/incubator-kyuubi/pull/2446#issuecomment-1109800858
thanks, merged 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] yaooqinn commented on a diff in pull request #2446: [KYUUBI #2433] Hive default load required jars
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2446:
URL: https://github.com/apache/incubator-kyuubi/pull/2446#discussion_r856058218
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala:
##########
@@ -81,16 +81,25 @@ class HiveProcessBuilder(
extraCp.foreach(classpathEntries.add)
if (extraCp.isEmpty) {
warn(s"The conf of kyuubi.engine.hive.extra.classpath is empty.")
- mainResource.foreach { path =>
- val devHadoopJars = Paths.get(path).getParent
- .resolve(s"scala-$SCALA_COMPILE_VERSION")
- .resolve("jars")
- if (!Files.exists(devHadoopJars)) {
- throw new KyuubiException(s"The path $devHadoopJars does not exists. Please set " +
- s"kyuubi.engine.hive.extra.classpath for configuring location of " +
- s"hadoop client jars, etc")
+ val distJars = new File(s"${env.get(KYUUBI_HOME).orNull}${File.separator}jars")
Review Comment:
Kyuubi Server lib is designed isolated with engine jars, if we mix them it's hard to maintain in the future
--
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] deadwind4 commented on pull request #2446: [KYUUBI #2433] HiveSQLEngine load required jars from HIVE_HADOOP_CLASSPATH
Posted by GitBox <gi...@apache.org>.
deadwind4 commented on PR #2446:
URL: https://github.com/apache/incubator-kyuubi/pull/2446#issuecomment-1108124430
> Maybe we shall add `HIVE_HADOOP_CLASSPATH` to kyuubi-env.sh.template
I added it in template @yaooqinn
--
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 a diff in pull request #2446: [KYUUBI #2433] Hive default load required jars
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2446:
URL: https://github.com/apache/incubator-kyuubi/pull/2446#discussion_r856105652
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala:
##########
@@ -81,16 +81,25 @@ class HiveProcessBuilder(
extraCp.foreach(classpathEntries.add)
if (extraCp.isEmpty) {
warn(s"The conf of kyuubi.engine.hive.extra.classpath is empty.")
- mainResource.foreach { path =>
- val devHadoopJars = Paths.get(path).getParent
- .resolve(s"scala-$SCALA_COMPILE_VERSION")
- .resolve("jars")
- if (!Files.exists(devHadoopJars)) {
- throw new KyuubiException(s"The path $devHadoopJars does not exists. Please set " +
- s"kyuubi.engine.hive.extra.classpath for configuring location of " +
- s"hadoop client jars, etc")
+ val distJars = new File(s"${env.get(KYUUBI_HOME).orNull}${File.separator}jars")
Review Comment:
Another thing that we shall not add the Hadoop client jars by default is
- both hive distribution and the hive-sql-engine.jar are not created for specific Hadoop versions, users can set accordingly. If we don't propagate them, class not found exception; if we do, compatibility issue lurks.
--
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 a diff in pull request #2446: [KYUUBI #2433] HiveSQLEngine load required jars from HIVE_HADOOP_CLASSPATH
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2446:
URL: https://github.com/apache/incubator-kyuubi/pull/2446#discussion_r857237375
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala:
##########
@@ -39,6 +39,8 @@ class HiveProcessBuilder(
private val hiveHome: String = getEngineHome("hive")
+ private val HIVE_HADOOP_CLASSPATH: String = "HIVE_HADOOP_CLASSPATH"
Review Comment:
nit: unnecessary val?
--
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] deadwind4 commented on a diff in pull request #2446: [KYUUBI #2433] Hive default load required jars
Posted by GitBox <gi...@apache.org>.
deadwind4 commented on code in PR #2446:
URL: https://github.com/apache/incubator-kyuubi/pull/2446#discussion_r857089816
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala:
##########
@@ -81,16 +81,25 @@ class HiveProcessBuilder(
extraCp.foreach(classpathEntries.add)
if (extraCp.isEmpty) {
warn(s"The conf of kyuubi.engine.hive.extra.classpath is empty.")
- mainResource.foreach { path =>
- val devHadoopJars = Paths.get(path).getParent
- .resolve(s"scala-$SCALA_COMPILE_VERSION")
- .resolve("jars")
- if (!Files.exists(devHadoopJars)) {
- throw new KyuubiException(s"The path $devHadoopJars does not exists. Please set " +
- s"kyuubi.engine.hive.extra.classpath for configuring location of " +
- s"hadoop client jars, etc")
+ val distJars = new File(s"${env.get(KYUUBI_HOME).orNull}${File.separator}jars")
Review Comment:
> If not, how to make it easier for user to understand and use
I second this. I will achieve loading both HIVE_HADOOP_CLASSPATH and `kyuubi.engine.hive.extra.classpath`.
HIVE_HADOOP_CLASSPATH is required. And I will explain how to set HIVE_HADOOP_CLASSPATH in doc.
--
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] deadwind4 commented on a diff in pull request #2446: [KYUUBI #2433] Hive default load required jars
Posted by GitBox <gi...@apache.org>.
deadwind4 commented on code in PR #2446:
URL: https://github.com/apache/incubator-kyuubi/pull/2446#discussion_r856064793
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala:
##########
@@ -81,16 +81,25 @@ class HiveProcessBuilder(
extraCp.foreach(classpathEntries.add)
if (extraCp.isEmpty) {
warn(s"The conf of kyuubi.engine.hive.extra.classpath is empty.")
- mainResource.foreach { path =>
- val devHadoopJars = Paths.get(path).getParent
- .resolve(s"scala-$SCALA_COMPILE_VERSION")
- .resolve("jars")
- if (!Files.exists(devHadoopJars)) {
- throw new KyuubiException(s"The path $devHadoopJars does not exists. Please set " +
- s"kyuubi.engine.hive.extra.classpath for configuring location of " +
- s"hadoop client jars, etc")
+ val distJars = new File(s"${env.get(KYUUBI_HOME).orNull}${File.separator}jars")
Review Comment:
Could I package `hadoop-client*jar` and `common-collection-*jar` into `dist/externals/engines/hive/jars`, and load this path into classpath? Or, I enable `HIVE_HADOOP_CLASSPATH` to let users set these 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] codecov-commenter commented on pull request #2446: [KYUUBI #2433] Hive default load required jars
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #2446:
URL: https://github.com/apache/incubator-kyuubi/pull/2446#issuecomment-1106415811
# [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2446?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 [#2446](https://codecov.io/gh/apache/incubator-kyuubi/pull/2446?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9763f34) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/7fa04947c4ea2a280648b837e134265430c7ec89?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7fa0494) will **increase** coverage by `0.11%`.
> The diff coverage is `43.75%`.
```diff
@@ Coverage Diff @@
## master #2446 +/- ##
============================================
+ Coverage 63.07% 63.18% +0.11%
Complexity 69 69
============================================
Files 366 366
Lines 17438 17445 +7
Branches 2341 2342 +1
============================================
+ Hits 10999 11023 +24
+ Misses 5409 5396 -13
+ Partials 1030 1026 -4
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/2446?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...apache/kyuubi/engine/hive/HiveProcessBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2446/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvaGl2ZS9IaXZlUHJvY2Vzc0J1aWxkZXIuc2NhbGE=) | `77.77% <43.75%> (-7.33%)` | :arrow_down: |
| [...org/apache/kyuubi/ha/client/ServiceDiscovery.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2446/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-a3l1dWJpLWhhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2hhL2NsaWVudC9TZXJ2aWNlRGlzY292ZXJ5LnNjYWxh) | `83.33% <0.00%> (-8.34%)` | :arrow_down: |
| [.../kyuubi/credentials/HadoopCredentialsManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2446/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jcmVkZW50aWFscy9IYWRvb3BDcmVkZW50aWFsc01hbmFnZXIuc2NhbGE=) | `93.12% <0.00%> (-0.77%)` | :arrow_down: |
| [...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2446/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvUHJvY0J1aWxkZXIuc2NhbGE=) | `80.12% <0.00%> (-0.63%)` | :arrow_down: |
| [...ache/kyuubi/plugin/spark/authz/OperationType.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2446/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-ZXh0ZW5zaW9ucy9zcGFyay9reXV1Ymktc3BhcmstYXV0aHovc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvcGx1Z2luL3NwYXJrL2F1dGh6L09wZXJhdGlvblR5cGUuc2NhbGE=) | `70.00% <0.00%> (+6.25%)` | :arrow_up: |
| [.../kyuubi/plugin/spark/authz/PrivilegesBuilder.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/2446/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-ZXh0ZW5zaW9ucy9zcGFyay9reXV1Ymktc3BhcmstYXV0aHovc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvcGx1Z2luL3NwYXJrL2F1dGh6L1ByaXZpbGVnZXNCdWlsZGVyLnNjYWxh) | `80.79% <0.00%> (+7.60%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/2446?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/2446?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 [7fa0494...9763f34](https://codecov.io/gh/apache/incubator-kyuubi/pull/2446?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] yaooqinn commented on pull request #2446: [KYUUBI #2433] HiveSQLEngine load required jars from HIVE_HADOOP_CLASSPATH
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on PR #2446:
URL: https://github.com/apache/incubator-kyuubi/pull/2446#issuecomment-1108040197
Maybe we shall add `HIVE_HADOOP_CLASSPATH` to kyuubi-env.sh.template
--
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] deadwind4 commented on a diff in pull request #2446: [KYUUBI #2433] HiveSQLEngine load required jars from HIVE_HADOOP_CLASSPATH
Posted by GitBox <gi...@apache.org>.
deadwind4 commented on code in PR #2446:
URL: https://github.com/apache/incubator-kyuubi/pull/2446#discussion_r857280810
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala:
##########
@@ -39,6 +39,8 @@ class HiveProcessBuilder(
private val hiveHome: String = getEngineHome("hive")
+ private val HIVE_HADOOP_CLASSPATH: String = "HIVE_HADOOP_CLASSPATH"
Review Comment:
It's usefull. I change all "HIVE_HADOOP_CLASSPATH" literal to this constant. In the future, we could move all similar constants to a scala file.
The following code uses this constant.
https://github.com/deadwind4/incubator-kyuubi/blob/9f9fcdac5b6f77eb03e20d6e41f95b03a1dc9568/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala#L82-L95
--
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 a diff in pull request #2446: [KYUUBI #2433] HiveSQLEngine load required jars from HIVE_HADOOP_CLASSPATH
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2446:
URL: https://github.com/apache/incubator-kyuubi/pull/2446#discussion_r857304096
##########
conf/kyuubi-env.sh.template:
##########
@@ -46,6 +46,10 @@
# - FLINK_HOME Flink distribution which you would like to use in Kyuubi.
# - FLINK_CONF_DIR Optional directory where the Flink configuration lives.
# (Default: $FLINK_HOME/conf)
+# - HIVE_HOME Hive distribution which you would like to use in Kyuubi.
+# - HIVE_CONF_DIR Optional directory where the Hive configuration lives.
+# (Default: $HIVE_HOME/conf)
+# - HIVE_HADOOP_CLASSPATH Required hadoop jars when you use Kyuubi Hive engine on Yarn.
Review Comment:
Not only for Yarn?
--
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 closed pull request #2446: [KYUUBI #2433] HiveSQLEngine load required jars from HIVE_HADOOP_CLASSPATH
Posted by GitBox <gi...@apache.org>.
yaooqinn closed pull request #2446: [KYUUBI #2433] HiveSQLEngine load required jars from HIVE_HADOOP_CLASSPATH
URL: https://github.com/apache/incubator-kyuubi/pull/2446
--
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] deadwind4 commented on pull request #2446: [KYUUBI #2433] HiveSQLEngine load required jars from HIVE_HADOOP_CLASSPATH
Posted by GitBox <gi...@apache.org>.
deadwind4 commented on PR #2446:
URL: https://github.com/apache/incubator-kyuubi/pull/2446#issuecomment-1107827669
I will complete doc in this PR https://github.com/apache/incubator-kyuubi/pull/2326
--
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] deadwind4 commented on a diff in pull request #2446: [KYUUBI #2433] HiveSQLEngine load required jars from HIVE_HADOOP_CLASSPATH
Posted by GitBox <gi...@apache.org>.
deadwind4 commented on code in PR #2446:
URL: https://github.com/apache/incubator-kyuubi/pull/2446#discussion_r857280810
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala:
##########
@@ -39,6 +39,8 @@ class HiveProcessBuilder(
private val hiveHome: String = getEngineHome("hive")
+ private val HIVE_HADOOP_CLASSPATH: String = "HIVE_HADOOP_CLASSPATH"
Review Comment:
It's usefull. I change all "HIVE_HADOOP_CLASSPATH" literal to this constant. In the future, we could move all similar constants to a scala file.
The following code uses this constant.
https://github.com/deadwind4/incubator-kyuubi/blob/9f9fcdac5b6f77eb03e20d6e41f95b03a1dc9568/kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala#L82-L95
--
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 a diff in pull request #2446: [KYUUBI #2433] Hive default load required jars
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on code in PR #2446:
URL: https://github.com/apache/incubator-kyuubi/pull/2446#discussion_r856091046
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/hive/HiveProcessBuilder.scala:
##########
@@ -81,16 +81,25 @@ class HiveProcessBuilder(
extraCp.foreach(classpathEntries.add)
if (extraCp.isEmpty) {
warn(s"The conf of kyuubi.engine.hive.extra.classpath is empty.")
- mainResource.foreach { path =>
- val devHadoopJars = Paths.get(path).getParent
- .resolve(s"scala-$SCALA_COMPILE_VERSION")
- .resolve("jars")
- if (!Files.exists(devHadoopJars)) {
- throw new KyuubiException(s"The path $devHadoopJars does not exists. Please set " +
- s"kyuubi.engine.hive.extra.classpath for configuring location of " +
- s"hadoop client jars, etc")
+ val distJars = new File(s"${env.get(KYUUBI_HOME).orNull}${File.separator}jars")
Review Comment:
> Could I package hadoop-client*jar and common-collection-*jar into dist/externals/engines/hive/jars
I have done this before, but this 1) make the binary distribution too large. 2) it may become a blocker for multiple hive version support
kyuubi.engine.hive.extra.classpath will do the same thing as HIVE_HADOOP_CLASSPATH
the thing is the AT-LEAST classpath of a hive engine is made by
- hive-sql-engine.jar
- HIVE_HOME/libs with a hive distribution
- hadoop client jars
And here we mainly focus on the **hadoop client jars** and the deps.
- Is it possible to make it transparent for users？
- a fat jar within hive-sql-engine.jar?
- make a separate folder and deliver it with the binary release?
- If not, how to make it easier for user to understand and use
- documentation is required
- HIVE_HADOOP_CLASSPATH or kyuubi.engine.hive.extra.classpath, or both?
- what are their priorities?
- shall we raise an error or not?
--
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