Skip to content

[SPARK-38713][SQL] Change spark.sessionstate.conf.getConf/setConf operation to spark.conf.get/set#35950

Closed
jackylee-ch wants to merge 4 commits intoapache:masterfrom
jackylee-ch:change_spark_sessionstate_conf_operation_to_spark_conf
Closed

[SPARK-38713][SQL] Change spark.sessionstate.conf.getConf/setConf operation to spark.conf.get/set#35950
jackylee-ch wants to merge 4 commits intoapache:masterfrom
jackylee-ch:change_spark_sessionstate_conf_operation_to_spark_conf

Conversation

@jackylee-ch
Copy link
Contributor

Why are the changes needed?

In the sql module, we provide SparkSession.conf as a unified entry for SQLConf.set/get, which can prevent users or logic from modifying StaticSQLConf and Spark configs. However, I found SparkSession.sessionstate.conf is used in some code to getConf or setConf, which can skip the check of RuntimeConfig.
In this PR, we want to unify the behavior of SQLConf.getConf/setConf to SparkSession.conf.

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Origin GA

@jackylee-ch
Copy link
Contributor Author

cc @srowen @cloud-fan @HyukjinKwon

@srowen
Copy link
Member

srowen commented Mar 23, 2022

I don't know this part well, but it seems reasonable to unify the checks here

@HyukjinKwon HyukjinKwon changed the title [SPARK-38419][SQL] change spark.sessionstate.conf.getConf/setConf operation to spark.conf.get/set [SPARK-38419][SQL] Change spark.sessionstate.conf.getConf/setConf operation to spark.conf.get/set Mar 23, 2022
@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@jackylee-ch
Copy link
Contributor Author

@cloud-fan @HyukjinKwon Any more question about this pr?
BTW, can we remove the AmplabJenkins report Since we won't use Jenkins any more?

@jackylee-ch
Copy link
Contributor Author

cc @srowen @cloud-fan @HyukjinKwon

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 454fe9c Mar 31, 2022
@cloud-fan
Copy link
Contributor

@jackylee-ch seems you link to the wrong JIRA ticket?

@jackylee-ch
Copy link
Contributor Author

@jackylee-ch seems you link to the wrong JIRA ticket?

This is a minor change, thus I didn't create a JIRA for it.

@cloud-fan
Copy link
Contributor

At least we shouldn't link to an unrelated JIRA... Please create a new one for this PR as it's not a minor one-line change.

@jackylee-ch jackylee-ch deleted the change_spark_sessionstate_conf_operation_to_spark_conf branch March 31, 2022 10:54
@jackylee-ch
Copy link
Contributor Author

At least we shouldn't link to an unrelated JIRA... Please create a new one for this PR as it's not a minor one-line change.

Sorry, I will create one inmediatly.

@jackylee-ch
Copy link
Contributor Author

https://issues.apache.org/jira/browse/SPARK-38713
This is the new Created Jira. @cloud-fan

@cloud-fan cloud-fan changed the title [SPARK-38419][SQL] Change spark.sessionstate.conf.getConf/setConf operation to spark.conf.get/set [SPARK-38713][SQL] Change spark.sessionstate.conf.getConf/setConf operation to spark.conf.get/set Mar 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants