You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@kyuubi.apache.org by "packyan (via GitHub)" <gi...@apache.org> on 2023/04/16 10:18:15 UTC
[GitHub] [kyuubi] packyan opened a new pull request, #4717: [Improvement] [AUTHZ] Spec json files should be auto generated in each build
packyan opened a new pull request, #4717:
URL: https://github.com/apache/kyuubi/pull/4717
<!--
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.
-->
to close #4715
### _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] packyan commented on pull request #4717: [KYUUBI 4715] [Improvement] [AUTHZ] Check Spec json files in UT
Posted by "packyan (via GitHub)" <gi...@apache.org>.
packyan commented on PR #4717:
URL: https://github.com/apache/kyuubi/pull/4717#issuecomment-1510645930
> Hi, the spec json file should not be regenerated in each build, by design. But yes, it should be checked during ci builds.
Yes, I will add a UT to check generated golden files.
--
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] packyan commented on pull request #4717: [AUTHZ] Check Authz plugin's spec json files in UT
Posted by "packyan (via GitHub)" <gi...@apache.org>.
packyan commented on PR #4717:
URL: https://github.com/apache/kyuubi/pull/4717#issuecomment-1514833096
> Would you like to remove redundant changes in generator and policy json file of #4716 which is already merged ? @packyan
Done, wait for CI finished.
--
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 #4717: [KYUUBI 4715] [Improvement] [AUTHZ] Spec json files should be auto generated in each build
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #4717:
URL: https://github.com/apache/kyuubi/pull/4717#issuecomment-1510383773
## [Codecov](https://codecov.io/gh/apache/kyuubi/pull/4717?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 [#4717](https://codecov.io/gh/apache/kyuubi/pull/4717?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bfe751a) into [master](https://codecov.io/gh/apache/kyuubi/commit/db46b5b320ffc3e58f84a0c3bb0d113783b9612b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (db46b5b) will **decrease** coverage by `0.09%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #4717 +/- ##
============================================
- Coverage 58.11% 58.02% -0.09%
Complexity 13 13
============================================
Files 580 580
Lines 32232 32232
Branches 4302 4302
============================================
- Hits 18731 18702 -29
- Misses 11713 11736 +23
- Partials 1788 1794 +6
```
[see 12 files with indirect coverage changes](https://codecov.io/gh/apache/kyuubi/pull/4717/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] bowenliang123 commented on pull request #4717: [KYUUBI 4715] [Improvement] [AUTHZ] Check Spec json files in UT
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #4717:
URL: https://github.com/apache/kyuubi/pull/4717#issuecomment-1510999413
Alright, I mean change it from executing the main method to running it in test suites, for both validation in testing and regeneration, just to be clear.
--
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] packyan commented on pull request #4717: [KYUUBI 4715] [Improvement] [AUTHZ] Check Spec json files in UT
Posted by "packyan (via GitHub)" <gi...@apache.org>.
packyan commented on PR #4717:
URL: https://github.com/apache/kyuubi/pull/4717#issuecomment-1511033800
> Alright, I mean change it from executing the main method to running it in test suites, for both validation in testing and regeneration, just to be clear.
understood
--
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 #4717: [AUTHZ] Check Authz plugin's spec json files in UT
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on code in PR #4717:
URL: https://github.com/apache/kyuubi/pull/4717#discussion_r1171457642
##########
extensions/spark/kyuubi-spark-authz/src/test/scala/org/apache/kyuubi/plugin/spark/authz/gen/JsonSpecFileGenerator.scala:
##########
@@ -18,37 +18,67 @@
package org.apache.kyuubi.plugin.spark.authz.gen
import java.nio.charset.StandardCharsets
-import java.nio.file.{Files, Paths}
+import java.nio.file.{Files, Paths, StandardOpenOption}
+
+import org.apache.commons.io.FileUtils
+//scalastyle:off
+import org.scalatest.funsuite.AnyFunSuite
import org.apache.kyuubi.plugin.spark.authz.serde.{mapper, CommandSpec}
/**
* Generates the default command specs to src/main/resources dir.
*
- * Usage:
- * mvn scala:run -DmainClass=this class -pl :kyuubi-spark-authz_2.12
+ * To run the test suite:
+ * build/mvn clean test -Pgen-policy -pl :kyuubi-spark-authz_2.12 -Dtest=none
+ * -DwildcardSuites=org.apache.kyuubi.plugin.spark.authz.gen.JsonSpecFileGenerator
+ *
+ * To regenerate the ranger policy file:
+ * KYUUBI_UPDATE=1 build/mvn clean test -Pgen-policy -pl :kyuubi-spark-authz_2.12 -Dtest=none
+ * -DwildcardSuites=org.apache.kyuubi.plugin.spark.authz.gen.JsonSpecFileGenerator
*/
-object JsonSpecFileGenerator {
- def main(args: Array[String]): Unit = {
+class JsonSpecFileGenerator extends AnyFunSuite {
+ // scalastyle:on
+ test("check spec json files") {
writeCommandSpecJson("database", DatabaseCommands.data)
writeCommandSpecJson("table", TableCommands.data ++ IcebergCommands.data)
writeCommandSpecJson("function", FunctionCommands.data)
writeCommandSpecJson("scan", Scans.data)
}
- def writeCommandSpecJson[T <: CommandSpec](commandType: String, specArr: Array[T]): Unit = {
+ def writeCommandSpecJson[T <: CommandSpec](
+ commandType: String,
+ specArr: Array[T]): Unit = {
val pluginHome = getClass.getProtectionDomain.getCodeSource.getLocation.getPath
.split("target").head
val filename = s"${commandType}_command_spec.json"
- val writer = {
- val p = Paths.get(pluginHome, "src", "main", "resources", filename)
- Files.newBufferedWriter(p, StandardCharsets.UTF_8)
+ val filePath = Paths.get(pluginHome, "src", "main", "resources", filename)
+
+ val generatedStr = mapper.writerWithDefaultPrettyPrinter()
+ .writeValueAsString(specArr.sortBy(_.classname))
+
+ if (sys.env.get("KYUUBI_UPDATE").contains("1")) {
+ // scalastyle:off println
+ println(s"writing ${specArr.length} specs to $filename")
+ // scalastyle:on println
+ Files.write(
+ filePath,
+ generatedStr.getBytes(StandardCharsets.UTF_8),
+ StandardOpenOption.CREATE,
+ StandardOpenOption.TRUNCATE_EXISTING)
+ } else {
+ val existedFileContent =
+ FileUtils.readFileToString(filePath.toFile, StandardCharsets.UTF_8)
+ // scalastyle:off println
+ println(s"checking ${specArr.length} specs in $filename")
Review Comment:
Remove this unnecessary print.
--
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 #4717: [AUTHZ] Check Authz plugin's spec json files in UT
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 closed pull request #4717: [AUTHZ] Check Authz plugin's spec json files in UT
URL: https://github.com/apache/kyuubi/pull/4717
--
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 #4717: [KYUUBI 4715] [Improvement] [AUTHZ] Spec json files should be auto generated in each build
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #4717:
URL: https://github.com/apache/kyuubi/pull/4717#issuecomment-1510563777
Hi, the spec json file should not be regenerated in each build, by design. But yes, it should be checked during ci builds.
--
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 #4717: [KYUUBI 4715] Check Authz plugin's spec json files in UT
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #4717:
URL: https://github.com/apache/kyuubi/pull/4717#issuecomment-1514474877
Would you like to remove redundant changes in generator and policy json file of https://github.com/apache/kyuubi/pull/4716 which is already merged ? @packyan
--
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 #4717: [AUTHZ] Check Authz plugin's spec json files in UT
Posted by "bowenliang123 (via GitHub)" <gi...@apache.org>.
bowenliang123 commented on PR #4717:
URL: https://github.com/apache/kyuubi/pull/4717#issuecomment-1515593850
Thanks for your continuous contribution. Merged to master.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@kyuubi.apache.org
For additional commands, e-mail: notifications-help@kyuubi.apache.org