[SPARK-46575][SQL][FOLLOWUP] Add back HiveThriftServer2.startWithContext(SQLContext) method for compatibility
#45727
Conversation
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
I agree with the goal of this PR, @dragqueen95 . Thank you for making a PR.
- Could you add
@Since(xxxVersionxxx)annotation for both methods to make it clear? - In addition, please enable GitHub Action on your repository to make CI pass. Apache Spark community uses your GitHub Action resource.
dongjoon-hyun
changed the title
HiveThriftServer2.startWithContext(SQLContext) method for compatibility
dongjoon-hyun
left a comment
dongjoon-hyun
left a comment
There was a problem hiding this comment.
+1, LGTM (Pending CIs).
To @dragqueen95 , we need to make CI happy.
https://github.com/apache/spark/pull/45727/checks?check_run_id=23117919159
| @Since("2.0.0") | ||
| @DeveloperApi | ||
| def startWithContext(sqlContext: SQLContext): HiveThriftServer2 = { | ||
| startWithContext(sqlContext, exitOnError) |
There was a problem hiding this comment.
Oh, could you give a value for exitOnError, @dragqueen95 ? The value seems to be missed here.
Error: /home/runner/work/spark/spark/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala:88: not found: value exitOnError
There was a problem hiding this comment.
true seems to be fine to me.
...e-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala
Outdated
Show resolved
Hide resolved
…/thriftserver/HiveThriftServer2.scala
|
Thank you for the update, @cloud-fan . |
|
thanks, merging to master! |
| * the call logs the error and exits the JVM with exit code -1. When false, the | ||
| * call throws an exception instead. | ||
| */ | ||
| @Since("3.5.2") |
There was a problem hiding this comment.
To @dragqueen95 , could you make a follow-up to fix this to 4.0.0 since SPARK-46575 is not in branch-3.5?
|
|
6 participants
What changes were proposed in this pull request?
#44575 added a default parameter to
HiveThriftServer2.startWithContextAPI. Although this is source-compatible, this is not binary-compatible - for eg. a binary built with new code won't be able to run with the old code. In this PR, we maintain forward and backward compatibility by keeping the API same and introducing a separate API for the additional parameter.Why are the changes needed?
See above
Does this PR introduce any user-facing change?
only developer API change
How was this patch tested?
Existing tests
Was this patch authored or co-authored using generative AI tooling?
no