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/27 06:42:40 UTC

[GitHub] [incubator-kyuubi] SteNicholas opened a new pull request #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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


   <!--
   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?_
   
   Support executing statement async for Flink SQL engine.
   
   ### _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: 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 #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?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 [#1845](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (616ddd2) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/53d59a02bf04bc5e1b81119ee56cdc6e819c4d8a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (53d59a0) will **increase** coverage by `0.24%`.
   > The diff coverage is `88.88%`.
   
   > :exclamation: Current head 616ddd2 differs from pull request most recent head 4ccf4b4. Consider uploading reports for the commit 4ccf4b4 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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/1845?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    #1845      +/-   ##
   ============================================
   + Coverage     59.46%   59.70%   +0.24%     
   + Complexity      272      270       -2     
   ============================================
     Files           277      282       +5     
     Lines         13853    13996     +143     
     Branches       1776     1785       +9     
   ============================================
   + Hits           8238     8357     +119     
   - Misses         4911     4927      +16     
   - Partials        704      712       +8     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...kyuubi/engine/spark/operation/SparkOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9vcGVyYXRpb24vU3BhcmtPcGVyYXRpb24uc2NhbGE=) | `81.57% <0.00%> (-1.32%)` | :arrow_down: |
   | [...uubi/engine/flink/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `79.04% <100.00%> (+1.49%)` | :arrow_up: |
   | [...e/authentication/KyuubiAuthenticationFactory.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0t5dXViaUF1dGhlbnRpY2F0aW9uRmFjdG9yeS5zY2FsYQ==) | `75.86% <0.00%> (-2.32%)` | :arrow_down: |
   | [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3RhdGVtZW50LnNjYWxh) | `64.70% <0.00%> (-1.18%)` | :arrow_down: |
   | [...apache/kyuubi/service/TBinaryFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL1RCaW5hcnlGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `85.10% <0.00%> (-0.32%)` | :arrow_down: |
   | [...ation/EngineSecureAuthenticationProviderImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0VuZ2luZVNlY3VyZUF1dGhlbnRpY2F0aW9uUHJvdmlkZXJJbXBsLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...uthentication/KyuubiInternalAccessIdentifier.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0t5dXViaUludGVybmFsQWNjZXNzSWRlbnRpZmllci5zY2FsYQ==) | `100.00% <0.00%> (ø)` | |
   | [...on/ZooKeeperEngineSecuritySecretProviderImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWhhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NlcnZpY2UvYXV0aGVudGljYXRpb24vWm9vS2VlcGVyRW5naW5lU2VjdXJpdHlTZWNyZXRQcm92aWRlckltcGwuc2NhbGE=) | `0.00% <0.00%> (ø)` | |
   | [...ervice/authentication/EngineSecurityAccessor.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0VuZ2luZVNlY3VyaXR5QWNjZXNzb3Iuc2NhbGE=) | `82.00% <0.00%> (ø)` | |
   | [.../authentication/EngineSecuritySecretProvider.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0VuZ2luZVNlY3VyaXR5U2VjcmV0UHJvdmlkZXIuc2NhbGE=) | `100.00% <0.00%> (ø)` | |
   | ... and [6 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?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/1845?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 [53d59a0...4ccf4b4](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?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 #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?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 [#1845](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d9c2741) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/53d59a02bf04bc5e1b81119ee56cdc6e819c4d8a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (53d59a0) will **increase** coverage by `0.24%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head d9c2741 differs from pull request most recent head 616ddd2. Consider uploading reports for the commit 616ddd2 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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/1845?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    #1845      +/-   ##
   ============================================
   + Coverage     59.46%   59.71%   +0.24%     
   + Complexity      272      271       -1     
   ============================================
     Files           277      282       +5     
     Lines         13853    13996     +143     
     Branches       1776     1785       +9     
   ============================================
   + Hits           8238     8358     +120     
   - Misses         4911     4927      +16     
   - Partials        704      711       +7     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...uubi/engine/flink/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `79.04% <100.00%> (+1.49%)` | :arrow_up: |
   | [...e/authentication/KyuubiAuthenticationFactory.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0t5dXViaUF1dGhlbnRpY2F0aW9uRmFjdG9yeS5zY2FsYQ==) | `75.86% <0.00%> (-2.32%)` | :arrow_down: |
   | [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3RhdGVtZW50LnNjYWxh) | `64.70% <0.00%> (-1.18%)` | :arrow_down: |
   | [...apache/kyuubi/service/TBinaryFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL1RCaW5hcnlGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `85.10% <0.00%> (-0.32%)` | :arrow_down: |
   | [.../authentication/EngineSecuritySecretProvider.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0VuZ2luZVNlY3VyaXR5U2VjcmV0UHJvdmlkZXIuc2NhbGE=) | `100.00% <0.00%> (ø)` | |
   | [...uthentication/KyuubiInternalAccessIdentifier.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0t5dXViaUludGVybmFsQWNjZXNzSWRlbnRpZmllci5zY2FsYQ==) | `100.00% <0.00%> (ø)` | |
   | [...on/ZooKeeperEngineSecuritySecretProviderImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWhhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL3NlcnZpY2UvYXV0aGVudGljYXRpb24vWm9vS2VlcGVyRW5naW5lU2VjdXJpdHlTZWNyZXRQcm92aWRlckltcGwuc2NhbGE=) | `0.00% <0.00%> (ø)` | |
   | [...ervice/authentication/EngineSecurityAccessor.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0VuZ2luZVNlY3VyaXR5QWNjZXNzb3Iuc2NhbGE=) | `82.00% <0.00%> (ø)` | |
   | [...ation/EngineSecureAuthenticationProviderImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0VuZ2luZVNlY3VyZUF1dGhlbnRpY2F0aW9uUHJvdmlkZXJJbXBsLnNjYWxh) | `0.00% <0.00%> (ø)` | |
   | [...a/org/apache/kyuubi/service/TFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL1RGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `94.75% <0.00%> (+0.01%)` | :arrow_up: |
   | ... and [5 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?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/1845?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 [53d59a0...616ddd2](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?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] hddong commented on a change in pull request #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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



##########
File path: externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/SparkOperation.scala
##########
@@ -62,7 +62,7 @@ abstract class SparkOperation(opType: OperationType, session: Session)
   protected def cleanup(targetState: OperationState): Unit = state.synchronized {
     if (!isTerminalState(state)) {
       setState(targetState)
-      Option(getBackgroundHandle).foreach(_.cancel(true))
+      if (shouldRunAsync) Option(getBackgroundHandle).foreach(_.cancel(true))

Review comment:
       Wrong file? This is `SparkOperation`

##########
File path: externals/kyuubi-spark-sql-engine/src/test/scala/org/apache/kyuubi/engine/spark/operation/SparkOperationSuite.scala
##########
@@ -323,9 +323,6 @@ class SparkOperationSuite extends WithSparkSQLEngine with HiveMetadataTests with
 
   test("basic open | execute | close") {
     withThriftClient { client =>
-      val operationManager =

Review comment:
       ditto




-- 
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] yanghua closed pull request #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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


   


-- 
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] SteNicholas commented on a change in pull request #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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



##########
File path: externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/SparkOperation.scala
##########
@@ -62,7 +62,7 @@ abstract class SparkOperation(opType: OperationType, session: Session)
   protected def cleanup(targetState: OperationState): Unit = state.synchronized {
     if (!isTerminalState(state)) {
       setState(targetState)
-      Option(getBackgroundHandle).foreach(_.cancel(true))
+      if (shouldRunAsync) Option(getBackgroundHandle).foreach(_.cancel(true))

Review comment:
       The `cleanup` of the `SparkOperation` doesn't check the `shouldRunAsync` to cancel the background handle. Therefore I fixed the logic in this 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] SteNicholas commented on pull request #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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


   @pan3793 @yanghua @yaooqinn , could you please help to review this 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] SteNicholas commented on a change in pull request #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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



##########
File path: externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/SparkOperation.scala
##########
@@ -62,7 +62,7 @@ abstract class SparkOperation(opType: OperationType, session: Session)
   protected def cleanup(targetState: OperationState): Unit = state.synchronized {
     if (!isTerminalState(state)) {
       setState(targetState)
-      Option(getBackgroundHandle).foreach(_.cancel(true))
+      if (shouldRunAsync) Option(getBackgroundHandle).foreach(_.cancel(true))

Review comment:
       @hddong, the `cleanup` of the `SparkOperation` doesn't check the `shouldRunAsync` to cancel the background handle. Therefore I fixed the logic in this 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] hddong commented on a change in pull request #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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



##########
File path: externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/SparkOperation.scala
##########
@@ -62,7 +62,7 @@ abstract class SparkOperation(opType: OperationType, session: Session)
   protected def cleanup(targetState: OperationState): Unit = state.synchronized {
     if (!isTerminalState(state)) {
       setState(targetState)
-      Option(getBackgroundHandle).foreach(_.cancel(true))
+      if (shouldRunAsync) Option(getBackgroundHandle).foreach(_.cancel(true))

Review comment:
       @SteNicholas : I'm not clear for `FlinkOperation`, just mean the change here is unnecessary.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@kyuubi.apache.org

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

[GitHub] [incubator-kyuubi] SteNicholas commented on a change in pull request #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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



##########
File path: externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/operation/ExecuteStatement.scala
##########
@@ -75,11 +75,18 @@ class ExecuteStatement(
       val asyncOperation = new Runnable {
         override def run(): Unit = {
           OperationLog.setCurrentOperationLog(operationLog)
+          val sessionHandle = session.sessionManager.openSession(

Review comment:
       @yaooqinn, I don't think this only need executeStatement because the `THREAD_PROVIDERS` thread local variable gets nothing in Flink. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@kyuubi.apache.org

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

[GitHub] [incubator-kyuubi] SteNicholas edited a comment on pull request #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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


   @pan3793, @yanghua, @yaooqinn, could you please help to review this 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] SteNicholas commented on a change in pull request #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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



##########
File path: externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/SparkOperation.scala
##########
@@ -62,7 +62,7 @@ abstract class SparkOperation(opType: OperationType, session: Session)
   protected def cleanup(targetState: OperationState): Unit = state.synchronized {
     if (!isTerminalState(state)) {
       setState(targetState)
-      Option(getBackgroundHandle).foreach(_.cancel(true))
+      if (shouldRunAsync) Option(getBackgroundHandle).foreach(_.cancel(true))

Review comment:
       @hddong, as you mentioned, the `cleanup` of the `FlinkOperation` doesn't also check the `shouldRunAsync`, right?




-- 
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 #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?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 [#1845](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4ccf4b4) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/53d59a02bf04bc5e1b81119ee56cdc6e819c4d8a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (53d59a0) will **increase** coverage by `0.25%`.
   > The diff coverage is `88.88%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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/1845?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    #1845      +/-   ##
   ============================================
   + Coverage     59.46%   59.72%   +0.25%     
   + Complexity      272      267       -5     
   ============================================
     Files           277      282       +5     
     Lines         13853    13983     +130     
     Branches       1776     1781       +5     
   ============================================
   + Hits           8238     8351     +113     
   - Misses         4911     4922      +11     
   - Partials        704      710       +6     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...kyuubi/engine/flink/operation/FlinkOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9vcGVyYXRpb24vRmxpbmtPcGVyYXRpb24uc2NhbGE=) | `54.83% <0.00%> (+0.87%)` | :arrow_up: |
   | [...uubi/engine/flink/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `79.04% <100.00%> (+1.49%)` | :arrow_up: |
   | [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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: |
   | [...e/authentication/KyuubiAuthenticationFactory.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0t5dXViaUF1dGhlbnRpY2F0aW9uRmFjdG9yeS5zY2FsYQ==) | `75.86% <0.00%> (-2.32%)` | :arrow_down: |
   | [...g/apache/spark/kyuubi/SparkSQLEngineListener.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsva3l1dWJpL1NwYXJrU1FMRW5naW5lTGlzdGVuZXIuc2NhbGE=) | `86.44% <0.00%> (-1.70%)` | :arrow_down: |
   | [...rg/apache/kyuubi/engine/spark/SparkSQLEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9TcGFya1NRTEVuZ2luZS5zY2FsYQ==) | `65.06% <0.00%> (-1.21%)` | :arrow_down: |
   | [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3RhdGVtZW50LnNjYWxh) | `64.70% <0.00%> (-1.18%)` | :arrow_down: |
   | [...apache/kyuubi/service/TBinaryFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL1RCaW5hcnlGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `85.10% <0.00%> (-0.32%)` | :arrow_down: |
   | [...he/kyuubi/engine/flink/operation/GetCatalogs.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9vcGVyYXRpb24vR2V0Q2F0YWxvZ3Muc2NhbGE=) | `83.33% <0.00%> (ø)` | |
   | [.../kyuubi/engine/flink/operation/GetTableTypes.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9vcGVyYXRpb24vR2V0VGFibGVUeXBlcy5zY2FsYQ==) | `100.00% <0.00%> (ø)` | |
   | ... and [14 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?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/1845?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 [53d59a0...4ccf4b4](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?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 a change in pull request #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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



##########
File path: externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/operation/ExecuteStatement.scala
##########
@@ -75,11 +75,18 @@ class ExecuteStatement(
       val asyncOperation = new Runnable {
         override def run(): Unit = {
           OperationLog.setCurrentOperationLog(operationLog)
+          val sessionHandle = session.sessionManager.openSession(

Review comment:
       we only need executeStatement here




-- 
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 #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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


   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?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 [#1845](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4ccf4b4) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/53d59a02bf04bc5e1b81119ee56cdc6e819c4d8a?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (53d59a0) will **increase** coverage by `0.25%`.
   > The diff coverage is `88.88%`.
   
   > :exclamation: Current head 4ccf4b4 differs from pull request most recent head 956739c. Consider uploading reports for the commit 956739c to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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/1845?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    #1845      +/-   ##
   ============================================
   + Coverage     59.46%   59.72%   +0.25%     
   + Complexity      272      267       -5     
   ============================================
     Files           277      282       +5     
     Lines         13853    13983     +130     
     Branches       1776     1781       +5     
   ============================================
   + Hits           8238     8351     +113     
   - Misses         4911     4922      +11     
   - Partials        704      710       +6     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...kyuubi/engine/flink/operation/FlinkOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9vcGVyYXRpb24vRmxpbmtPcGVyYXRpb24uc2NhbGE=) | `54.83% <0.00%> (+0.87%)` | :arrow_up: |
   | [...uubi/engine/flink/operation/ExecuteStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9vcGVyYXRpb24vRXhlY3V0ZVN0YXRlbWVudC5zY2FsYQ==) | `79.04% <100.00%> (+1.49%)` | :arrow_up: |
   | [...ache/kyuubi/operation/KyuubiOperationManager.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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: |
   | [...e/authentication/KyuubiAuthenticationFactory.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0t5dXViaUF1dGhlbnRpY2F0aW9uRmFjdG9yeS5zY2FsYQ==) | `75.86% <0.00%> (-2.32%)` | :arrow_down: |
   | [...g/apache/spark/kyuubi/SparkSQLEngineListener.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUvc3Bhcmsva3l1dWJpL1NwYXJrU1FMRW5naW5lTGlzdGVuZXIuc2NhbGE=) | `86.44% <0.00%> (-1.70%)` | :arrow_down: |
   | [...rg/apache/kyuubi/engine/spark/SparkSQLEngine.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS1zcGFyay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9zcGFyay9TcGFya1NRTEVuZ2luZS5zY2FsYQ==) | `65.06% <0.00%> (-1.21%)` | :arrow_down: |
   | [...rg/apache/kyuubi/engine/trino/TrinoStatement.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS10cmluby1lbmdpbmUvc3JjL21haW4vc2NhbGEvb3JnL2FwYWNoZS9reXV1YmkvZW5naW5lL3RyaW5vL1RyaW5vU3RhdGVtZW50LnNjYWxh) | `64.70% <0.00%> (-1.18%)` | :arrow_down: |
   | [...apache/kyuubi/service/TBinaryFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL1RCaW5hcnlGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `85.10% <0.00%> (-0.32%)` | :arrow_down: |
   | [...he/kyuubi/engine/flink/operation/GetCatalogs.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9vcGVyYXRpb24vR2V0Q2F0YWxvZ3Muc2NhbGE=) | `83.33% <0.00%> (ø)` | |
   | [.../kyuubi/engine/flink/operation/GetTableTypes.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/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-ZXh0ZXJuYWxzL2t5dXViaS1mbGluay1zcWwtZW5naW5lL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2VuZ2luZS9mbGluay9vcGVyYXRpb24vR2V0VGFibGVUeXBlcy5zY2FsYQ==) | `100.00% <0.00%> (ø)` | |
   | ... and [14 more](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?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/1845?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 [53d59a0...956739c](https://codecov.io/gh/apache/incubator-kyuubi/pull/1845?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] hddong commented on a change in pull request #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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



##########
File path: externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/SparkOperation.scala
##########
@@ -62,7 +62,7 @@ abstract class SparkOperation(opType: OperationType, session: Session)
   protected def cleanup(targetState: OperationState): Unit = state.synchronized {
     if (!isTerminalState(state)) {
       setState(targetState)
-      Option(getBackgroundHandle).foreach(_.cancel(true))
+      if (shouldRunAsync) Option(getBackgroundHandle).foreach(_.cancel(true))

Review comment:
       Seems the check is dispensable, `Option(getBackgroundHandle)` is None when sync execute.




-- 
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] SteNicholas commented on a change in pull request #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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



##########
File path: externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/operation/ExecuteStatement.scala
##########
@@ -75,11 +75,18 @@ class ExecuteStatement(
       val asyncOperation = new Runnable {
         override def run(): Unit = {
           OperationLog.setCurrentOperationLog(operationLog)
+          val sessionHandle = session.sessionManager.openSession(

Review comment:
       @yaooqinn, I don't think this only need executeStatement because the `THREAD_PROVIDERS` thread local get nothing in Flink. 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@kyuubi.apache.org

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

[GitHub] [incubator-kyuubi] SteNicholas commented on a change in pull request #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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



##########
File path: externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/operation/SparkOperation.scala
##########
@@ -62,7 +62,7 @@ abstract class SparkOperation(opType: OperationType, session: Session)
   protected def cleanup(targetState: OperationState): Unit = state.synchronized {
     if (!isTerminalState(state)) {
       setState(targetState)
-      Option(getBackgroundHandle).foreach(_.cancel(true))
+      if (shouldRunAsync) Option(getBackgroundHandle).foreach(_.cancel(true))

Review comment:
       @hddong, as you mentioned, the `cleanup` of the `FlinkOperation` doesn't also check the `shouldRunAsync`, right?




-- 
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 #1845: [KYUUBI #1829] Support executing statement async for Flink SQL engine

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



##########
File path: externals/kyuubi-flink-sql-engine/src/main/scala/org/apache/kyuubi/engine/flink/operation/ExecuteStatement.scala
##########
@@ -75,11 +75,18 @@ class ExecuteStatement(
       val asyncOperation = new Runnable {
         override def run(): Unit = {
           OperationLog.setCurrentOperationLog(operationLog)
+          val sessionHandle = session.sessionManager.openSession(

Review comment:
       I do not follow you. However，there will not be any chance that we call openSession in an operation




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