You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "bowenliang123 (via GitHub)" <gi...@apache.org> on 2023/02/27 14:53:40 UTC
[GitHub] [kyuubi] bowenliang123 opened a new pull request, #4427: Simplify assertions in Authz tests
bowenliang123 opened a new pull request, #4427:
URL: https://github.com/apache/kyuubi/pull/4427
<!--
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.
-->
Simplify assertions in Authz tests, by
- add `doAs(user) { ... }`
- add `runSqlAsInSuccess` for asserting a successful `Try`
- add `runSqlAsWithAccessException` to intercept `AccessControlException` and assert error message
### _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] bowenliang123 closed pull request #4427: Simplify assertions in Authz tests in declarative style
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 closed pull request #4427: Simplify assertions in Authz tests in declarative style
URL: https://github.com/apache/kyuubi/pull/4427
--
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 #4427: Simplify assertions in Authz tests in declarative style
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #4427:
URL: https://github.com/apache/kyuubi/pull/4427#issuecomment-1446759811
# [Codecov](https://codecov.io/gh/apache/kyuubi/pull/4427?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 [#4427](https://codecov.io/gh/apache/kyuubi/pull/4427?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (b8bc7f1) into [master](https://codecov.io/gh/apache/kyuubi/commit/5aed270c453516fd3559aa668e56b2e1badf1c93?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5aed270) will **decrease** coverage by `0.02%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #4427 +/- ##
============================================
- Coverage 53.20% 53.18% -0.02%
Complexity 13 13
============================================
Files 569 569
Lines 31055 31055
Branches 4194 4194
============================================
- Hits 16522 16518 -4
- Misses 12973 12978 +5
+ Partials 1560 1559 -1
```
| [Impacted Files](https://codecov.io/gh/apache/kyuubi/pull/4427?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...apache/kyuubi/engine/JpsApplicationOperation.scala](https://codecov.io/gh/apache/kyuubi/pull/4427?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: |
| [...g/apache/kyuubi/operation/BatchJobSubmission.scala](https://codecov.io/gh/apache/kyuubi/pull/4427?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9vcGVyYXRpb24vQmF0Y2hKb2JTdWJtaXNzaW9uLnNjYWxh) | `75.82% <0.00%> (-1.65%)` | :arrow_down: |
| [...n/scala/org/apache/kyuubi/engine/ProcBuilder.scala](https://codecov.io/gh/apache/kyuubi/pull/4427?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLXNlcnZlci9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9lbmdpbmUvUHJvY0J1aWxkZXIuc2NhbGE=) | `78.39% <0.00%> (-0.62%)` | :arrow_down: |
| [...in/scala/org/apache/kyuubi/config/KyuubiConf.scala](https://codecov.io/gh/apache/kyuubi/pull/4427?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWNvbW1vbi9zcmMvbWFpbi9zY2FsYS9vcmcvYXBhY2hlL2t5dXViaS9jb25maWcvS3l1dWJpQ29uZi5zY2FsYQ==) | `97.30% <0.00%> (-0.07%)` | :arrow_down: |
| [...he/kyuubi/ha/client/etcd/EtcdDiscoveryClient.scala](https://codecov.io/gh/apache/kyuubi/pull/4427?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-a3l1dWJpLWhhL3NyYy9tYWluL3NjYWxhL29yZy9hcGFjaGUva3l1dWJpL2hhL2NsaWVudC9ldGNkL0V0Y2REaXNjb3ZlcnlDbGllbnQuc2NhbGE=) | `69.56% <0.00%> (+0.54%)` | :arrow_up: |
| [...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala](https://codecov.io/gh/apache/kyuubi/pull/4427?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=) | `60.60% <0.00%> (+1.51%)` | :arrow_up: |
: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] yaooqinn commented on a diff in pull request #4427: Simplify assertions in Authz tests in declarative style
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #4427:
URL: https://github.com/apache/kyuubi/pull/4427#discussion_r1119512230
##########
extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/SparkSessionProvider.scala:
##########
@@ -71,23 +75,78 @@ trait SparkSessionProvider {
protected val sql: String => DataFrame = spark.sql
- protected def doAs[T](user: String, f: => T): T = {
+ protected def doAs[T](user: String)(f: => T): T = {
UserGroupInformation.createRemoteUser(user).doAs[T](
new PrivilegedExceptionAction[T] {
override def run(): T = f
})
}
+
+ protected def runSqlAs(user: String)(
Review Comment:
+1
--
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 #4427: Simplify assertions in Authz tests in declarative style
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #4427:
URL: https://github.com/apache/kyuubi/pull/4427#discussion_r1119547740
##########
extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/SparkSessionProvider.scala:
##########
@@ -71,23 +75,78 @@ trait SparkSessionProvider {
protected val sql: String => DataFrame = spark.sql
- protected def doAs[T](user: String, f: => T): T = {
+ protected def doAs[T](user: String)(f: => T): T = {
UserGroupInformation.createRemoteUser(user).doAs[T](
new PrivilegedExceptionAction[T] {
override def run(): T = f
})
}
+
+ protected def runSqlAs(user: String)(
Review Comment:
I have added a `checkAnswer` method and it looks better than this one. Meanwhile, there would be great if we could have a similar one for expected failures, for example.
```scala
def checkError[T <: Throwable : ClassTag](
user: String,
query: String,
expectedErrMsg: String): Unit = {
...
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] bowenliang123 commented on a diff in pull request #4427: Simplify assertions in Authz tests in declarative style
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #4427:
URL: https://github.com/apache/kyuubi/pull/4427#discussion_r1119593028
##########
extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/SparkSessionProvider.scala:
##########
@@ -71,23 +75,78 @@ trait SparkSessionProvider {
protected val sql: String => DataFrame = spark.sql
- protected def doAs[T](user: String, f: => T): T = {
+ protected def doAs[T](user: String)(f: => T): T = {
UserGroupInformation.createRemoteUser(user).doAs[T](
new PrivilegedExceptionAction[T] {
override def run(): T = f
})
}
+
+ protected def runSqlAs(user: String)(
Review Comment:
It's not sure `checkAnswer` is better in some way.
As for this PR, the key points for changes in signatures of `doAs` and `runAs` includes:
- extract user as first param group for `As`, to focus in the user idendity and also to eliminate unhelpfule line break every time
- handy to config the decision points in assertion and and the execution modes
- wrapping repeated exectution template
After all, these methods are used in test cases only. It's good enough to provide well polished signatures for simplication.
##########
extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/SparkSessionProvider.scala:
##########
@@ -71,23 +75,78 @@ trait SparkSessionProvider {
protected val sql: String => DataFrame = spark.sql
- protected def doAs[T](user: String, f: => T): T = {
+ protected def doAs[T](user: String)(f: => T): T = {
UserGroupInformation.createRemoteUser(user).doAs[T](
new PrivilegedExceptionAction[T] {
override def run(): T = f
})
}
+
+ protected def runSqlAs(user: String)(
Review Comment:
It's not sure `checkAnswer` is better in some way.
As for this PR, the key points for changes in signatures of `doAs` and `runAs` includes:
- extract user as first param group for `As`, to focus in the user idendity and also to eliminate unhelpfule line break every time
- handy to config the decision points in assertion and and the execution modes
- wrapping repeated execution templates
After all, these methods are used in test cases only. It's good enough to provide well polished signatures for simplication.
--
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 #4427: Simplify assertions in Authz tests in declarative style
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #4427:
URL: https://github.com/apache/kyuubi/pull/4427#discussion_r1119605302
##########
extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/SparkSessionProvider.scala:
##########
@@ -71,23 +75,78 @@ trait SparkSessionProvider {
protected val sql: String => DataFrame = spark.sql
- protected def doAs[T](user: String, f: => T): T = {
+ protected def doAs[T](user: String)(f: => T): T = {
UserGroupInformation.createRemoteUser(user).doAs[T](
new PrivilegedExceptionAction[T] {
override def run(): T = f
})
}
+
+ protected def runSqlAs(user: String)(
Review Comment:
Please use simple functions for tests, especially in test traits/helpers, etc.
- Complex helpers make the debug experience worse.
- Playing tricks with if-else, etc., make the function itself hard to understand. while you wrote once, thousands of reads may follow.
- Currying is not so that welcome in tests
- etc
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] pan3793 commented on a diff in pull request #4427: Simplify assertions in Authz tests in declarative style
Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #4427:
URL: https://github.com/apache/kyuubi/pull/4427#discussion_r1119509460
##########
extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/SparkSessionProvider.scala:
##########
@@ -71,23 +75,78 @@ trait SparkSessionProvider {
protected val sql: String => DataFrame = spark.sql
- protected def doAs[T](user: String, f: => T): T = {
+ protected def doAs[T](user: String)(f: => T): T = {
UserGroupInformation.createRemoteUser(user).doAs[T](
new PrivilegedExceptionAction[T] {
override def run(): T = f
})
}
+
+ protected def runSqlAs(user: String)(
Review Comment:
I think it's kind of over-engineering, which increases the onboarding costs for new developer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] bowenliang123 commented on a diff in pull request #4427: Simplify assertions in Authz tests in declarative style
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #4427:
URL: https://github.com/apache/kyuubi/pull/4427#discussion_r1119593028
##########
extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/SparkSessionProvider.scala:
##########
@@ -71,23 +75,78 @@ trait SparkSessionProvider {
protected val sql: String => DataFrame = spark.sql
- protected def doAs[T](user: String, f: => T): T = {
+ protected def doAs[T](user: String)(f: => T): T = {
UserGroupInformation.createRemoteUser(user).doAs[T](
new PrivilegedExceptionAction[T] {
override def run(): T = f
})
}
+
+ protected def runSqlAs(user: String)(
Review Comment:
It's not sure `checkAnswer` is better in some way.
As for this PR, the key points for changes in signatures of `doAs` and `runAs` includes:
- extract user as first param group for `As`, to focus in the user idendity and also to eliminate unhelpfule line break every time
- handy to config the decision points in assertion and and the execution modes
- wrapping repeated execution templates
After all, these methods are used in test cases only. It's good enough to provide well polished signatures for implications. And they do help reduce over 100 lines and the same amount of distraction for Ranger ut developers.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] bowenliang123 commented on a diff in pull request #4427: Simplify assertions in Authz tests in declarative style
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #4427:
URL: https://github.com/apache/kyuubi/pull/4427#discussion_r1119514967
##########
extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/SparkSessionProvider.scala:
##########
@@ -71,23 +75,78 @@ trait SparkSessionProvider {
protected val sql: String => DataFrame = spark.sql
- protected def doAs[T](user: String, f: => T): T = {
+ protected def doAs[T](user: String)(f: => T): T = {
UserGroupInformation.createRemoteUser(user).doAs[T](
new PrivilegedExceptionAction[T] {
override def run(): T = f
})
}
+
+ protected def runSqlAs(user: String)(
Review Comment:
It's not forcing user/developer to use `runSqlAs`. `runSqlAs` and `runSqlAsWithAccessException` are together providing a concentrative and declarative way in most general scenarios to decide running what SQL in which user with asserting what messages. `doAs` is still good to be used with them, and remained as you see.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] bowenliang123 commented on pull request #4427: Simplify assertions in Authz tests in declarative style
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #4427:
URL: https://github.com/apache/kyuubi/pull/4427#issuecomment-1447652583
Closing this PR as it's in the immature status of early stage. Not too many common benefits.
--
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 #4427: Simplify assertions in Authz tests in declarative style
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #4427:
URL: https://github.com/apache/kyuubi/pull/4427#discussion_r1119605302
##########
extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/SparkSessionProvider.scala:
##########
@@ -71,23 +75,78 @@ trait SparkSessionProvider {
protected val sql: String => DataFrame = spark.sql
- protected def doAs[T](user: String, f: => T): T = {
+ protected def doAs[T](user: String)(f: => T): T = {
UserGroupInformation.createRemoteUser(user).doAs[T](
new PrivilegedExceptionAction[T] {
override def run(): T = f
})
}
+
+ protected def runSqlAs(user: String)(
Review Comment:
Please use simple functions for tests, especially in test traits/helpers, etc.
- Complex helpers make the debug experience worse. the failure point may be delayed show at this complicated method. With a long error stack, it cost more time to trace back
- Playing tricks with if-else, etc., make the function itself hard to understand. while you wrote once, thousands of reads may follow.
- Currying is not so that welcome in tests
- etc
--
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 pull request #4427: Simplify assertions in Authz tests in declarative style
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on PR #4427:
URL: https://github.com/apache/kyuubi/pull/4427#issuecomment-1447642887
Discussions for PR are welcome, people are free to express their agreements/disagreements.
BTW, better to comment with a reason while closing a PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org
[GitHub] [kyuubi] bowenliang123 commented on a diff in pull request #4427: Simplify assertions in Authz tests in declarative style
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #4427:
URL: https://github.com/apache/kyuubi/pull/4427#discussion_r1119514967
##########
extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/SparkSessionProvider.scala:
##########
@@ -71,23 +75,78 @@ trait SparkSessionProvider {
protected val sql: String => DataFrame = spark.sql
- protected def doAs[T](user: String, f: => T): T = {
+ protected def doAs[T](user: String)(f: => T): T = {
UserGroupInformation.createRemoteUser(user).doAs[T](
new PrivilegedExceptionAction[T] {
override def run(): T = f
})
}
+
+ protected def runSqlAs(user: String)(
Review Comment:
It's not forcing user/developer to use `runSqlAs`. `runSqlAs` and `runSqlAsWithAccessException` are together providing a concentrative and declarative way in most general scenarios to decide running what SQL in which user with asserting what messages. `doAs` is still good to be used with them, and remained as you see.
It helps developer to make decisions rather than holding execution details and be forced to repeating duplicate wrappings, and it's all in their favor.
--
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