[GitHub] [spark] kevincmchen commented on a change in pull request #33746: [SPARK-36514][SQL] Support to set the meta conf in HiveMetastoreClient
kevincmchen commented on a change in pull request #33746:
URL: https://github.com/apache/spark/pull/33746#discussion_r689373076
##
File path:
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##
@@ -265,6 +265,10 @@ private[hive] class HiveClientImpl(
clientLoader.cachedHive.asInstanceOf[Hive]
} else {
val c = getHive(conf)
+ val metaConfPrefix =
SQLConf.get.getConf(HiveUtils.HIVE_METASTORE_METACONF_PREFIX)
+ conf.getAllProperties.asScala
+.filterKeys(_.startsWith(metaConfPrefix))
+.foreach{ case(key, value) =>
c.setMetaConf(key.substring(metaConfPrefix.length), value)}
Review comment:
i have add the description, pls check
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] kevincmchen commented on a change in pull request #33746: [SPARK-36514][SQL] Support to set the meta conf in HiveMetastoreClient
kevincmchen commented on a change in pull request #33746:
URL: https://github.com/apache/spark/pull/33746#discussion_r689373891
##
File path:
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##
@@ -265,6 +265,10 @@ private[hive] class HiveClientImpl(
clientLoader.cachedHive.asInstanceOf[Hive]
} else {
val c = getHive(conf)
+ val metaConfPrefix =
SQLConf.get.getConf(HiveUtils.HIVE_METASTORE_METACONF_PREFIX)
+ conf.getAllProperties.asScala
+.filterKeys(_.startsWith(metaConfPrefix))
+.foreach{ case(key, value) =>
c.setMetaConf(key.substring(metaConfPrefix.length), value)}
Review comment:
in our case, we pass tenant info to an external multi-tenant metasotre
by utilizing the meta conf, such as tenant name, auth info 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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] kevincmchen commented on a change in pull request #33746: [SPARK-36514][SQL] Support to set the meta conf in HiveMetastoreClient
kevincmchen commented on a change in pull request #33746:
URL: https://github.com/apache/spark/pull/33746#discussion_r689380411
##
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala
##
@@ -118,6 +118,13 @@ private[spark] object HiveUtils extends Logging {
.toSequence
.createWithDefault(Nil)
+ val HIVE_METASTORE_METACONF_PREFIX =
buildStaticConf("spark.sql.hive.metastore.metaconf.prefix")
+.doc("the prefix of the meta conf in HiveMetastoreClient, which is used to
extract " +
Review comment:
I want to update the description to "prefix of the meta conf that could
be used to set the HiveMetastoreClient.", do you think it's better?
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] kevincmchen commented on a change in pull request #33746: [SPARK-36514][SQL] Support to set the meta conf in HiveMetastoreClient
kevincmchen commented on a change in pull request #33746:
URL: https://github.com/apache/spark/pull/33746#discussion_r689373076
##
File path:
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##
@@ -265,6 +265,10 @@ private[hive] class HiveClientImpl(
clientLoader.cachedHive.asInstanceOf[Hive]
} else {
val c = getHive(conf)
+ val metaConfPrefix =
SQLConf.get.getConf(HiveUtils.HIVE_METASTORE_METACONF_PREFIX)
+ conf.getAllProperties.asScala
+.filterKeys(_.startsWith(metaConfPrefix))
+.foreach{ case(key, value) =>
c.setMetaConf(key.substring(metaConfPrefix.length), value)}
Review comment:
i have added the description, pls check
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] kevincmchen commented on a change in pull request #33746: [SPARK-36514][SQL] Support to set the meta conf in HiveMetastoreClient
kevincmchen commented on a change in pull request #33746:
URL: https://github.com/apache/spark/pull/33746#discussion_r689380411
##
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala
##
@@ -118,6 +118,13 @@ private[spark] object HiveUtils extends Logging {
.toSequence
.createWithDefault(Nil)
+ val HIVE_METASTORE_METACONF_PREFIX =
buildStaticConf("spark.sql.hive.metastore.metaconf.prefix")
+.doc("the prefix of the meta conf in HiveMetastoreClient, which is used to
extract " +
Review comment:
I want to update the description to "prefix of the meta conf that could
be used to set the HiveMetastoreClient.", do you think it's better?
@dongjoon-hyun
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] kevincmchen commented on a change in pull request #33746: [SPARK-36514][SQL] Support to set the meta conf in HiveMetastoreClient
kevincmchen commented on a change in pull request #33746:
URL: https://github.com/apache/spark/pull/33746#discussion_r689373891
##
File path:
sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala
##
@@ -265,6 +265,10 @@ private[hive] class HiveClientImpl(
clientLoader.cachedHive.asInstanceOf[Hive]
} else {
val c = getHive(conf)
+ val metaConfPrefix =
SQLConf.get.getConf(HiveUtils.HIVE_METASTORE_METACONF_PREFIX)
+ conf.getAllProperties.asScala
+.filterKeys(_.startsWith(metaConfPrefix))
+.foreach{ case(key, value) =>
c.setMetaConf(key.substring(metaConfPrefix.length), value)}
Review comment:
in our case, we pass tenant info to an external multi-tenant metasotre
by utilizing the meta conf, such as tenant name, auth info etc. @dongjoon-hyun
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] kevincmchen commented on a change in pull request #33746: [SPARK-36514][SQL] Support to set the meta conf in HiveMetastoreClient
kevincmchen commented on a change in pull request #33746:
URL: https://github.com/apache/spark/pull/33746#discussion_r689380411
##
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala
##
@@ -118,6 +118,13 @@ private[spark] object HiveUtils extends Logging {
.toSequence
.createWithDefault(Nil)
+ val HIVE_METASTORE_METACONF_PREFIX =
buildStaticConf("spark.sql.hive.metastore.metaconf.prefix")
+.doc("the prefix of the meta conf in HiveMetastoreClient, which is used to
extract " +
Review comment:
I want to update the description to "prefix of the meta conf that could
be used to set the HiveMetastoreClient.", do you think it's better?
@dongjoon-hyun
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] kevincmchen commented on a change in pull request #33746: [SPARK-36514][SQL] Support to set the meta conf in HiveMetastoreClient
kevincmchen commented on a change in pull request #33746:
URL: https://github.com/apache/spark/pull/33746#discussion_r689492375
##
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala
##
@@ -118,6 +118,13 @@ private[spark] object HiveUtils extends Logging {
.toSequence
.createWithDefault(Nil)
+ val HIVE_METASTORE_METACONF_PREFIX =
buildStaticConf("spark.sql.hive.metastore.metaconf.prefix")
+.doc("the prefix of the meta conf in HiveMetastoreClient, which is used to
extract " +
Review comment:
> This sounds misleading because this PR calls `c.setMetaConf` instead
of extracting. Can we use more direct terms to describe what this conf aims?
cause we couldn't get the meta conf directly and the prefix is used to
extract meta conf from HiveConf.
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] kevincmchen commented on a change in pull request #33746: [SPARK-36514][SQL] Support to set the meta conf in HiveMetastoreClient
kevincmchen commented on a change in pull request #33746:
URL: https://github.com/apache/spark/pull/33746#discussion_r689492375
##
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala
##
@@ -118,6 +118,13 @@ private[spark] object HiveUtils extends Logging {
.toSequence
.createWithDefault(Nil)
+ val HIVE_METASTORE_METACONF_PREFIX =
buildStaticConf("spark.sql.hive.metastore.metaconf.prefix")
+.doc("the prefix of the meta conf in HiveMetastoreClient, which is used to
extract " +
Review comment:
> This sounds misleading because this PR calls `c.setMetaConf` instead
of extracting. Can we use more direct terms to describe what this conf aims?
1. the prefix is used to extract meta conf from HiveConf, cause we couldn't
get the meta conf directly.
2. we should get the meta conf before set 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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] kevincmchen commented on a change in pull request #33746: [SPARK-36514][SQL] Support to set the meta conf in HiveMetastoreClient
kevincmchen commented on a change in pull request #33746:
URL: https://github.com/apache/spark/pull/33746#discussion_r689524588
##
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala
##
@@ -118,6 +118,13 @@ private[spark] object HiveUtils extends Logging {
.toSequence
.createWithDefault(Nil)
+ val HIVE_METASTORE_METACONF_PREFIX =
buildStaticConf("spark.sql.hive.metastore.metaconf.prefix")
+.doc("the prefix of the meta conf in HiveMetastoreClient, which is used to
extract " +
Review comment:
I will add detailed description for the configuration
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] kevincmchen commented on a change in pull request #33746: [SPARK-36514][SQL] Support to set the meta conf in HiveMetastoreClient
kevincmchen commented on a change in pull request #33746:
URL: https://github.com/apache/spark/pull/33746#discussion_r690018846
##
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala
##
@@ -118,6 +118,13 @@ private[spark] object HiveUtils extends Logging {
.toSequence
.createWithDefault(Nil)
+ val HIVE_METASTORE_METACONF_PREFIX =
buildStaticConf("spark.sql.hive.metastore.metaconf.prefix")
+.doc("the prefix of the meta conf in HiveMetastoreClient, which is used to
extract " +
Review comment:
ok, i add a example about how to use the additional conf in this PR.
pls check
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] kevincmchen commented on a change in pull request #33746: [SPARK-36514][SQL] Support to set the meta conf in HiveMetastoreClient
kevincmchen commented on a change in pull request #33746:
URL: https://github.com/apache/spark/pull/33746#discussion_r690018846
##
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala
##
@@ -118,6 +118,13 @@ private[spark] object HiveUtils extends Logging {
.toSequence
.createWithDefault(Nil)
+ val HIVE_METASTORE_METACONF_PREFIX =
buildStaticConf("spark.sql.hive.metastore.metaconf.prefix")
+.doc("the prefix of the meta conf in HiveMetastoreClient, which is used to
extract " +
Review comment:
ok, i add a example about how to use the additional conf in this PR.
pls check @HyukjinKwon
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] kevincmchen commented on a change in pull request #33746: [SPARK-36514][SQL] Support to set the meta conf in HiveMetastoreClient
kevincmchen commented on a change in pull request #33746:
URL: https://github.com/apache/spark/pull/33746#discussion_r690018846
##
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala
##
@@ -118,6 +118,13 @@ private[spark] object HiveUtils extends Logging {
.toSequence
.createWithDefault(Nil)
+ val HIVE_METASTORE_METACONF_PREFIX =
buildStaticConf("spark.sql.hive.metastore.metaconf.prefix")
+.doc("the prefix of the meta conf in HiveMetastoreClient, which is used to
extract " +
Review comment:
ok, i have added a example about how to use the additional conf in this
PR. pls check @HyukjinKwon
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] kevincmchen commented on a change in pull request #33746: [SPARK-36514][SQL] Support to set the meta conf in HiveMetastoreClient
kevincmchen commented on a change in pull request #33746:
URL: https://github.com/apache/spark/pull/33746#discussion_r690018846
##
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala
##
@@ -118,6 +118,13 @@ private[spark] object HiveUtils extends Logging {
.toSequence
.createWithDefault(Nil)
+ val HIVE_METASTORE_METACONF_PREFIX =
buildStaticConf("spark.sql.hive.metastore.metaconf.prefix")
+.doc("the prefix of the meta conf in HiveMetastoreClient, which is used to
extract " +
Review comment:
ok, i have added an example about how to use the additional conf in this
PR. pls check @HyukjinKwon
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] kevincmchen commented on a change in pull request #33746: [SPARK-36514][SQL] Support to set the meta conf in HiveMetastoreClient
kevincmchen commented on a change in pull request #33746:
URL: https://github.com/apache/spark/pull/33746#discussion_r690018846
##
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala
##
@@ -118,6 +118,13 @@ private[spark] object HiveUtils extends Logging {
.toSequence
.createWithDefault(Nil)
+ val HIVE_METASTORE_METACONF_PREFIX =
buildStaticConf("spark.sql.hive.metastore.metaconf.prefix")
+.doc("the prefix of the meta conf in HiveMetastoreClient, which is used to
extract " +
Review comment:
ok, i have added a example about how to use the additional conf in this
PR. pls check @HyukjinKwon
##
File path: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveUtils.scala
##
@@ -118,6 +118,13 @@ private[spark] object HiveUtils extends Logging {
.toSequence
.createWithDefault(Nil)
+ val HIVE_METASTORE_METACONF_PREFIX =
buildStaticConf("spark.sql.hive.metastore.metaconf.prefix")
+.doc("the prefix of the meta conf in HiveMetastoreClient, which is used to
extract " +
Review comment:
ok, i have added an example about how to use the additional conf in this
PR. pls check @HyukjinKwon
--
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: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
