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 2021/10/14 03:40:38 UTC
[GitHub] [incubator-kyuubi] TyrealBM opened a new pull request #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
TyrealBM opened a new pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232
feature: kyuubi.engine.share.level.subdomain can be more tolerant
now end-user can configure kyuubi.engine.share.level.subdomain by zookeeper valid path
I refer to the zookeeper doc http://zookeeper.apache.org/doc/r3.7.0/zookeeperProgrammers.html#:~:text=ZooKeeper%20Basic%20Operations.-,The%20ZooKeeper%20Data%20Model,-ZooKeeper%20has%20a
and source code:
https://github.com/apache/zookeeper/blob/master/zookeeper-server/src/main/java/org/apache/zookeeper/common/PathUtils.java#:~:text=public%20static%20void-,validatePath,-(String%20path)%20throws
--
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 #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#discussion_r728730234
##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -725,15 +725,24 @@ object KyuubiConf {
"must be [1, 14] length alphabet string, e.g. 'abc', 'apache'")
.createOptional
- val ENGINE_SHARE_LEVEL_SUBDOMAIN: ConfigEntry[Option[String]] =
+ // [ZooKeeper Data Model]
+ // (http://zookeeper.apache.org/doc/r3.7.0/zookeeperProgrammers.html#ch_zkDataModel)
+ private val validEngineSubdomain: Pattern = ("(?!^[\u002e]{1,2}$)" +
+ "(^[\u0020-\u002e\u0030-\u007e\u00a0-\ud7ff\uf900-\uffef]{1,}$)").r.pattern
+
+ val ENGINE_SHARE_LEVEL_SUBDOMAIN: OptionalConfigEntry[String] =
buildConf("engine.share.level.subdomain")
.doc("Allow end-users to create a subdomain for the share level of an engine. A" +
- " subdomain is a case-insensitive string values in `^[a-zA-Z_-]{1,14}$` form." +
+ " subdomain is a case-insensitive string values that must be a valid zookeeper sub path." +
" For example, for `USER` share level, an end-user can share a certain engine within" +
" a subdomain, not for all of its clients. End-users are free to create multiple" +
" engines in the `USER` share level")
.version("1.4.0")
- .fallbackConf(ENGINE_SHARE_LEVEL_SUB_DOMAIN)
+ .stringConf
+ .transform(_.toLowerCase(Locale.ROOT))
+ .checkValue(validEngineSubdomain.matcher(_).matches(),
+ "must be valid zookeeper sub path."
+ ).createOptional
Review comment:
ENGINE_SHARE_LEVEL_SUB_DOMAIN and ENGINE_SHARE_LEVEL_SUBDOMAIN are the same, and the former is the upstream one, the `checkValue` and `transform` should be able to be inherited by the latter. So we can only modify ```SUB_DOMAIN```
--
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] TyrealBM commented on a change in pull request #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
TyrealBM commented on a change in pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#discussion_r728772734
##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -725,15 +725,24 @@ object KyuubiConf {
"must be [1, 14] length alphabet string, e.g. 'abc', 'apache'")
.createOptional
- val ENGINE_SHARE_LEVEL_SUBDOMAIN: ConfigEntry[Option[String]] =
+ // [ZooKeeper Data Model]
+ // (http://zookeeper.apache.org/doc/r3.7.0/zookeeperProgrammers.html#ch_zkDataModel)
+ private val validEngineSubdomain: Pattern = ("(?!^[\u002e]{1,2}$)" +
+ "(^[\u0020-\u002e\u0030-\u007e\u00a0-\ud7ff\uf900-\uffef]{1,}$)").r.pattern
+
+ val ENGINE_SHARE_LEVEL_SUBDOMAIN: OptionalConfigEntry[String] =
buildConf("engine.share.level.subdomain")
.doc("Allow end-users to create a subdomain for the share level of an engine. A" +
- " subdomain is a case-insensitive string values in `^[a-zA-Z_-]{1,14}$` form." +
+ " subdomain is a case-insensitive string values that must be a valid zookeeper sub path." +
" For example, for `USER` share level, an end-user can share a certain engine within" +
" a subdomain, not for all of its clients. End-users are free to create multiple" +
" engines in the `USER` share level")
.version("1.4.0")
- .fallbackConf(ENGINE_SHARE_LEVEL_SUB_DOMAIN)
+ .stringConf
+ .transform(_.toLowerCase(Locale.ROOT))
+ .checkValue(validEngineSubdomain.matcher(_).matches(),
+ "must be valid zookeeper sub path."
+ ).createOptional
Review comment:
i moved these to ENGINE_SHARE_LEVEL_SUB_DOMAIN
--
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] TyrealBM commented on pull request #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
TyrealBM commented on pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#issuecomment-944859507
new invalid sub domains in the old tests
--
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 pull request #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#issuecomment-945310266
FYI, `guoqing.yang <gu...@advance.ai>` can not be recognized by Github, it's better to add the email to you Github account
--
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] TyrealBM commented on a change in pull request #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
TyrealBM commented on a change in pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#discussion_r728721776
##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -725,15 +725,24 @@ object KyuubiConf {
"must be [1, 14] length alphabet string, e.g. 'abc', 'apache'")
.createOptional
- val ENGINE_SHARE_LEVEL_SUBDOMAIN: ConfigEntry[Option[String]] =
+ // [ZooKeeper Data Model]
+ // (http://zookeeper.apache.org/doc/r3.7.0/zookeeperProgrammers.html#ch_zkDataModel)
+ private val validEngineSubdomain: Pattern = ("(?!^[\u002e]{1,2}$)" +
+ "(^[\u0020-\u002e\u0030-\u007e\u00a0-\ud7ff\uf900-\uffef]{1,}$)").r.pattern
+
+ val ENGINE_SHARE_LEVEL_SUBDOMAIN: OptionalConfigEntry[String] =
buildConf("engine.share.level.subdomain")
.doc("Allow end-users to create a subdomain for the share level of an engine. A" +
- " subdomain is a case-insensitive string values in `^[a-zA-Z_-]{1,14}$` form." +
+ " subdomain is a case-insensitive string values that must be a valid zookeeper sub path." +
" For example, for `USER` share level, an end-user can share a certain engine within" +
" a subdomain, not for all of its clients. End-users are free to create multiple" +
" engines in the `USER` share level")
.version("1.4.0")
- .fallbackConf(ENGINE_SHARE_LEVEL_SUB_DOMAIN)
+ .stringConf
+ .transform(_.toLowerCase(Locale.ROOT))
+ .checkValue(validEngineSubdomain.matcher(_).matches(),
+ "must be valid zookeeper sub path."
+ ).createOptional
Review comment:
keep `private val validEngineSubDomain: Pattern = "^[a-zA-Z_-]{1,14}$".r.pattern` or delete?
--
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 pull request #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#issuecomment-945309427
thanks @TyrealBM for your first 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: commits-unsubscribe@kyuubi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-kyuubi] TyrealBM commented on pull request #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
TyrealBM commented on pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#issuecomment-943003045
@yaooqinn Thank you for your review, I listened to your opinion and i have fixed it
--
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] TyrealBM commented on a change in pull request #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
TyrealBM commented on a change in pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#discussion_r728833587
##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -725,15 +725,24 @@ object KyuubiConf {
"must be [1, 14] length alphabet string, e.g. 'abc', 'apache'")
.createOptional
- val ENGINE_SHARE_LEVEL_SUBDOMAIN: ConfigEntry[Option[String]] =
+ // [ZooKeeper Data Model]
+ // (http://zookeeper.apache.org/doc/r3.7.0/zookeeperProgrammers.html#ch_zkDataModel)
+ private val validEngineSubdomain: Pattern = ("(?!^[\u002e]{1,2}$)" +
+ "(^[\u0020-\u002e\u0030-\u007e\u00a0-\ud7ff\uf900-\uffef]{1,}$)").r.pattern
+
+ val ENGINE_SHARE_LEVEL_SUBDOMAIN: OptionalConfigEntry[String] =
buildConf("engine.share.level.subdomain")
.doc("Allow end-users to create a subdomain for the share level of an engine. A" +
- " subdomain is a case-insensitive string values in `^[a-zA-Z_-]{1,14}$` form." +
+ " subdomain is a case-insensitive string values that must be a valid zookeeper sub path." +
" For example, for `USER` share level, an end-user can share a certain engine within" +
" a subdomain, not for all of its clients. End-users are free to create multiple" +
" engines in the `USER` share level")
.version("1.4.0")
- .fallbackConf(ENGINE_SHARE_LEVEL_SUB_DOMAIN)
+ .stringConf
+ .transform(_.toLowerCase(Locale.ROOT))
+ .checkValue(validEngineSubdomain.matcher(_).matches(),
+ "must be valid zookeeper sub path."
+ ).createOptional
Review comment:
code style fixed
--
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 #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#discussion_r728647478
##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -725,15 +725,23 @@ object KyuubiConf {
"must be [1, 14] length alphabet string, e.g. 'abc', 'apache'")
.createOptional
- val ENGINE_SHARE_LEVEL_SUBDOMAIN: ConfigEntry[Option[String]] =
+ private val validEngineSubdomain: Pattern = ("(?!^[\u002e]{1,2}$)" +
Review comment:
Add a comment here to point to zookeeper documentation？
--
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 #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#discussion_r728677760
##########
File path: kyuubi-common/src/test/scala/org/apache/kyuubi/config/KyuubiConfSuite.scala
##########
@@ -150,4 +150,25 @@ class KyuubiConfSuite extends KyuubiFunSuite {
val e1 = intercept[IllegalArgumentException](kyuubiConf.get(OPERATION_QUERY_TIMEOUT))
assert(e1.getMessage.contains("must >= 1s if set"))
}
+
+ test("kyuubi conf engine.share.level.subdomain valid path test") {
+ val kyuubiConf = KyuubiConf()
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, ".")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "..")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "/")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "/tmp/")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "tmp/")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "/tmp")
Review comment:
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: 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 #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#discussion_r728647635
##########
File path: kyuubi-common/src/test/scala/org/apache/kyuubi/config/KyuubiConfSuite.scala
##########
@@ -150,4 +150,25 @@ class KyuubiConfSuite extends KyuubiFunSuite {
val e1 = intercept[IllegalArgumentException](kyuubiConf.get(OPERATION_QUERY_TIMEOUT))
assert(e1.getMessage.contains("must >= 1s if set"))
}
+
+ test(KyuubiConf.ENGINE_SHARE_LEVEL_SUBDOMAIN.key) {
Review comment:
we can use string literal here for the test name
--
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] TyrealBM commented on a change in pull request #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
TyrealBM commented on a change in pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#discussion_r728676721
##########
File path: kyuubi-common/src/test/scala/org/apache/kyuubi/config/KyuubiConfSuite.scala
##########
@@ -150,4 +150,25 @@ class KyuubiConfSuite extends KyuubiFunSuite {
val e1 = intercept[IllegalArgumentException](kyuubiConf.get(OPERATION_QUERY_TIMEOUT))
assert(e1.getMessage.contains("must >= 1s if set"))
}
+
+ test("kyuubi conf engine.share.level.subdomain valid path test") {
+ val kyuubiConf = KyuubiConf()
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, ".")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "..")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "/")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "/tmp/")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "tmp/")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "/tmp")
Review comment:
Ok! It has been added to the unit test
--
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 #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#discussion_r728646861
##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -725,15 +725,23 @@ object KyuubiConf {
"must be [1, 14] length alphabet string, e.g. 'abc', 'apache'")
.createOptional
- val ENGINE_SHARE_LEVEL_SUBDOMAIN: ConfigEntry[Option[String]] =
+ private val validEngineSubdomain: Pattern = ("(?!^[\u002e]{1,2}$)" +
+ "(^[\u0020-\u002e\u0030-\u007e\u00a0-\ud7ff\uf900-\uffef]{1,}$)").r.pattern
+
+ val ENGINE_SHARE_LEVEL_SUBDOMAIN: OptionalConfigEntry[String] =
buildConf("engine.share.level.subdomain")
.doc("Allow end-users to create a subdomain for the share level of an engine. A" +
- " subdomain is a case-insensitive string values in `^[a-zA-Z_-]{1,14}$` form." +
+ " subdomain is a case-insensitive string values in `(?!^[\u002e]{1,2}$)" +
+ "(^[\u0020-\u002e\u0030-\u007e\u00a0-\ud7ff\uf900-\uffef]{1,}$)` form." +
Review comment:
can it be more human-readable？
--
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 #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#discussion_r728671794
##########
File path: kyuubi-common/src/test/scala/org/apache/kyuubi/config/KyuubiConfSuite.scala
##########
@@ -150,4 +150,25 @@ class KyuubiConfSuite extends KyuubiFunSuite {
val e1 = intercept[IllegalArgumentException](kyuubiConf.get(OPERATION_QUERY_TIMEOUT))
assert(e1.getMessage.contains("must >= 1s if set"))
}
+
+ test("kyuubi conf engine.share.level.subdomain valid path test") {
+ val kyuubiConf = KyuubiConf()
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, ".")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "..")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "/")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "/tmp/")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "tmp/")
+ assertThrows[IllegalArgumentException](kyuubiConf.get(ENGINE_SHARE_LEVEL_SUBDOMAIN))
+ kyuubiConf.set(ENGINE_SHARE_LEVEL_SUBDOMAIN, "/tmp")
Review comment:
can we also add a test for `"abc/efg"`, we shall also avoid this case
--
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 closed pull request #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
yaooqinn closed pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232
--
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] ulysses-you commented on pull request #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
ulysses-you commented on pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#issuecomment-945319060
thank you @TyrealBM for the contribution. `Kyubbi` is wrong, it's `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: 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 #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#discussion_r728700343
##########
File path: kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala
##########
@@ -725,15 +725,24 @@ object KyuubiConf {
"must be [1, 14] length alphabet string, e.g. 'abc', 'apache'")
.createOptional
- val ENGINE_SHARE_LEVEL_SUBDOMAIN: ConfigEntry[Option[String]] =
+ // [ZooKeeper Data Model]
+ // (http://zookeeper.apache.org/doc/r3.7.0/zookeeperProgrammers.html#ch_zkDataModel)
+ private val validEngineSubdomain: Pattern = ("(?!^[\u002e]{1,2}$)" +
+ "(^[\u0020-\u002e\u0030-\u007e\u00a0-\ud7ff\uf900-\uffef]{1,}$)").r.pattern
+
+ val ENGINE_SHARE_LEVEL_SUBDOMAIN: OptionalConfigEntry[String] =
buildConf("engine.share.level.subdomain")
.doc("Allow end-users to create a subdomain for the share level of an engine. A" +
- " subdomain is a case-insensitive string values in `^[a-zA-Z_-]{1,14}$` form." +
+ " subdomain is a case-insensitive string values that must be a valid zookeeper sub path." +
" For example, for `USER` share level, an end-user can share a certain engine within" +
" a subdomain, not for all of its clients. End-users are free to create multiple" +
" engines in the `USER` share level")
.version("1.4.0")
- .fallbackConf(ENGINE_SHARE_LEVEL_SUB_DOMAIN)
+ .stringConf
+ .transform(_.toLowerCase(Locale.ROOT))
+ .checkValue(validEngineSubdomain.matcher(_).matches(),
+ "must be valid zookeeper sub path."
+ ).createOptional
Review comment:
we need to move these to ENGINE_SHARE_LEVEL_SUB_DOMAIN
--
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 pull request #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#issuecomment-943079832
can you fix the style
```
error file=/home/runner/work/incubator-kyuubi/incubator-kyuubi/kyuubi-common/src/main/scala/org/apache/kyuubi/config/KyuubiConf.scala message=nonascii.message line=731 column=4
```
--
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] TyrealBM commented on pull request #1232: [KYUUBI #1209] kyuubi.engine.share.level.subdomain can be more tolerant
Posted by GitBox <gi...@apache.org>.
TyrealBM commented on pull request #1232:
URL: https://github.com/apache/incubator-kyuubi/pull/1232#issuecomment-945313572
tnanks @yaooqinn .It's really exciting.I'll pay more attention to the Kyubbi community later
--
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