[GitHub] [spark] beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL] Add version property for ConfigEntry and ConfigBuilder
beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL]
Add version property for ConfigEntry and ConfigBuilder
URL: https://github.com/apache/spark/pull/27592#discussion_r382341913
##
File path:
core/src/main/scala/org/apache/spark/internal/config/ConfigBuilder.scala
##
@@ -199,6 +200,11 @@ private[spark] case class ConfigBuilder(key: String) {
this
}
+ def version(v: String): ConfigBuilder = {
Review comment:
Although we can checked it, but I think convention is greater than
configuration.
This is very similar to `ExpressionDescription.since`.
If we create an Enum, it has big size. 0.5.0 ~ 3.1.0
So I don’t think it ’s appropriate to use Enum.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL] Add version property for ConfigEntry and ConfigBuilder
beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL]
Add version property for ConfigEntry and ConfigBuilder
URL: https://github.com/apache/spark/pull/27592#discussion_r382341913
##
File path:
core/src/main/scala/org/apache/spark/internal/config/ConfigBuilder.scala
##
@@ -199,6 +200,11 @@ private[spark] case class ConfigBuilder(key: String) {
this
}
+ def version(v: String): ConfigBuilder = {
Review comment:
Although we can checked it, but I think convention is greater than
configuration.
This is very similar to `ExpressionDescription.since`.
If we create an Enum, it has big size. So I don’t think it ’s appropriate to
use Enum.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL] Add version property for ConfigEntry and ConfigBuilder
beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL]
Add version property for ConfigEntry and ConfigBuilder
URL: https://github.com/apache/spark/pull/27592#discussion_r381148548
##
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala
##
@@ -115,14 +115,15 @@ case class SetCommand(kv: Option[(String,
Option[String])]) extends RunnableComm
case Some(("-v", None)) =>
val runFunc = (sparkSession: SparkSession) => {
sparkSession.sessionState.conf.getAllDefinedConfs.sorted.map {
- case (key, defaultValue, doc) =>
-Row(key, Option(defaultValue).getOrElse(""), doc)
+ case (key, defaultValue, doc, version) =>
+Row(key, Option(defaultValue).getOrElse(""), doc,
version)
Review comment:
I personally prefer the latter.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL] Add version property for ConfigEntry and ConfigBuilder
beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL]
Add version property for ConfigEntry and ConfigBuilder
URL: https://github.com/apache/spark/pull/27592#discussion_r381145687
##
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala
##
@@ -115,14 +115,15 @@ case class SetCommand(kv: Option[(String,
Option[String])]) extends RunnableComm
case Some(("-v", None)) =>
val runFunc = (sparkSession: SparkSession) => {
sparkSession.sessionState.conf.getAllDefinedConfs.sorted.map {
- case (key, defaultValue, doc) =>
-Row(key, Option(defaultValue).getOrElse(""), doc)
+ case (key, defaultValue, doc, version) =>
+Row(key, Option(defaultValue).getOrElse(""), doc,
version)
}
}
val schema = StructType(
StructField("key", StringType, nullable = false) ::
StructField("value", StringType, nullable = false) ::
- StructField("meaning", StringType, nullable = false) :: Nil)
+ StructField("meaning", StringType, nullable = false) ::
+ StructField("version", StringType, nullable = false) :: Nil)
Review comment:
OK
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL] Add version property for ConfigEntry and ConfigBuilder
beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL] Add version property for ConfigEntry and ConfigBuilder URL: https://github.com/apache/spark/pull/27592#discussion_r381145643 ## File path: sql/gen-sql-config-docs.py ## @@ -68,7 +70,7 @@ def generate_sql_configs_table(sql_configs, path): f.write(dedent( """ -Property NameDefaultMeaning +Property NameDefaultMeaningVersion Review comment: Oh my fault 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL] Add version property for ConfigEntry and ConfigBuilder
beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL] Add version property for ConfigEntry and ConfigBuilder URL: https://github.com/apache/spark/pull/27592#discussion_r380477033 ## File path: sql/gen-sql-config-docs.py ## @@ -49,12 +50,13 @@ def generate_sql_configs_table(sql_configs, path): ```html -Property NameDefaultMeaning +Property NameDefaultMeaningVersion Review comment: OK 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL] Add version property for ConfigEntry and ConfigBuilder
beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL] Add version property for ConfigEntry and ConfigBuilder URL: https://github.com/apache/spark/pull/27592#discussion_r380412194 ## File path: sql/gen-sql-config-docs.py ## @@ -49,12 +50,13 @@ def generate_sql_configs_table(sql_configs, path): ```html -Property NameDefaultMeaning +Property NameDefaultMeaningVersion Review comment: How about `Since Version`? 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL] Add version property for ConfigEntry and ConfigBuilder
beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL]
Add version property for ConfigEntry and ConfigBuilder
URL: https://github.com/apache/spark/pull/27592#discussion_r380064769
##
File path:
core/src/main/scala/org/apache/spark/internal/config/ConfigBuilder.scala
##
@@ -199,6 +200,11 @@ private[spark] case class ConfigBuilder(key: String) {
this
}
+ def version(v: String): ConfigBuilder = {
+_version = v
Review comment:
Although we can checked it, but I think convention is greater than
configuration.
This is very similar to `ExpressionDescription.since`.
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL] Add version property for ConfigEntry and ConfigBuilder
beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL]
Add version property for ConfigEntry and ConfigBuilder
URL: https://github.com/apache/spark/pull/27592#discussion_r380064769
##
File path:
core/src/main/scala/org/apache/spark/internal/config/ConfigBuilder.scala
##
@@ -199,6 +200,11 @@ private[spark] case class ConfigBuilder(key: String) {
this
}
+ def version(v: String): ConfigBuilder = {
+_version = v
Review comment:
Although we can checked it, but I think convention is greater than
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL] Add version property for ConfigEntry and ConfigBuilder
beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL] Add version property for ConfigEntry and ConfigBuilder URL: https://github.com/apache/spark/pull/27592#discussion_r380065154 ## File path: core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala ## @@ -39,6 +39,7 @@ package org.apache.spark.internal.config * @param doc the documentation for the configuration * @param isPublic if this configuration is public to the user. If it's `false`, this * configuration is only used internally and we should not expose it to users. + * @tparam version the spark version when the configuration was released. Review comment: OK. Thanks. 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org With regards, Apache Git Services - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] [spark] beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL] Add version property for ConfigEntry and ConfigBuilder
beliefer commented on a change in pull request #27592: [SPARK-30840][CORE][SQL]
Add version property for ConfigEntry and ConfigBuilder
URL: https://github.com/apache/spark/pull/27592#discussion_r379881659
##
File path:
core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala
##
@@ -74,7 +76,8 @@ private[spark] abstract class ConfigEntry[T] (
def defaultValue: Option[T] = None
override def toString: String = {
-s"ConfigEntry(key=$key, defaultValue=$defaultValueString, doc=$doc,
public=$isPublic)"
+s"ConfigEntry(key=$key, defaultValue=$defaultValueString, doc=$doc, " +
+ s"public=$isPublic, version = $version)"
Review comment:
OK
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
