[GitHub] [spark] advancedxy commented on a diff in pull request #41168: [SPARK-43454][CORE] support substitution for SparkConf's get and getAllWithPrefix

["advancedxy (via GitHub)" <gi...@apache.org>]

advancedxy commented on code in PR #41168:
URL: https://github.com/apache/spark/pull/41168#discussion_r1194522841


##########
core/src/main/scala/org/apache/spark/SparkConf.scala:
##########
@@ -403,7 +403,7 @@ class SparkConf(loadDefaults: Boolean) extends Cloneable with Logging with Seria
    */
   def getAllWithPrefix(prefix: String): Array[(String, String)] = {

Review Comment:
   > We may want to be more explicit about when to substitute and when not to. 
   
   Sure and agreed. In my opinion, `getOption` should be substituted which is consistent with `get(xx_config_entry)`. 
   
   `getAll` shall not be substituted, as it's also used to create spark.properties file when submitted by spark-submit to resource managers, if substituted, the replaced value may reference the submit client's environment, which might not work as expected. We may add  `private[spark] def getAllWithSubstitution` method though.
   
   Variants deriving from `getAll`, such as `getAllWithPrefix`, as long as that's a query only operation, should be substituted, so they are consistent with get one by one.
   
   > shouldn't that mean the current inconsistent behavior would only be partially resolved by this PR?
   
   The `spark.hadoop.*` and `spark.hive.*` cases didn't occur to me. I'm not sure whether they should be substituted or not as they are also passed to hadoop/hive configuration, which may already have substitution enabled.
   
   For other cases, we may solve the inconsistent behavior case by case.



-- 
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