You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "merrily01 (via GitHub)" <gi...@apache.org> on 2023/03/15 13:04:51 UTC
[GitHub] [kyuubi] merrily01 opened a new pull request, #4527: [KYUUBI #4526][FOLLOWUP] Ensure that the feature of `Supplying pluggable GroupProvider` is only valid for the GROUP engine share level.
merrily01 opened a new pull request, #4527:
URL: https://github.com/apache/kyuubi/pull/4527
<!--
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.
-->
As described in the discussion: https://github.com/apache/kyuubi/discussions/4512
Since the pr https://github.com/apache/kyuubi/pull/3897 , we introduced the feature of `Supplying pluggable GroupProvider`.
But now when we create a new EngineRef, no matter which engine share level is used, getGroupNames will always be called.
This may cause users to encounter some confusion stack trace when they do not use the GROUP engine share level.
So we solve the problem through moving the `sessionManager.groupProvider.primaryGroup` to the inside of `EngineRef.appUser`, to ensure that the feature of `Supplying pluggable GroupProvider` is only valid for the GROUP engine share level.
### _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.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] yaooqinn commented on a diff in pull request #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#discussion_r1138081110
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -77,7 +77,7 @@ class KyuubiSessionImpl(
lazy val engine: EngineRef = new EngineRef(
sessionConf,
user,
- sessionManager.groupProvider.primaryGroup(user, optimizedConf.asJava),
Review Comment:
this is also a bugfix as using optimized conf is wrong 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: 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] merrily01 commented on pull request #4527: [KYUUBI #4526][FOLLOWUP] Ensure that the feature of `Supplying pluggable GroupProvider` is only valid for the GROUP engine share level.
Posted by "merrily01 (via GitHub)" <gi...@apache.org>.
merrily01 commented on PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#issuecomment-1469975404
cc / @pan3793 @yaooqinn Could you please kindly review ?
--
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] merrily01 commented on pull request #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "merrily01 (via GitHub)" <gi...@apache.org>.
merrily01 commented on PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#issuecomment-1471173231
> @merrily01, thanks for your contribution, would you mind updating your GitHub profile to link the email [maruilei@58.com](mailto:maruilei@58.com) to your GitHub account? Otherwise, your contributions will not be counted by GitHub.
Sorry，already updated.
--
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] yaooqinn commented on a diff in pull request #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#discussion_r1138081472
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala:
##########
@@ -85,7 +87,7 @@ private[kyuubi] class EngineRef(
// Launcher of the engine
private[kyuubi] val appUser: String = shareLevel match {
case SERVER => Utils.currentUser
- case GROUP => primaryGroup
+ case GROUP => groupProvider.primaryGroup(user, Map.empty[String, String].asJava)
Review Comment:
sessionConf.getAll
--
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 closed pull request #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 closed pull request #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
URL: https://github.com/apache/kyuubi/pull/4527
--
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 #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#issuecomment-1471259412
## [Codecov](https://codecov.io/gh/apache/kyuubi/pull/4527?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 [#4527](https://codecov.io/gh/apache/kyuubi/pull/4527?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c63a0f9) into [master](https://codecov.io/gh/apache/kyuubi/commit/38cf59d47be8f6d1b45562259c9e6342215888de?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (38cf59d) will **decrease** coverage by `0.03%`.
> The diff coverage is `50.00%`.
> :exclamation: Current head c63a0f9 differs from pull request most recent head 0a858a9. Consider uploading reports for the commit 0a858a9 to get more accurate results
```diff
@@ Coverage Diff @@
## master #4527 +/- ##
============================================
- Coverage 53.42% 53.40% -0.03%
Complexity 13 13
============================================
Files 571 571
Lines 31350 31350
Branches 4225 4225
============================================
- Hits 16750 16741 -9
- Misses 13022 13029 +7
- Partials 1578 1580 +2
```
| [Impacted Files](https://codecov.io/gh/apache/kyuubi/pull/4527?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...ain/scala/org/apache/kyuubi/engine/EngineRef.scala](https://codecov.io/gh/apache/kyuubi/pull/4527?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) | `71.65% <0.00%> (ø)` | |
| [.../org/apache/kyuubi/session/KyuubiSessionImpl.scala](https://codecov.io/gh/apache/kyuubi/pull/4527?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) | `79.72% <100.00%> (ø)` | |
... and [6 files with indirect coverage changes](https://codecov.io/gh/apache/kyuubi/pull/4527/indirect-changes?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] pan3793 commented on pull request #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#issuecomment-1470488322
@merrily01, thanks for your contribution, would you mind updating your GitHub profile to link the email <ma...@58.com> to your GitHub account? Otherwise, your contributions will not be counted by GitHub.
--
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 #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#issuecomment-1471685662
> ... whether it is related to code change or test environment.
@merrily01 it looks like a flaky test, @ulysses-you I remember you have fixed it but seems flaky again.
> who should close the Resolve conversation? the contributor or reviewer?
we don't have strict rules for that, normally, the committer will glance the conversations and evaluate if the PR can get merged.
--
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] yaooqinn commented on a diff in pull request #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#discussion_r1138018335
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala:
##########
@@ -51,7 +53,7 @@ import org.apache.kyuubi.operation.log.OperationLog
private[kyuubi] class EngineRef(
conf: KyuubiConf,
user: String,
- primaryGroup: String,
+ groupProvider: GroupProvider,
Review Comment:
It seems also can be lazily initialized, not needed in the constructor?
--
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] merrily01 commented on a diff in pull request #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "merrily01 (via GitHub)" <gi...@apache.org>.
merrily01 commented on code in PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#discussion_r1138085480
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala:
##########
@@ -51,7 +53,7 @@ import org.apache.kyuubi.operation.log.OperationLog
private[kyuubi] class EngineRef(
conf: KyuubiConf,
user: String,
- primaryGroup: String,
+ groupProvider: GroupProvider,
Review Comment:
Ok, thank you
--
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] merrily01 commented on pull request #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "merrily01 (via GitHub)" <gi...@apache.org>.
merrily01 commented on PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#issuecomment-1471673366
Hi~ @yaooqinn @pan3793 sorry to bother you guys,
I found that some checks in the workflow failed, such as:
```
Error: Exception in thread "pool-182-thread-3" java.lang.Error: org.apache.kyuubi.KyuubiSQLException: Timeout to lock on path [/engine_test2_1.8.0-SNAPSHOT_USER_SPARK_SQL_lock/runner/default] after 4500 ms. There would be some problem that other session may create engine timeout.
```
And I don't know whether it is related to code change or test environment.
So, mabey I want to know how to re-trigger the pipline?
In addition, by the way, in our project specification, when I modify or reply to the reviewer's comments,
who should close the `Resolve conversation`? the contributor or reviewer?
--
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] merrily01 commented on a diff in pull request #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "merrily01 (via GitHub)" <gi...@apache.org>.
merrily01 commented on code in PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#discussion_r1138087399
##########
kyuubi-server/src/test/scala/org/apache/kyuubi/engine/EngineRefTests.scala:
##########
@@ -68,7 +71,9 @@ trait EngineRefTests extends KyuubiFunSuite {
Seq(None, Some("suffix")).foreach { domain =>
conf.set(KyuubiConf.ENGINE_SHARE_LEVEL, CONNECTION.toString)
domain.foreach(conf.set(KyuubiConf.ENGINE_SHARE_LEVEL_SUBDOMAIN.key, _))
- val engine = new EngineRef(conf, user, "grp", id, null)
+ conf.set(KyuubiConf.GROUP_PROVIDER, "hadoop")
+
+ val engine = new EngineRef(conf, user, PluginLoader.loadGroupProvider(conf), id, null)
Review Comment:
Ok, thank you
--
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] yaooqinn commented on a diff in pull request #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#discussion_r1138074192
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala:
##########
@@ -51,7 +53,7 @@ import org.apache.kyuubi.operation.log.OperationLog
private[kyuubi] class EngineRef(
conf: KyuubiConf,
user: String,
- primaryGroup: String,
+ groupProvider: GroupProvider,
Review Comment:
the cost is mostly test code refactoring, but I'm OK to keep the PR AS-IS.
--
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] merrily01 commented on a diff in pull request #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "merrily01 (via GitHub)" <gi...@apache.org>.
merrily01 commented on code in PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#discussion_r1138087239
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/session/KyuubiSessionImpl.scala:
##########
@@ -77,7 +77,7 @@ class KyuubiSessionImpl(
lazy val engine: EngineRef = new EngineRef(
sessionConf,
user,
- sessionManager.groupProvider.primaryGroup(user, optimizedConf.asJava),
Review Comment:
Yeah, actually it seems that conf is not used in subsequent calls ?
```
# HadoopGroupProvider.scala
override def groups(user: String, sessionConf: JMap[String, String]): Array[String] =
UserGroupInformation.createRemoteUser(user).getGroupNames match {
case Array() =>
warn(s"There is no group for $user, use the client user name as group directly")
Array(user)
case groups => groups
}
```
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala:
##########
@@ -85,7 +87,7 @@ private[kyuubi] class EngineRef(
// Launcher of the engine
private[kyuubi] val appUser: String = shareLevel match {
case SERVER => Utils.currentUser
- case GROUP => primaryGroup
+ case GROUP => groupProvider.primaryGroup(user, Map.empty[String, String].asJava)
Review Comment:
Done
--
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] merrily01 commented on a diff in pull request #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "merrily01 (via GitHub)" <gi...@apache.org>.
merrily01 commented on code in PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#discussion_r1138053857
##########
kyuubi-server/src/main/scala/org/apache/kyuubi/engine/EngineRef.scala:
##########
@@ -51,7 +53,7 @@ import org.apache.kyuubi.operation.log.OperationLog
private[kyuubi] class EngineRef(
conf: KyuubiConf,
user: String,
- primaryGroup: String,
+ groupProvider: GroupProvider,
Review Comment:
Thanks for your reply.
It seems that the groupProvider needs to obtain from KyuubiSessionManager. Whether it is passing KyuubiSessionManager through the constructor or trying to initialize and create one, the chaotic changes introduced and the cost paid may be greater than the current transformation.
--
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] yaooqinn commented on a diff in pull request #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#discussion_r1138082550
##########
kyuubi-server/src/test/scala/org/apache/kyuubi/engine/EngineRefTests.scala:
##########
@@ -68,7 +71,9 @@ trait EngineRefTests extends KyuubiFunSuite {
Seq(None, Some("suffix")).foreach { domain =>
conf.set(KyuubiConf.ENGINE_SHARE_LEVEL, CONNECTION.toString)
domain.foreach(conf.set(KyuubiConf.ENGINE_SHARE_LEVEL_SUBDOMAIN.key, _))
- val engine = new EngineRef(conf, user, "grp", id, null)
+ conf.set(KyuubiConf.GROUP_PROVIDER, "hadoop")
+
+ val engine = new EngineRef(conf, user, PluginLoader.loadGroupProvider(conf), id, null)
Review Comment:
The group provider is unnecessary here, but OK to hold down the LOC
--
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] merrily01 commented on pull request #4527: [KYUUBI #4526][FOLLOWUP] Defer evaluation for GroupProvider
Posted by "merrily01 (via GitHub)" <gi...@apache.org>.
merrily01 commented on PR #4527:
URL: https://github.com/apache/kyuubi/pull/4527#issuecomment-1471689164
> > ... whether it is related to code change or test environment.
>
> @merrily01 it looks like a flaky test, @ulysses-you I remember you have fixed it but seems flaky again.
>
> > who should close the Resolve conversation? the contributor or reviewer?
>
> we don't have strict rules for that, normally, the committer will glance the conversations and evaluate if the PR can get merged.
OK， Got it！
Thanks for your patience and help. :) @yaooqinn @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: 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