You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@gluten.apache.org by "yikf (via GitHub)" <gi...@apache.org> on 2024/11/28 03:05:39 UTC

[PR] [GLUTEN-8070][CORE] Generate gluten configuration doc and check it [incubator-gluten]

yikf opened a new pull request, #8071:
URL: https://github.com/apache/incubator-gluten/pull/8071

   ## What changes were proposed in this pull request?
   https://github.com/apache/incubator-gluten/issues/8070, Add generate configurations golden file tools to auto generate Configuration.md doc and check it if PR change configs.
   
   ## How was this patch tested?
   manual 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@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-8070][CORE] Generate gluten configuration doc and check it [incubator-gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8071:
URL: https://github.com/apache/incubator-gluten/pull/8071#issuecomment-2586046177

   This PR is stale because it has been open 45 days with no activity. Remove stale label or comment or this will be closed in 10 days.


-- 
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@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-8070][CORE] Generate gluten configuration doc and check it [incubator-gluten]

Posted by "jackylee-ch (via GitHub)" <gi...@apache.org>.
jackylee-ch commented on code in PR #8071:
URL: https://github.com/apache/incubator-gluten/pull/8071#discussion_r1861521506


##########
docs/Configuration.md:
##########
@@ -1,107 +1,218 @@
+<!-- DO NOT MODIFY THIS FILE DIRECTLY, IT IS AUTO-GENERATED BY [org.apache.gluten.config.AllGlutenConfiguration] -->
+
 ---
+
 layout: page
 title: Configuration
 nav_order: 3
----
+------------
 
 # Spark Configurations for Gluten Plugin
 
 There are many configurations could impact the Gluten Plugin performance and can be fine-tuned in Spark.
 You can add these configurations into spark-defaults.conf to enable or disable the setting.
 
-## Spark parameters
-
-| Parameters                                                   | Description                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                               | Recommend Setting                                    |
-|--------------------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------|
-| spark.driver.extraClassPath                                  | Gluten Plugin jar file to prepend to the classpath of the driver.                                                                                                                                                                                                                                                                                                                                                                                                                                                                         | /path/to/jar_file                                    |

Review Comment:
   There is a issue #6970 that discussed `GlutenConfig` before.



-- 
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@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-8070][CORE] Generate gluten configuration doc and check it [incubator-gluten]

Posted by "zhztheplayer (via GitHub)" <gi...@apache.org>.
zhztheplayer commented on PR #8071:
URL: https://github.com/apache/incubator-gluten/pull/8071#issuecomment-2506928709

   > I recommend dividing the optimization efforts for config into two parts
   
   +1, works for me. 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.

To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-8070][CORE] Generate gluten configuration doc and check it [incubator-gluten]

Posted by "jackylee-ch (via GitHub)" <gi...@apache.org>.
jackylee-ch commented on PR #8071:
URL: https://github.com/apache/incubator-gluten/pull/8071#issuecomment-2505285475

   also cc @zhztheplayer @zzcclp 


-- 
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@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-8070][CORE] Generate gluten configuration doc and check it [incubator-gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8071:
URL: https://github.com/apache/incubator-gluten/pull/8071#issuecomment-2611379808

   This PR was auto-closed because it has been stalled for 10 days with no activity. Please feel free to reopen if it is still valid. 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.

To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-8070][CORE] Generate gluten configuration doc and check it [incubator-gluten]

Posted by "PHILO-HE (via GitHub)" <gi...@apache.org>.
PHILO-HE commented on code in PR #8071:
URL: https://github.com/apache/incubator-gluten/pull/8071#discussion_r1861472367


