You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by GitBox <gi...@apache.org> on 2022/08/20 15:09:00 UTC

[GitHub] [incubator-kyuubi] zhaomin1423 opened a new pull request, #3289: Filter Spark event logger outside the spark engine

zhaomin1423 opened a new pull request, #3289:
URL: https://github.com/apache/incubator-kyuubi/pull/3289

   <!--
   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.
   -->
   The default value of kyuubi.engine.event.loggers is SPARK, but it can't be used outside the spark engine, so filter it.
   
   ### _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.apache.org/docs/latest/develop_tools/testing.html#running-tests) locally before make a pull request
   


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

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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [incubator-kyuubi] github-actions[bot] commented on pull request #3289: Filter Spark event logger outside the spark engine

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #3289:
URL: https://github.com/apache/incubator-kyuubi/pull/3289#issuecomment-1332932657

   Thanks for the PR! This PR is being closed due to inactivity. This isn't a judgement on the merit of the PR in any way. If this is still an issue with the latest version of Kyuubi, please reopen it and ask a committer to remove the Stale tag!
   
   Thank you for using Kyuubi!


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

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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #3289: Filter Spark event logger outside the spark engine

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #3289:
URL: https://github.com/apache/incubator-kyuubi/pull/3289#discussion_r951024055


##########
externals/kyuubi-hive-sql-engine/src/main/scala/org/apache/kyuubi/engine/hive/HiveSQLEngine.scala:
##########
@@ -66,7 +65,6 @@ object HiveSQLEngine extends Logging {
   var currentEngine: Option[HiveSQLEngine] = None
   val hiveConf = new HiveConf()
   val kyuubiConf = new KyuubiConf()
-  kyuubiConf.set(ENGINE_EVENT_LOGGERS.key, "JSON")

Review Comment:
   one option maybe, we can add engine event loggers config for each engine.
   e.g.
   ```
   kyuubi.engine.spark.event.loggers
   kyuubi.engine.jdbc.event.loggers
   ...
   ```



##########
externals/kyuubi-hive-sql-engine/src/main/scala/org/apache/kyuubi/engine/hive/HiveSQLEngine.scala:
##########
@@ -66,7 +65,6 @@ object HiveSQLEngine extends Logging {
   var currentEngine: Option[HiveSQLEngine] = None
   val hiveConf = new HiveConf()
   val kyuubiConf = new KyuubiConf()
-  kyuubiConf.set(ENGINE_EVENT_LOGGERS.key, "JSON")

Review Comment:
   cc @yaooqinn 



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

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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [incubator-kyuubi] ulysses-you commented on a diff in pull request #3289: Filter Spark event logger outside the spark engine

Posted by GitBox <gi...@apache.org>.
ulysses-you commented on code in PR #3289:
URL: https://github.com/apache/incubator-kyuubi/pull/3289#discussion_r951023440


##########
externals/kyuubi-hive-sql-engine/src/main/scala/org/apache/kyuubi/engine/hive/HiveSQLEngine.scala:
##########
@@ -66,7 +65,6 @@ object HiveSQLEngine extends Logging {
   var currentEngine: Option[HiveSQLEngine] = None
   val hiveConf = new HiveConf()
   val kyuubiConf = new KyuubiConf()
-  kyuubiConf.set(ENGINE_EVENT_LOGGERS.key, "JSON")

Review Comment:
   this is a behavior change.. the default value is `SPARK` and you just filter that



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

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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [incubator-kyuubi] github-actions[bot] closed pull request #3289: Filter Spark event logger outside the spark engine

Posted by GitBox <gi...@apache.org>.
github-actions[bot] closed pull request #3289: Filter Spark event logger outside the spark engine
URL: https://github.com/apache/incubator-kyuubi/pull/3289


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

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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [incubator-kyuubi] codecov-commenter commented on pull request #3289: Filter Spark event logger outside the spark engine

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

   # [Codecov](https://codecov.io/gh/apache/incubator-kyuubi/pull/3289?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 [#3289](https://codecov.io/gh/apache/incubator-kyuubi/pull/3289?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0b5eeb9) into [master](https://codecov.io/gh/apache/incubator-kyuubi/commit/327336f1d6aac67e3cdbc5074be40cbf324e5022?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (327336f) will **decrease** coverage by `0.00%`.
   > The diff coverage is `50.00%`.
   
   ```diff
   @@             Coverage Diff              @@
   ##             master    #3289      +/-   ##
   ============================================
   - Coverage     51.38%   51.37%   -0.01%     
     Complexity       13       13              
   ============================================
     Files           468      469       +1     
     Lines         26220    26232      +12     
     Branches       3632     3631       -1     
   ============================================
   + Hits          13473    13477       +4     
   - Misses        11465    11470       +5     
   - Partials       1282     1285       +3     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/incubator-kyuubi/pull/3289?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...rg/apache/kyuubi/events/EventHandlerRegister.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3289/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-a3l1dWJpLWV2ZW50cy9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9ldmVudHMvRXZlbnRIYW5kbGVyUmVnaXN0ZXIuc2NhbGE=) | `56.52% <50.00%> (-3.48%)` | :arrow_down: |
   | [...uthentication/JdbcAuthenticationProviderImpl.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3289/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2aWNlL2F1dGhlbnRpY2F0aW9uL0pkYmNBdXRoZW50aWNhdGlvblByb3ZpZGVySW1wbC5zY2FsYQ==) | `73.21% <0.00%> (-6.54%)` | :arrow_down: |
   | [...apache/kyuubi/engine/JpsApplicationOperation.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3289/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvSnBzQXBwbGljYXRpb25PcGVyYXRpb24uc2NhbGE=) | `77.41% <0.00%> (-3.23%)` | :arrow_down: |
   | [...ain/scala/org/apache/kyuubi/engine/EngineRef.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3289/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvRW5naW5lUmVmLnNjYWxh) | `74.07% <0.00%> (-0.93%)` | :arrow_down: |
   | [...che/kyuubi/server/KyuubiTHttpFrontendService.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3289/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-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9zZXJ2ZXIvS3l1dWJpVEh0dHBGcm9udGVuZFNlcnZpY2Uuc2NhbGE=) | `64.23% <0.00%> (-0.73%)` | :arrow_down: |
   | [...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3289/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-a3l1dWJpLWN0bC9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jdGwvY21kL2xvZy9Mb2dCYXRjaENvbW1hbmQuc2NhbGE=) | `80.00% <0.00%> (ø)` | |
   | [.../main/scala/org/apache/kyuubi/util/JdbcUtils.scala](https://codecov.io/gh/apache/incubator-kyuubi/pull/3289/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-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS91dGlsL0pkYmNVdGlscy5zY2FsYQ==) | `87.50% <0.00%> (ø)` | |
   
   :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] [incubator-kyuubi] zhaomin1423 commented on a diff in pull request #3289: Filter Spark event logger outside the spark engine

Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on code in PR #3289:
URL: https://github.com/apache/incubator-kyuubi/pull/3289#discussion_r951599982


##########
externals/kyuubi-hive-sql-engine/src/main/scala/org/apache/kyuubi/engine/hive/HiveSQLEngine.scala:
##########
@@ -66,7 +65,6 @@ object HiveSQLEngine extends Logging {
   var currentEngine: Option[HiveSQLEngine] = None
   val hiveConf = new HiveConf()
   val kyuubiConf = new KyuubiConf()
-  kyuubiConf.set(ENGINE_EVENT_LOGGERS.key, "JSON")

Review Comment:
   add config for each engine. https://github.com/apache/incubator-kyuubi/pull/3309#issue-1346621427 



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

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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org

[GitHub] [incubator-kyuubi] zhaomin1423 commented on a diff in pull request #3289: Filter Spark event logger outside the spark engine

Posted by GitBox <gi...@apache.org>.
zhaomin1423 commented on code in PR #3289:
URL: https://github.com/apache/incubator-kyuubi/pull/3289#discussion_r951253018


##########
externals/kyuubi-hive-sql-engine/src/main/scala/org/apache/kyuubi/engine/hive/HiveSQLEngine.scala:
##########
@@ -66,7 +65,6 @@ object HiveSQLEngine extends Logging {
   var currentEngine: Option[HiveSQLEngine] = None
   val hiveConf = new HiveConf()
   val kyuubiConf = new KyuubiConf()
-  kyuubiConf.set(ENGINE_EVENT_LOGGERS.key, "JSON")

Review Comment:
   > one option maybe, we can add engine event loggers config for each engine.
   > e.g.
   > ```
   > kyuubi.engine.spark.event.loggers
   > kyuubi.engine.jdbc.event.loggers
   > ...
   > ```
   
   good idea



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