-
Notifications
You must be signed in to change notification settings - Fork 28.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[SPARK-48775][SQL][STS] Replace SQLContext with SparkSession in STS #47176
Conversation
also cc @dongjoon-hyun |
...-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIService.scala
Outdated
Show resolved
Hide resolved
* @param exitOnError Whether to exit the JVM if HiveThriftServer2 fails to initialize. When true, | ||
* the call logs the error and exits the JVM with exit code -1. When false, the | ||
* call throws an exception instead. | ||
*/ | ||
@Since("4.0.0") | ||
@DeveloperApi | ||
def startWithContext(sqlContext: SQLContext, exitOnError: Boolean): HiveThriftServer2 = { | ||
def startWithSparkSession( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that aside from the unreleased API added in 4.0, this pr has not caused any breaking changes.
If so, the changes is fine to me. WDYT? @HyukjinKwon @yaooqinn
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's okay to change . Maybe, this is the last chance if we want to change this.
@@ -82,10 +84,11 @@ object HiveThriftServer2 extends Logging { | |||
* | |||
* @param sqlContext SQLContext to use for the server | |||
*/ | |||
@deprecated("use startWithSparkSession instead", since = "4.0.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit. use
-> Use
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, LGTM.
…/thriftserver/SparkSQLCLIService.scala Co-authored-by: YangJie <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for updating. Pending CIs.
### What changes were proposed in this pull request? Remove the exposed `SQLContext` which was added in SPARK-46575. And migrate STS internal used `SQLContext` to `SparkSession`. ### Why are the changes needed? `SQLContext` is not recommended since Spark 2.0, the suggested replacement is `SparkSession`. We should avoid exposing the deprecated class to Developer API in new versions. ### Does this PR introduce _any_ user-facing change? No. It touched the Developer API added in SPARK-46575, but is not released yet. ### How was this patch tested? Pass GHA, and `dev/mima` (not breaking changes involved) ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#47176 from pan3793/SPARK-48775. Lead-authored-by: Cheng Pan <[email protected]> Co-authored-by: Cheng Pan <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
### What changes were proposed in this pull request? Remove the exposed `SQLContext` which was added in SPARK-46575. And migrate STS internal used `SQLContext` to `SparkSession`. ### Why are the changes needed? `SQLContext` is not recommended since Spark 2.0, the suggested replacement is `SparkSession`. We should avoid exposing the deprecated class to Developer API in new versions. ### Does this PR introduce _any_ user-facing change? No. It touched the Developer API added in SPARK-46575, but is not released yet. ### How was this patch tested? Pass GHA, and `dev/mima` (not breaking changes involved) ### Was this patch authored or co-authored using generative AI tooling? No. Closes apache#47176 from pan3793/SPARK-48775. Lead-authored-by: Cheng Pan <[email protected]> Co-authored-by: Cheng Pan <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>
What changes were proposed in this pull request?
Remove the exposed
SQLContext
which was added in SPARK-46575. And migrate STS internal usedSQLContext
toSparkSession
.Why are the changes needed?
SQLContext
is not recommended since Spark 2.0, the suggested replacement isSparkSession
. We should avoid exposing the deprecated class to Developer API in new versions.Does this PR introduce any user-facing change?
No. It touched the Developer API added in SPARK-46575, but is not released yet.
How was this patch tested?
Pass GHA, and
dev/mima
(not breaking changes involved)Was this patch authored or co-authored using generative AI tooling?
No.