You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/01/10 14:43:00 UTC

[GitHub] [incubator-kyuubi] yaooqinn opened a new pull request #1716: Unify Hive deps for server and client

yaooqinn opened a new pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716


   <!--
   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.
   -->
   
   
   ### _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
   
   - [ ] [Run test](https://kyuubi.readthedocs.io/en/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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] pan3793 edited a comment on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
pan3793 edited a comment on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1009990261


   I'm +0 on this change. ~~The initial idea of using shaded version is to hide the implementation of kyuubi hive jdbc client, and only expose standard JDBC API, then we can refactor the jdbc client step by step to cut the relation between hive deps.~~
   
   The deleted comment is for #1715.
   
   The initial idea of separating hive server/client deps management becasue we need hive jars in 1) kyuubi-server for kerberos 2) kyuubi-hive-jdbc for jdbc client 3) spark-extension for test. The minimal requirements of hive transitive deps of those are significant different, if we manage the exclusion of hive in root pom, it will affect anywhere. And because of 3) that spark 3.0/3.1 use hive 2.3.7 but spark 3.2 uses hive 2.3.9, we'd better use the default version of spark-hive depends to do test.
   
   Then we proposed to seal the hive in submodules, leave them to manage hive deps by themselves, and with the refactor of kyuubi hive jdbc client, we can drop hive jars step by step, without side effect of other modules.


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#discussion_r782140903



##########
File path: kyuubi-assembly/pom.xml
##########
@@ -94,6 +94,12 @@
             <scope>provided</scope>
         </dependency>
 
+        <dependency>

Review comment:
       we shall avoid introduce to server path, but this is needed for 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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] pan3793 edited a comment on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
