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 2023/01/19 06:40:03 UTC

[GitHub] [kyuubi] bowenliang123 opened a new pull request, #4191: [Improvement] Resolve Maven dependencies before building `dependencyList`

bowenliang123 opened a new pull request, #4191:
URL: https://github.com/apache/kyuubi/pull/4191

   <!--
   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/CONTRIBUTING.html
     2. If the PR is related to an issue in https://github.com/apache/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.
   -->
   - Resolve Maven dependencies before building `dependencyList`
   
   We often use `build/dependency.sh` to check/update `dependencyList` on the snapshot branches, thus many daily updated snapshot releases will be downloaded again and this process is invisible and make it seems jammed.
   
   Considering this behaving and the reason for it, suggest to run `mvn dependency:resolve` for making sure snapshot and changed  Maven dependencies ready with readable outputs before building `dependencyList` .
   
   
   ### _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/master/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] [kyuubi] cfmcgrady commented on pull request #4191: [Improvement] Resolve Maven dependencies before building `dependencyList`

Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on PR #4191:
URL: https://github.com/apache/kyuubi/pull/4191#issuecomment-1407898553

   Since all dependencies are stable, and only project submodules are snapshot versions, why don't we skip snapshot updates when updating the dependencies list?
   
   We can skip snapshot update by running
   
   ```
   mvn goal --no-snapshot-updates
   ```
   or
   ```
   mvn goal -nsu
   ```
   
   FYI: https://stackoverflow.com/questions/1348603/how-can-i-get-maven-to-stop-attempting-to-check-for-updates-for-artifacts-from-a


