You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "jwang0306 (via GitHub)" <gi...@apache.org> on 2023/05/25 00:00:27 UTC

[GitHub] [spark] jwang0306 opened a new pull request, #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

jwang0306 opened a new pull request, #41302:
URL: https://github.com/apache/spark/pull/41302

   <!--
   Thanks for sending a pull request!  Here are some tips for you:
     1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
     2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
     4. Be sure to keep the PR description updated to reflect all changes.
     5. Please write your PR title to summarize what this PR proposes.
     6. If possible, provide a concise example to reproduce the issue for a faster review.
     7. If you want to add a new configuration, please read the guideline first for naming configurations in
        'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
     8. If you want to add or modify an error type or message, please read the guideline first in
        'core/src/main/resources/error/README.md'.
   -->
   
   ### What changes were proposed in this pull request?
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
   If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
     1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
     2. If you fix some SQL features, you can provide some references of other DBMSes.
     3. If there is design documentation, please add the link.
     4. If there is a discussion in the mailing list, please add the link.
   -->
   This PR proposes to add a static Spark conf to configure and override to the Log4j logging level. It’s a config version of the `SparkContext.setLogLevel()` semantics, when set it’ll trigger a log level override at the beginning of `SparkContext` startup.
   
   ### Why are the changes needed?
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   This is particularly helpful in the Spark Connect scenario where there’s no way for the client to call `setLogLevel` because `SparkContext` is not yet supported in its API, and when it connects to a platform where it can't change the Log4j config file.
   
   ### Does this PR introduce _any_ user-facing change?
   <!--
   Note that it means *any* user-facing change including all aspects such as the documentation fix.
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   It adds a new public configuration `"spark.log.level"`, and by default it's unset which means there's no override.
   
   ### How was this patch tested?
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
   -->
   A new unit test in `SparkContextSuite`.
   


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] jwang0306 commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "jwang0306 (via GitHub)" <gi...@apache.org>.
jwang0306 commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1562077605

   cc @grundprinzip @cloud-fan, please take a look.


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun closed pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf
URL: https://github.com/apache/spark/pull/41302


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1563683823

   cc @rednaxelafx @dongjoon-hyun FYI


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] jwang0306 commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "jwang0306 (via GitHub)" <gi...@apache.org>.
jwang0306 commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1569028481

   @dongjoon-hyun thanks for the suggestion, the variable is now renamed.
   
   For (3), I think I slightly prefer adding a new field in `Logging` to check whether the warning message is printed, so we could print the warning message under the same condition. Does this sounds good to you?
   
   ```diff
   diff --git a/core/src/main/scala/org/apache/spark/SparkContext.scala b/core/src/main/scala/org/apache/spark/SparkContext.scala
   index e142d79523..34fdd4fc32 100644
   --- a/core/src/main/scala/org/apache/spark/SparkContext.scala
   +++ b/core/src/main/scala/org/apache/spark/SparkContext.scala
   @@ -396,7 +396,12 @@ class SparkContext(config: SparkConf) extends Logging {
    
      try {
        _conf = config.clone()
   -    _conf.get(SPARK_LOG_LEVEL).foreach(setLogLevel(_))
   +    _conf.get(SPARK_LOG_LEVEL).foreach { level =>
   +      if (Logging.setLogLevelPrinted) {
   +        System.err.printf("Setting Spark log level to \"%s\".\n", level)
   +      }
   +      setLogLevel(level)
   +    }
        _conf.validateSettings()
        _conf.set("spark.app.startTime", startTime.toString)
    
   diff --git a/core/src/main/scala/org/apache/spark/internal/Logging.scala b/core/src/main/scala/org/apache/spark/internal/Logging.scala
   index 614103dee7..fa00783a9d 100644
   --- a/core/src/main/scala/org/apache/spark/internal/Logging.scala
   +++ b/core/src/main/scala/org/apache/spark/internal/Logging.scala
   @@ -139,6 +139,7 @@ trait Logging {
                context.setConfigLocation(url.toURI)
                if (!silent) {
                  System.err.println(s"Using Spark's default log4j profile: $defaultLogProps")
   +              Logging.setLogLevelPrinted = true
                }
              case None =>
                System.err.println(s"Spark was unable to load $defaultLogProps")
   @@ -164,6 +165,7 @@ trait Logging {
                System.err.printf("Setting default log level to \"%s\".\n", replLevel)
                System.err.println("To adjust logging level use sc.setLogLevel(newLevel). " +
                  "For SparkR, use setLogLevel(newLevel).")
   +            Logging.setLogLevelPrinted = true
              }
              Logging.sparkShellThresholdLevel = replLevel
              rootLogger.getAppenders().asScala.foreach {
   @@ -189,6 +191,7 @@ private[spark] object Logging {
      @volatile private var defaultRootLevel: Level = null
      @volatile private var defaultSparkLog4jConfig = false
      @volatile private[spark] var sparkShellThresholdLevel: Level = null
   +  @volatile var setLogLevelPrinted: Boolean = false
    
      val initLock = new Object()
      try {
   ```


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1569097104

   So, does the proposal still have three lines of logs like the following when `spark.log.level=TRACE`?
   ```
   Setting default log level to "WARN".
   To adjust logging level use sc.setLogLevel(newLevel). For SparkR, use setLogLevel(newLevel).
   ...
   Setting Spark log level to "TRACE".
   ```
   
   Is the main difficult that `Logging` class doesn't have `SparkConf`?


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1569552423

   Thank you, @jwang0306 , @HyukjinKwon , @grundprinzip .
   Merged to master for Apache Spark 3.5.0.
   
   
   And, welcome to the Apache Spark community, @jwang0306 . I added you to the Apache Spark contributor group and assigned SPARK-43782 to 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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] jwang0306 commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "jwang0306 (via GitHub)" <gi...@apache.org>.