pan3793 edited a comment on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1009990261


   I'm +0 on this change. ~~The initial idea of using shaded version is to hide the implementation of kyuubi hive jdbc client, and only expose standard JDBC API, then we can refactor the jdbc client step by step to cut the relation between hive deps.~~
   
   The initial idea of separating hive server/client deps management becasue we need hive jars in 1) kyuubi-server for kerberos 2) kyuubi-hive-jdbc for jdbc client 3) spark-extension for test. The minimal requirements of hive transitive deps of those are significant different, if we manage the exclusion of hive in root pom, it will affect anywhere. And because of 3) that spark 3.0/3.1 use hive 2.3.7 but spark 3.2 uses hive 2.3.9, we'd better use the default version of spark-hive depends to do test.
   
   Then we proposed to seal the hive in submodules, leave them to manage hive deps by themselves, and with the refactor of kyuubi hive jdbc client, we can drop hive jars step by step, without side effect of other modules.


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] yaooqinn removed a comment on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
yaooqinn removed a comment on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1010019598


   > if we manage the exclusion of hive in root pom
   
   Your thoughts seem to come from this misjudgment, we do not do this in the scope of this PR


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1009992080


   why does a unified hive.version block the cutting thing you mentioned?


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] ulysses-you commented on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1010774642


   looks fine. 
   
   I think we needn't specify the hive version in spark extension module and it's clear to make it consistent with client and kyuubi server. We can do further optimization if we meet the real issue about dependency.
   


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] pan3793 commented on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
pan3793 commented on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1010026373


   `hive-service-rpc` are quite light, but idealy we can 1) relocated it or 2)regenete from thrift IDL but change package name to `o.a.kyuubi` then we will never conflict with Spark runtime 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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1010019598


   > if we manage the exclusion of hive in root pom
   
   Your thoughts seem to come from this misjudgment, we do not do this in the scope of this PR


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] pan3793 commented on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
pan3793 commented on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1010022765


   > > if we manage the exclusion of hive in root pom
   > 
   > We are still do exclusion from module to module?
   
   Specific for Hive stuffs, yes.


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1009924283


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?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 [#1716](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2eb3bdf) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/90e974637a075408c0b0ef3d17a8e4b88ace72e8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (90e9746) will **increase** coverage by `0.01%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1716      +/-   ##
   ============================================
   + Coverage     59.51%   59.53%   +0.01%     
   - Complexity      232      233       +1     
   ============================================
     Files           268      268              
     Lines         13305    13305              
     Branches       1688     1688              
   ============================================
   + Hits           7919     7921       +2     
   + Misses         4709     4708       -1     
   + Partials        677      676       -1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...he/kyuubi/engine/spark/repl/KyuubiSparkILoop.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9yZXBsL0t5dXViaVNwYXJrSUxvb3Auc2NhbGE=) | `93.44% <0.00%> (+3.27%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?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/1716?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 [90e9746...2eb3bdf](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] ulysses-you commented on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1010775262


   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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] ulysses-you closed pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
ulysses-you closed pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716


   


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1009924283


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?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 [#1716](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (645d76a) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/77f860407be857f2b0b69a0c3facfe27c204f326?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (77f8604) will **decrease** coverage by `0.01%`.
   > The diff coverage is `n/a`.
   
   > :exclamation: Current head 645d76a differs from pull request most recent head 2eb3bdf. Consider uploading reports for the commit 2eb3bdf to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1716      +/-   ##
   ============================================
   - Coverage     59.52%   59.51%   -0.02%     
     Complexity      233      233              
   ============================================
     Files           268      268              
     Lines         13305    13304       -1     
     Branches       1688     1688              
   ============================================
   - Hits           7920     7918       -2     
     Misses         4709     4709              
   - Partials        676      677       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716/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==) | `92.50% <0.00%> (-2.50%)` | :arrow_down: |
   | [...in/scala/org/apache/kyuubi/server/api/v1/dto.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvYXBpL3YxL2R0by5zY2FsYQ==) | `64.70% <0.00%> (-0.68%)` | :arrow_down: |
   | [.../apache/kyuubi/server/api/v1/ApiRootResource.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvYXBpL3YxL0FwaVJvb3RSZXNvdXJjZS5zY2FsYQ==) | `91.66% <0.00%> (-0.34%)` | :arrow_down: |
   | [...rg/apache/kyuubi/zookeeper/EmbeddedZookeeper.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716/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-a3l1dWJpLXpvb2tlZXBlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS96b29rZWVwZXIvRW1iZWRkZWRab29rZWVwZXIuc2NhbGE=) | `87.09% <0.00%> (+0.43%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?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/1716?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 [90e9746...2eb3bdf](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1009924283


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?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 [#1716](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (645d76a) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/d2f10e9da800307dd793b014bcfb05720c5731b3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d2f10e9) will **decrease** coverage by `0.01%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1716      +/-   ##
   ============================================
   - Coverage     59.53%   59.51%   -0.02%     
   + Complexity      234      233       -1     
   ============================================
     Files           268      268              
     Lines         13304    13304              
     Branches       1688     1688              
   ============================================
   - Hits           7920     7918       -2     
   - Misses         4708     4709       +1     
   - Partials        676      677       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...he/kyuubi/engine/spark/repl/KyuubiSparkILoop.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9yZXBsL0t5dXViaVNwYXJrSUxvb3Auc2NhbGE=) | `90.16% <0.00%> (-3.28%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?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/1716?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 [b0d7dac...645d76a](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1009963532


   cc @pan3793 


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] pan3793 commented on a change in pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
pan3793 commented on a change in pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#discussion_r782162829



##########
File path: pom.xml
##########
@@ -309,6 +299,12 @@
                 </exclusions>
             </dependency>
 
+            <dependency>
+                <groupId>com.google.guava</groupId>
+                <artifactId>failureaccess</artifactId>
+                <version>1.0.1</version>

Review comment:
       nit: use property




-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] pan3793 edited a comment on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
pan3793 edited a comment on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1009990261






-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] pan3793 edited a comment on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
pan3793 edited a comment on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1009990261


   I'm +0 on this change. ~~The initial idea of using shaded version is to hide the implementation of kyuubi hive jdbc client, and only expose standard JDBC API, then we can refactor the jdbc client step by step to cut the relation between hive deps.~~
   
   The deleted comments for #1715.
   
   The initial idea of separating hive server/client deps management becasue we need hive jars in 1) kyuubi-server for kerberos 2) kyuubi-hive-jdbc for jdbc client 3) spark-extension for test. The minimal requirements of hive transitive deps of those are significant different, if we manage the exclusion of hive in root pom, it will affect anywhere. And because of 3) that spark 3.0/3.1 use hive 2.3.7 but spark 3.2 uses hive 2.3.9, we'd better use the default version of spark-hive depends to do test.
   
   Then we proposed to seal the hive in submodules, leave them to manage hive deps by themselves, and with the refactor of kyuubi hive jdbc client, we can drop hive jars step by step, without side effect of other modules.


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1010737009


   also, cc @ulysses-you and @turboFei 


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1010027931


   what's your point? in this pr, we still manage the exclustion as-is except hive-service-rpc


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#discussion_r782139958



##########
File path: kyuubi-assembly/pom.xml
##########
@@ -94,6 +94,12 @@
             <scope>provided</scope>
         </dependency>
 
+        <dependency>

Review comment:
       > Contains com.google.common.util.concurrent.internal.InternalFutureFailureAccess and InternalFutures. Most users will never need to use this artifact. Its classes is conceptually a part of Guava, but they're in this separate artifact so that Android libraries can use them without pulling in all of Guava (just as they can use ListenableFuture by depending on the listenablefuture artifact).
   




-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] pan3793 commented on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
pan3793 commented on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1010016040


   FYI @yaooqinn, I updated my previous comments 


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] yaooqinn commented on a change in pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#discussion_r782163605



##########
File path: pom.xml
##########
@@ -141,20 +142,13 @@
         <zookeeper.version>3.4.14</zookeeper.version>
 
         <!-- apply to kyuubi-hive-jdbc/kyuubi-hive-beeline module -->
-        <hive.client.version>2.3.9</hive.client.version>
-        <hive.client.commons-lang.version>2.6</hive.client.commons-lang.version>
         <hive.client.commons-cli.version>1.2</hive.client.commons-cli.version>
         <hive.client.commons-codec.version>1.15</hive.client.commons-codec.version>
-        <hive.client.curator.version>2.12.0</hive.client.curator.version>
-        <hive.client.guava.version>14.0.1</hive.client.guava.version>
         <hive.client.httpcomponents-client.version>4.5.13</hive.client.httpcomponents-client.version>
         <hive.client.httpcomponents-core.version>4.4.13</hive.client.httpcomponents-core.version>
         <hive.client.jline.version>2.12</hive.client.jline.version>
-        <hive.client.storage-api.version>2.4.0</hive.client.storage-api.version>
+        <hive.client.storage-api.version>2.7.2</hive.client.storage-api.version>

Review comment:
       align with spark




-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] pan3793 commented on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
pan3793 commented on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1009990261


   I'm +0 on this change. The initial idea of using shaded version is to hide the implementation of kyuubi hive jdbc client, and only expose standard JDBC API, then we can refactor the jdbc client step by step to cut the relation between hive deps.


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] codecov-commenter edited a comment on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1009924283


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?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 [#1716](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (645d76a) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/d2f10e9da800307dd793b014bcfb05720c5731b3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d2f10e9) will **decrease** coverage by `0.01%`.
   > The diff coverage is `n/a`.
   
   > :exclamation: Current head 645d76a differs from pull request most recent head c7f0f26. Consider uploading reports for the commit c7f0f26 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716/graphs/tree.svg?width=650&height=150&src=pr&token=925D4tb9AH&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #1716      +/-   ##
   ============================================
   - Coverage     59.53%   59.51%   -0.02%     
   + Complexity      234      233       -1     
   ============================================
     Files           268      268              
     Lines         13304    13304              
     Branches       1688     1688              
   ============================================
   - Hits           7920     7918       -2     
   - Misses         4708     4709       +1     
   - Partials        676      677       +1     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...he/kyuubi/engine/spark/repl/KyuubiSparkILoop.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9yZXBsL0t5dXViaVNwYXJrSUxvb3Auc2NhbGE=) | `90.16% <0.00%> (-3.28%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?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/1716?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 [b0d7dac...c7f0f26](https://codecov.io/gh/apache/incubator-kyuubi/pull/1716?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: commits-unsubscribe@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] pan3793 commented on a change in pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
pan3793 commented on a change in pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#discussion_r782162670



##########
File path: pom.xml
##########
@@ -141,20 +142,13 @@
         <zookeeper.version>3.4.14</zookeeper.version>
 
         <!-- apply to kyuubi-hive-jdbc/kyuubi-hive-beeline module -->
-        <hive.client.version>2.3.9</hive.client.version>
-        <hive.client.commons-lang.version>2.6</hive.client.commons-lang.version>
         <hive.client.commons-cli.version>1.2</hive.client.commons-cli.version>
         <hive.client.commons-codec.version>1.15</hive.client.commons-codec.version>
-        <hive.client.curator.version>2.12.0</hive.client.curator.version>
-        <hive.client.guava.version>14.0.1</hive.client.guava.version>
         <hive.client.httpcomponents-client.version>4.5.13</hive.client.httpcomponents-client.version>
         <hive.client.httpcomponents-core.version>4.4.13</hive.client.httpcomponents-core.version>
         <hive.client.jline.version>2.12</hive.client.jline.version>
-        <hive.client.storage-api.version>2.4.0</hive.client.storage-api.version>
+        <hive.client.storage-api.version>2.7.2</hive.client.storage-api.version>

Review comment:
       Why does this change required?




-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1010022646


   Or you mean the `hive-service-rpc` and it's deps(thrift s) sepically?


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1010023242


   > > > if we manage the exclusion of hive in root pom
   > > 
   > > 
   > > We are still do exclusion from module to module?
   > 
   > Specific for Hive stuffs, yes.
   
   NONONO, I mean "We are still do exclusion from module to module"


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [incubator-kyuubi] yaooqinn commented on pull request #1716: Unify Hive deps for server and client

Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #1716:
URL: https://github.com/apache/incubator-kyuubi/pull/1716#issuecomment-1010021816


   > if we manage the exclusion of hive in root pom
   
   We are still do exclusion from module to module?


-- 
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@kyuubi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org