##########
gluten-core/src/test/scala/org/apache/gluten/MarkdownBuilder.scala:
##########
@@ -0,0 +1,127 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.gluten
+
+import com.vladsch.flexmark.formatter.Formatter
+import com.vladsch.flexmark.parser.{Parser, ParserEmulationProfile, PegdownExtensions}
+import com.vladsch.flexmark.profile.pegdown.PegdownOptionsAdapter
+import com.vladsch.flexmark.util.data.{MutableDataHolder, MutableDataSet}
+import com.vladsch.flexmark.util.sequence.SequenceUtils.EOL
+
+import scala.collection.mutable.ListBuffer
+
+class MarkdownBuilder {
+  private val buffer = new ListBuffer[String]
+
+  /**
+   * append a single line with replacing EOL to empty string

Review Comment:
   Nit: 
   comment style, capitalize the initial letter and end up with period mark. Please also check other comments.



##########
gluten-core/src/test/scala/org/apache/gluten/config/AllGlutenConfiguration.scala:
##########
@@ -0,0 +1,260 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.gluten.config
+
+import org.apache.gluten.GlutenConfig
+import org.apache.gluten.MarkdownBuilder
+
+import org.apache.spark.sql.internal.SQLConfBridge
+
+import org.scalactic.Prettifier
+import org.scalactic.source.Position
+import org.scalatest.Assertions._
+import org.scalatest.funsuite.AnyFunSuite
+
+import java.nio.charset.StandardCharsets
+import java.nio.file.{Files, Path, Paths, StandardOpenOption}
+
+import scala.collection.JavaConverters._
+import scala.collection.Traversable
+import scala.io.Source
+
+/**
+ * End-to-end test cases for configuration doc file The golden result file is
+ * "docs/Configuration.md".
+ *
+ * To run the entire test suite:
+ * {{{
+ *   GLUTEN_UPDATE=0 dev/gen/gen_all_config_docs.sh
+ * }}}
+ *
+ * To re-generate golden files for entire suite, run:
+ * {{{
+ *   dev/gen/gen_all_config_docs.sh
+ * }}}
+ */
+class AllGlutenConfiguration extends AnyFunSuite {
+  private val glutenHome: String =
+    AllGlutenConfiguration.getCodeSourceLocation(this.getClass).split("gluten-core")(0)
+  private val markdown = Paths.get(glutenHome, "docs", "Configuration.md").toAbsolutePath
+
+  private val veloxBackendIdentify = ".velox."
+  private val chBackendIdentify = ".ch."
+
+  private def loadConfigs = Array(GlutenConfig)
+
+  test("Check all gluten configs") {
+    loadConfigs
+    val builder = MarkdownBuilder(getClass.getName)
+
+    builder ++=
+      s"""
+         |---
+         |layout: page
+         |title: Configuration
+         |nav_order: 3
+         |---
+         |
+         |"""
+
+    builder ++=
+      s"""
+         |# Spark Configurations for Gluten Plugin
+         |
+         |""" +=
+      """There are many configurations could impact the Gluten Plugin performance
+        | and can be fine-tuned in Spark.""" +=
+      """You can add these configurations into spark-defaults.conf to enable or
+        | disable the setting."""
+
+    builder ++=
+      s"""
+         |## Gluten configurations
+         |
+         | Key | Default | Meaning

Review Comment:
   Suggestion: 
   Meaning -> Description



-- 
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@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-8070][CORE] Generate gluten configuration doc and check it [incubator-gluten]

Posted by "yikf (via GitHub)" <gi...@apache.org>.
yikf commented on PR #8071:
URL: https://github.com/apache/incubator-gluten/pull/8071#issuecomment-2505322631

   thanks @jackylee-ch and @PHILO-HE , After learning more about your suggestions and relevant background, I recommend dividing the optimization efforts for config into two parts:
   
   1. Optimize the relevant config definitions, just like what was mentioned in #6970.
   2. Support generate gluten configuration doc.
   
   Regarding `#1`, I will be putting together a draft in the next few days and initiating discussion in the community.
   
   We can continue with this RP after `#1` is complete.


-- 
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@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-8070][CORE] Generate gluten configuration doc and check it [incubator-gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8071:
URL: https://github.com/apache/incubator-gluten/pull/8071#issuecomment-2505177179

   Run Gluten Clickhouse CI on x86


-- 
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@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-8070][CORE] Generate gluten configuration doc and check it [incubator-gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #8071: [GLUTEN-8070][CORE] Generate gluten configuration doc and check it
URL: https://github.com/apache/incubator-gluten/pull/8071


-- 
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@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-8070][CORE] Generate gluten configuration doc and check it [incubator-gluten]

Posted by "jackylee-ch (via GitHub)" <gi...@apache.org>.
jackylee-ch commented on code in PR #8071:
URL: https://github.com/apache/incubator-gluten/pull/8071#discussion_r1861513186


##########
docs/Configuration.md:
##########
@@ -1,107 +1,218 @@
+<!-- DO NOT MODIFY THIS FILE DIRECTLY, IT IS AUTO-GENERATED BY [org.apache.gluten.config.AllGlutenConfiguration] -->
+
 ---
+
 layout: page
 title: Configuration
 nav_order: 3
----
+------------
 
 # Spark Configurations for Gluten Plugin
 
 There are many configurations could impact the Gluten Plugin performance and can be fine-tuned in Spark.
 You can add these configurations into spark-defaults.conf to enable or disable the setting.
 
-## Spark parameters
-
-| Parameters                                                   | Description                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                               | Recommend Setting                                    |
-|--------------------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|------------------------------------------------------|
-| spark.driver.extraClassPath                                  | Gluten Plugin jar file to prepend to the classpath of the driver.                                                                                                                                                                                                                                                                                                                                                                                                                                                                         | /path/to/jar_file                                    |

Review Comment:
   We need something to do before regenrating the `Configuration.md`.
   1. A way to define the spark parameters that used for gluten;
   2. Synchronize the configuration information in `Configuration.md` and `GlutenConfig`, because some configurations are not well defined in GlutenConfig;
   3. Use a unified interface to define the gluten configs, such as internal, backends, and so on, so we can define how to show each config in `Configuration.md`



-- 
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@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-8070][CORE] Generate gluten configuration doc and check it [incubator-gluten]

Posted by "yikf (via GitHub)" <gi...@apache.org>.
yikf commented on PR #8071:
URL: https://github.com/apache/incubator-gluten/pull/8071#issuecomment-2505178381

   @zhouyuan @PHILO-HE @jackylee-ch do you like the feature and Is this feature necessary?


-- 
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@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org


Re: [PR] [GLUTEN-8070][CORE] Generate gluten configuration doc and check it [incubator-gluten]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #8071:
URL: https://github.com/apache/incubator-gluten/pull/8071#issuecomment-2505176996

   https://github.com/apache/incubator-gluten/issues/8070


-- 
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@gluten.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@gluten.apache.org
For additional commands, e-mail: commits-help@gluten.apache.org