-- 
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] [kyuubi] codecov-commenter commented on pull request #4191: [Improvement] Resolve Maven dependencies before building `dependencyList`

Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #4191:
URL: https://github.com/apache/kyuubi/pull/4191#issuecomment-1408027776

   # [Codecov](https://codecov.io/gh/apache/kyuubi/pull/4191?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 [#4191](https://codecov.io/gh/apache/kyuubi/pull/4191?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e077cd8) into [master](https://codecov.io/gh/apache/kyuubi/commit/3d91a9651bdf2ff5ed701218f6f8d9b3525a5f16?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (3d91a96) will **decrease** coverage by `0.06%`.
   > The diff coverage is `n/a`.
   
   > :exclamation: Current head e077cd8 differs from pull request most recent head 0bf3681. Consider uploading reports for the commit 0bf3681 to get more accurate results
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #4191      +/-   ##
   ============================================
   - Coverage     53.01%   52.95%   -0.06%     
     Complexity       13       13              
   ============================================
     Files           548      548              
     Lines         29946    29948       +2     
     Branches       4025     4025              
   ============================================
   - Hits          15875    15860      -15     
   - Misses        12596    12608      +12     
   - Partials       1475     1480       +5     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/kyuubi/pull/4191?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/jdbc/hive/ClosedOrCancelledException.java](https://codecov.io/gh/apache/kyuubi/pull/4191?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhpdmUtamRiYy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUva3l1dWJpL2pkYmMvaGl2ZS9DbG9zZWRPckNhbmNlbGxlZEV4Y2VwdGlvbi5qYXZh) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
   | [.../kyuubi/server/mysql/constant/MySQLErrorCode.scala](https://codecov.io/gh/apache/kyuubi/pull/4191?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbXlzcWwvY29uc3RhbnQvTXlTUUxFcnJvckNvZGUuc2NhbGE=) | `13.84% <0.00%> (-6.16%)` | :arrow_down: |
   | [...ache/kyuubi/server/mysql/MySQLCommandHandler.scala](https://codecov.io/gh/apache/kyuubi/pull/4191?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbXlzcWwvTXlTUUxDb21tYW5kSGFuZGxlci5zY2FsYQ==) | `77.77% <0.00%> (-4.05%)` | :arrow_down: |
   | [...ache/kyuubi/server/mysql/MySQLGenericPackets.scala](https://codecov.io/gh/apache/kyuubi/pull/4191?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbXlzcWwvTXlTUUxHZW5lcmljUGFja2V0cy5zY2FsYQ==) | `76.59% <0.00%> (-2.13%)` | :arrow_down: |
   | [...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala](https://codecov.io/gh/apache/kyuubi/pull/4191?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2xvZy9Mb2dCYXRjaENvbW1hbmQuc2NhbGE=) | `59.09% <0.00%> (-1.52%)` | :arrow_down: |
   | [.../engine/spark/session/SparkSQLSessionManager.scala](https://codecov.io/gh/apache/kyuubi/pull/4191?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9zZXNzaW9uL1NwYXJrU1FMU2Vzc2lvbk1hbmFnZXIuc2NhbGE=) | `78.48% <0.00%> (-1.27%)` | :arrow_down: |
   | [...pache/kyuubi/server/metadata/MetadataManager.scala](https://codecov.io/gh/apache/kyuubi/pull/4191?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvbWV0YWRhdGEvTWV0YWRhdGFNYW5hZ2VyLnNjYWxh) | `83.01% <0.00%> (-1.26%)` | :arrow_down: |
   | [...apache/kyuubi/service/TBinaryFrontendService.scala](https://codecov.io/gh/apache/kyuubi/pull/4191?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL1RCaW5hcnlGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `48.38% <0.00%> (-1.08%)` | :arrow_down: |
   | [...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala](https://codecov.io/gh/apache/kyuubi/pull/4191?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.24% <0.00%> (-0.62%)` | :arrow_down: |
   | [.../org/apache/kyuubi/session/KyuubiSessionImpl.scala](https://codecov.io/gh/apache/kyuubi/pull/4191?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXNzaW9uL0t5dXViaVNlc3Npb25JbXBsLnNjYWxh) | `76.35% <0.00%> (-0.37%)` | :arrow_down: |
   | ... and [1 more](https://codecov.io/gh/apache/kyuubi/pull/4191?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   :mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?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] [kyuubi] bowenliang123 commented on pull request #4191: [Improvement] Skip fetching Maven dependency snapshot update in building `dependencyList`

Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #4191:
URL: https://github.com/apache/kyuubi/pull/4191#issuecomment-1408031249

   > Since all dependencies are stable, and only project submodules are snapshot versions, why don't we skip snapshot updates when updating the dependencies list?
   > 
   > We can skip snapshot update by running
   > 
   > ```
   > mvn goal --no-snapshot-updates
   > ```
   > 
   > or
   > 
   > ```
   > mvn goal -nsu
   > ```
   > 
   > FYI: https://stackoverflow.com/questions/1348603/how-can-i-get-maven-to-stop-attempting-to-check-for-updates-for-artifacts-from-a
   
   SGTM. Applied the suggestion of using `--no-snapshot-updates` option and also changed the title and description 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: 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] [kyuubi] pan3793 commented on pull request #4191: [Improvement] Resolve Maven dependencies before building `dependencyList`

Posted by GitBox <gi...@apache.org>.
pan3793 commented on PR #4191:
URL: https://github.com/apache/kyuubi/pull/4191#issuecomment-1398115338

   cc @cfmcgrady 


-- 
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] [kyuubi] cfmcgrady commented on pull request #4191: [Improvement] Resolve Maven dependencies before building `dependencyList`

Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on PR #4191:
URL: https://github.com/apache/kyuubi/pull/4191#issuecomment-1407896991

   Since all dependencies are stable, and only project submodules are snapshot versions, why don't we skip snapshot updates when updating the dependencies list?
   
   We can skip snapshot update by running
   
   ```
   mvn goal --no-snapshot-updates
   ```
   or
   ```
   mvn goal -nsu
   ```
   
   FYI: https://stackoverflow.com/questions/1348603/how-can-i-get-maven-to-stop-attempting-to-check-for-updates-for-artifacts-from-a


-- 
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] [kyuubi] bowenliang123 commented on pull request #4191: [Improvement] Resolve Maven dependencies before building `dependencyList`

Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #4191:
URL: https://github.com/apache/kyuubi/pull/4191#issuecomment-1407823991

   > > thus many daily updated snapshot releases will be downloaded again and this process is invisible and make it seems jammed with no console output.
   > 
   > do we have any daily updated snapshot dependencies?
   
   Maven is redownloading the submodules from `apache.snapshots` every day after daily snapshots are published.
   <img width="1452" alt="image" src="https://user-images.githubusercontent.com/1935105/215365436-985de7d1-728c-407a-a04b-67cccb5d90ed.png">
   


-- 
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] [kyuubi] pan3793 commented on a diff in pull request #4191: [Improvement] Resolve Maven dependencies before building `dependencyList`

Posted by GitBox <gi...@apache.org>.
pan3793 commented on code in PR #4191:
URL: https://github.com/apache/kyuubi/pull/4191#discussion_r1081488223


##########
build/dependency.sh:
##########
@@ -81,6 +81,8 @@ cat >"${DEP_PR}"<<EOF
 
 EOF
 
+$MVN dependency:resolve

Review Comment:
   is it possible to combine it w/ line 35? just like 
   
   ```
   $MVN dependency:resolve dependency:build-classpath -pl ...
   ```



-- 
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] [kyuubi] bowenliang123 commented on pull request #4191: [Improvement] Skip fetching Maven dependency snapshot update when building `dependencyList`

Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #4191:
URL: https://github.com/apache/kyuubi/pull/4191#issuecomment-1408032830

   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] [kyuubi] bowenliang123 commented on a diff in pull request #4191: [Improvement] Resolve Maven dependencies before building `dependencyList`

Posted by GitBox <gi...@apache.org>.
bowenliang123 commented on code in PR #4191:
URL: https://github.com/apache/kyuubi/pull/4191#discussion_r1081497986


##########
build/dependency.sh:
##########
@@ -81,6 +81,8 @@ cat >"${DEP_PR}"<<EOF
 
 EOF
 
+$MVN dependency:resolve

Review Comment:
   Several reasons to do it separately,
   1.  better not to do in one place as not sure whether the change will impact or contaminate the outputs for dependencyList.
   2. solely resolving maven dependency and the output will be more clear to read and to know which process we are waiting for (eg. resolving deps , building_classpath)



-- 
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] [kyuubi] cfmcgrady commented on pull request #4191: [Improvement] Resolve Maven dependencies before building `dependencyList`

Posted by "cfmcgrady (via GitHub)" <gi...@apache.org>.
cfmcgrady commented on PR #4191:
URL: https://github.com/apache/kyuubi/pull/4191#issuecomment-1407422247

   > thus many daily updated snapshot releases will be downloaded again and this process is invisible and make it seems jammed with no console output.
   
   do we have any daily updated snapshot dependencies?


-- 
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] [kyuubi] bowenliang123 closed pull request #4191: [Improvement] Skip fetching Maven dependency snapshot update when building `dependencyList`

Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 closed pull request #4191: [Improvement] Skip fetching Maven dependency snapshot update when building `dependencyList`
URL: https://github.com/apache/kyuubi/pull/4191


-- 
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