jwang0306 commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1572973821

   > And, welcome to the Apache Spark community, @jwang0306 . I added you to the Apache Spark contributor group and assigned [SPARK-43782](https://issues.apache.org/jira/browse/SPARK-43782) to you.
   
   Many 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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] grundprinzip commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "grundprinzip (via GitHub)" <gi...@apache.org>.
grundprinzip commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1563242549

   It's not meant to be used by spark Connect but simply a convenience option that folks can provide on cluster startup without manipulating the whole log4j.conf which can be quite cumbersome. 


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] jwang0306 commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "jwang0306 (via GitHub)" <gi...@apache.org>.
jwang0306 commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1569110301

   >  does the proposal still have three lines of logs like the following when spark.log.level=TRACE in spark-shell/pyspark/sparkr shells?
   
   Yes.
   
   > Is the main difficult that Logging class doesn't have SparkConf?
   
   Yes, the `Logging` trait can be mixed in to multiple places, and unfortunately it’s not feasible to take in `SparkConf` in that trait.


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] jwang0306 commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "jwang0306 (via GitHub)" <gi...@apache.org>.
jwang0306 commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1569462012

   Gentle ping @dongjoon-hyun, I further added `private[spark]` to the newly added field in `Logging`. Would you mind taking another pass?


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] LuciferYang commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1563237475

   @jwang0306 Sorry I may miss the point, can you give an example to illustrate how we use this configuration in the Spark Connect scenario?


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] jwang0306 commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "jwang0306 (via GitHub)" <gi...@apache.org>.
jwang0306 commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1564666698

   @dongjoon-hyun thanks for the review,
   1. This config statically overrides the log level, but that doesn't  prevent users from dynamically setting new overrides with `setLogLevel`. If you have a strong preference, we can change it to a name without the override word in it, what would you suggest?
   2. Sounds good, but let's decide whether to add it or not after (3).
   3. That's a very good point. To solve this inconsistency, I think we have multiple alternatives:
       - That line of log happens when `isInterpreter` is true and `silent` = false, however, these are not fields in the Logging trait. So we can't read them in `SparkContext` to warn users about the override under the exact same condition. Maybe we can made the `isInterpreter` and `silent` fields in the Logging trait, or add a new filed to tell whether or not the REPL warning is printed. If we don't care about the original condition (i.e., we don't care about the `silent` mode), then we can print the warning log unconditionally like you suggested.
       - Alternatively, the Logging trait itself might be a better place to implement the override, but `SparkConf` is not available when the trait is initializing. So if we choose to configure the log level override there, we'll have to change the conf to use something else that's already available at that point, like environment variable or Java System Properties. WDYT? cc. @grundprinzip @HyukjinKwon @cloud-fan @LuciferYang 


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #41302:
URL: https://github.com/apache/spark/pull/41302#discussion_r1210956892


##########
core/src/main/scala/org/apache/spark/internal/Logging.scala:
##########
@@ -189,6 +191,7 @@ private[spark] object Logging {
   @volatile private var defaultRootLevel: Level = null
   @volatile private var defaultSparkLog4jConfig = false
   @volatile private[spark] var sparkShellThresholdLevel: Level = null
+  @volatile var setLogLevelPrinted: Boolean = false

Review Comment:
   `private[spark]`?



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #41302:
URL: https://github.com/apache/spark/pull/41302#discussion_r1210959787


##########
core/src/main/scala/org/apache/spark/SparkContext.scala:
##########
@@ -2679,7 +2685,7 @@ class SparkContext(config: SparkConf) extends Logging {
  * various Spark features.
  */
 object SparkContext extends Logging {
-  private val VALID_LOG_LEVELS =
+  val VALID_LOG_LEVELS =

Review Comment:
   Given the use case, the following will be sufficient.
   ```scala
   -  val VALID_LOG_LEVELS =
   +  private[spark] val VALID_LOG_LEVELS =
   ```



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1569190409

   Got it. Given the limitation, +1 for the proposed approach. Thank you, @jwang0306 .


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a diff in pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #41302:
URL: https://github.com/apache/spark/pull/41302#discussion_r1210956457


##########
docs/configuration.md:
##########
@@ -412,6 +412,15 @@ of the most common options to set are:
   </td>
   <td>2.2.0</td>
 </tr>
+<tr>
+  <td><code>spark.log.level</code></td>
+  <td>(none)</td>
+  <td>
+    When set, overrides any user-defined log settings as if calling
+    <code>SparkContext.setLogLevel()</code> at Spark startup. Valid log levels include: "ALL", "DEBUG", "ERROR", "FATAL", "INFO", "OFF", "TRACE", "WARN".
+  </td>
+  <td>3.5.0</td>
+</tr>

Review Comment:
   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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1565169240

   Give the config name, `spark.log.level`, simply `LOG_LEVEL` or `SPARK_LOG_LEVEL`?
   
   > This config statically overrides the log level, but that doesn't prevent users from dynamically setting new overrides with setLogLevel. If you have a strong preference, we can change it to a name without the override word in it, what would you suggest?
   


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] jwang0306 commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "jwang0306 (via GitHub)" <gi...@apache.org>.
jwang0306 commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1563294461

   @LuciferYang thanks for the question, given the context provided by @grundprinzip, do you have any suggestions on how to update the PR description to better help understand?
   
   > For example launching a cluster with the following command: `./bin/pyspark --conf spark.log.level ERROR`
   > In this setup, the Spark Connect client doesn't have access to `SparkContext`, so we can't configure the log level using `sc.setLogLevel()`. But now with this --conf, the launcher is able to 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-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] grundprinzip commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "grundprinzip (via GitHub)" <gi...@apache.org>.
grundprinzip commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1563250041

   Correct, with Spark Connect does not have access to the spark context. For users that want to transition this config it becomes bow very easy. Before they would use set log level as it provides the easiest way to increase or decrease the level. With this option the user has the same surface but using a spark conf. 


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] LuciferYang commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1563246239

   But the PR description mentions `This is particularly helpful in the Spark Connect scenario` ...
   
   


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] jwang0306 commented on a diff in pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "jwang0306 (via GitHub)" <gi...@apache.org>.
jwang0306 commented on code in PR #41302:
URL: https://github.com/apache/spark/pull/41302#discussion_r1210963365


##########
core/src/main/scala/org/apache/spark/SparkContext.scala:
##########
@@ -2679,7 +2685,7 @@ class SparkContext(config: SparkConf) extends Logging {
  * various Spark features.
  */
 object SparkContext extends Logging {
-  private val VALID_LOG_LEVELS =
+  val VALID_LOG_LEVELS =

Review Comment:
   Thanks! It's now updated.



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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] jwang0306 commented on pull request #41302: [SPARK-43782][CORE] Support log level configuration with static Spark conf

Posted by "jwang0306 (via GitHub)" <gi...@apache.org>.
jwang0306 commented on PR #41302:
URL: https://github.com/apache/spark/pull/41302#issuecomment-1569307185

   @dongjoon-hyun both (2) and (3) are now updated, please take another look.


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

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org