Re: [PR] [SPARK-46395][CORE] Assign Spark configs to groups for use in documentation [spark]
github-actions[bot] closed pull request #44755: [SPARK-46395][CORE] Assign Spark configs to groups for use in documentation URL: https://github.com/apache/spark/pull/44755 -- 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
Re: [PR] [SPARK-46395][CORE] Assign Spark configs to groups for use in documentation [spark]
github-actions[bot] commented on PR #44755: URL: https://github.com/apache/spark/pull/44755#issuecomment-2319532296 We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag! -- 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
Re: [PR] [SPARK-46395][CORE] Assign Spark configs to groups for use in documentation [spark]
nchammas commented on code in PR #44755:
URL: https://github.com/apache/spark/pull/44755#discussion_r1608468487
##
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##
@@ -5704,10 +5704,9 @@ class SQLConf extends Serializable with Logging with
SqlApiConf {
settings.synchronized { settings.asScala.toMap }
/**
- * Return all the configuration definitions that have been defined in
[[SQLConf]]. Each
- * definition contains key, defaultValue and doc.
+ * Return all the public configuration definitions that have been defined in
[[SQLConf]].
*/
- def getAllDefinedConfs: Seq[(String, String, String, String)] = {
+ def getAllDefinedConfs: Seq[(String, String, String, String, Set[String])] =
{
Review Comment:
I think this is only used by us, and I also think it's the right place to
make a change judging from the docstring. We also have a rare chance to "get
away" with breaking public APIs due to the upcoming 4.0 release.
But it is indeed a public API. Do you prefer I just create a new API? It
will duplicate this existing API but include the new documentation groups as
well. I was thinking to also have this return a `Seq` of a case class with
descriptive attribute names, so we're not calling `._1` and the like everywhere.
--
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
Re: [PR] [SPARK-46395][CORE] Assign Spark configs to groups for use in documentation [spark]
holdenk commented on PR #44755: URL: https://github.com/apache/spark/pull/44755#issuecomment-2108746918 My bad github staged my review comment for some reason, should be visible now. -- 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
Re: [PR] [SPARK-46395][CORE] Assign Spark configs to groups for use in documentation [spark]
holdenk commented on code in PR #44755:
URL: https://github.com/apache/spark/pull/44755#discussion_r1580256318
##
sql/catalyst/src/main/scala/org/apache/spark/sql/internal/SQLConf.scala:
##
@@ -5704,10 +5704,9 @@ class SQLConf extends Serializable with Logging with
SqlApiConf {
settings.synchronized { settings.asScala.toMap }
/**
- * Return all the configuration definitions that have been defined in
[[SQLConf]]. Each
- * definition contains key, defaultValue and doc.
+ * Return all the public configuration definitions that have been defined in
[[SQLConf]].
*/
- def getAllDefinedConfs: Seq[(String, String, String, String)] = {
+ def getAllDefinedConfs: Seq[(String, String, String, String, Set[String])] =
{
Review Comment:
This does change a public APIs type signature, I'm not sure it's super
important or widely used but given the stated goal of the PR change is to
improve the docs do we need this part? Or could a new internal API be
introduced if we need this functionality.
--
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
Re: [PR] [SPARK-46395][CORE] Assign Spark configs to groups for use in documentation [spark]
nchammas commented on PR #44755: URL: https://github.com/apache/spark/pull/44755#issuecomment-2098662588 Following up regarding your feedback on the API change, @holdenk. Do you recall what it was? -- 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
Re: [PR] [SPARK-46395][CORE] Assign Spark configs to groups for use in documentation [spark]
nchammas commented on PR #44755: URL: https://github.com/apache/spark/pull/44755#issuecomment-2078593953 > I like it, one small note around the API change I'm missing the note. Did a line comment on the diff get swallowed up, perhaps? -- 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
Re: [PR] [SPARK-46395][CORE] Assign Spark configs to groups for use in documentation [spark]
holdenk commented on PR #44755: URL: https://github.com/apache/spark/pull/44755#issuecomment-2078345591 I like it, one small note around the API change -- 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
Re: [PR] [SPARK-46395][CORE] Assign Spark configs to groups for use in documentation [spark]
holdenk commented on PR #44755: URL: https://github.com/apache/spark/pull/44755#issuecomment-2037640463 Oh yes, sorry it's been a busy quarter. Let me try and schedule some review time tomorrow / next week (and do please feel free to ping me again if I forget, work is just very busy as of late). -- 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
Re: [PR] [SPARK-46395][CORE] Assign Spark configs to groups for use in documentation [spark]
nchammas commented on PR #44755: URL: https://github.com/apache/spark/pull/44755#issuecomment-2037633304 @holdenk - Friendly ping. Are you still interested in shepherding this work? No hard feelings if not. -- 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
Re: [PR] [SPARK-46395][CORE] Assign Spark configs to groups for use in documentation [spark]
nchammas commented on PR #44755: URL: https://github.com/apache/spark/pull/44755#issuecomment-1979311637 @holdenk - This is the config documentation approach we discussed on the mailing list. (The alternative, YAML-based approach is over on #44756.) This PR just adds the fields and methods we need on `ConfigEntry`. All the work to update our documentation scripts and migrate the HTML tables into the appropriate config entries will happen in subsequent PRs. I'll group them up under an umbrella Jira ticket. -- 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
