You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "dragqueen95 (via GitHub)" <gi...@apache.org> on 2024/03/26 17:00:15 UTC
[PR] [SPARK-46575][SQL][FOLLOWUP] Maintain backward/ forward compatibility for the DeveloperApi HiveThriftServer2.startWithContext [spark]
dragqueen95 opened a new pull request, #45727:
URL: https://github.com/apache/spark/pull/45727
### What changes were proposed in this pull request?
https://github.com/apache/spark/pull/44575 added a default parameter to `HiveThriftServer2.startWithContext` API. 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
--
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
Re: [PR] [SPARK-46575][SQL][FOLLOWUP] Add back `HiveThriftServer2.startWithContext(SQLContext)` method for compatibility [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #45727:
URL: https://github.com/apache/spark/pull/45727#issuecomment-2022059729
thanks, merging to master!
--
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
Re: [PR] [SPARK-46575][SQL][FOLLOWUP] Add back `HiveThriftServer2.startWithContext(SQLContext)` method for compatibility [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan closed pull request #45727: [SPARK-46575][SQL][FOLLOWUP] Add back `HiveThriftServer2.startWithContext(SQLContext)` method for compatibility
URL: https://github.com/apache/spark/pull/45727
--
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
Re: [PR] [SPARK-46575][SQL][FOLLOWUP] Add back `HiveThriftServer2.startWithContext(SQLContext)` method for compatibility [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #45727:
URL: https://github.com/apache/spark/pull/45727#discussion_r1540415445
##########
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala:
##########
@@ -75,6 +76,18 @@ object HiveThriftServer2 extends Logging {
server
}
+ /**
+ * :: DeveloperApi ::
+ * Starts a new thrift server with the given context.
+ *
+ * @param sqlContext SQLContext to use for the server
+ */
+ @Since("2.0.0")
+ @DeveloperApi
+ def startWithContext(sqlContext: SQLContext): HiveThriftServer2 = {
+ startWithContext(sqlContext, exitOnError)
Review 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
```
--
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
Re: [PR] [SPARK-46575][SQL][FOLLOWUP] Add back `HiveThriftServer2.startWithContext(SQLContext)` method for compatibility [spark]
Posted by "dragqueen95 (via GitHub)" <gi...@apache.org>.
dragqueen95 commented on code in PR #45727:
URL: https://github.com/apache/spark/pull/45727#discussion_r1540768811
##########
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala:
##########
@@ -75,6 +76,18 @@ object HiveThriftServer2 extends Logging {
server
}
+ /**
+ * :: DeveloperApi ::
+ * Starts a new thrift server with the given context.
+ *
+ * @param sqlContext SQLContext to use for the server
+ */
+ @Since("2.0.0")
+ @DeveloperApi
+ def startWithContext(sqlContext: SQLContext): HiveThriftServer2 = {
+ startWithContext(sqlContext, exitOnError)
Review Comment:
Thanks Wenchen
--
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
Re: [PR] [SPARK-46575][SQL][FOLLOWUP] Maintain backward/ forward compatibility for the DeveloperApi HiveThriftServer2.startWithContext [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #45727:
URL: https://github.com/apache/spark/pull/45727#issuecomment-2021008046
Also, cc @yaooqinn and @cloud-fan from
- https://github.com/apache/spark/pull/44575
--
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
Re: [PR] [SPARK-46575][SQL][FOLLOWUP] Add back `HiveThriftServer2.startWithContext(SQLContext)` method for compatibility [spark]
Posted by "yaooqinn (via GitHub)" <gi...@apache.org>.
yaooqinn commented on code in PR #45727:
URL: https://github.com/apache/spark/pull/45727#discussion_r1540656458
##########
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala:
##########
@@ -56,8 +56,9 @@ object HiveThriftServer2 extends Logging {
* 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")
Review Comment:
4.0.0?
--
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
Re: [PR] [SPARK-46575][SQL][FOLLOWUP] Add back `HiveThriftServer2.startWithContext(SQLContext)` method for compatibility [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #45727:
URL: https://github.com/apache/spark/pull/45727#issuecomment-2022008794
Thank you for the update, @cloud-fan .
--
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
Re: [PR] [SPARK-46575][SQL][FOLLOWUP] Add back `HiveThriftServer2.startWithContext(SQLContext)` method for compatibility [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #45727:
URL: https://github.com/apache/spark/pull/45727#discussion_r1540688320
##########
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala:
##########
@@ -56,8 +56,9 @@ object HiveThriftServer2 extends Logging {
* 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")
Review 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?
--
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
Re: [PR] [SPARK-46575][SQL][FOLLOWUP] Add back `HiveThriftServer2.startWithContext(SQLContext)` method for compatibility [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #45727:
URL: https://github.com/apache/spark/pull/45727#discussion_r1540687344
##########
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala:
##########
@@ -56,8 +56,9 @@ object HiveThriftServer2 extends Logging {
* 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")
Review Comment:
Oh, I thought https://github.com/apache/spark/pull/44575 was backported.
--
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
Re: [PR] [SPARK-46575][SQL][FOLLOWUP] Add back `HiveThriftServer2.startWithContext(SQLContext)` method for compatibility [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #45727:
URL: https://github.com/apache/spark/pull/45727#discussion_r1540416094
##########
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala:
##########
@@ -75,6 +76,18 @@ object HiveThriftServer2 extends Logging {
server
}
+ /**
+ * :: DeveloperApi ::
+ * Starts a new thrift server with the given context.
+ *
+ * @param sqlContext SQLContext to use for the server
+ */
+ @Since("2.0.0")
+ @DeveloperApi
+ def startWithContext(sqlContext: SQLContext): HiveThriftServer2 = {
+ startWithContext(sqlContext, exitOnError)
Review Comment:
`true` seems to be fine to me.
--
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
Re: [PR] [SPARK-46575][SQL][FOLLOWUP] Add back `HiveThriftServer2.startWithContext(SQLContext)` method for compatibility [spark]
Posted by "dragqueen95 (via GitHub)" <gi...@apache.org>.
dragqueen95 commented on code in PR #45727:
URL: https://github.com/apache/spark/pull/45727#discussion_r1540791542
##########
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala:
##########
@@ -56,8 +56,9 @@ object HiveThriftServer2 extends Logging {
* 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")
Review Comment:
Raised https://github.com/apache/spark/pull/45737
--
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
Re: [PR] [SPARK-46575][SQL][FOLLOWUP] Add back `HiveThriftServer2.startWithContext(SQLContext)` method for compatibility [spark]
Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on code in PR #45727:
URL: https://github.com/apache/spark/pull/45727#discussion_r1540415445
##########
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala:
##########
@@ -75,6 +76,18 @@ object HiveThriftServer2 extends Logging {
server
}
+ /**
+ * :: DeveloperApi ::
+ * Starts a new thrift server with the given context.
+ *
+ * @param sqlContext SQLContext to use for the server
+ */
+ @Since("2.0.0")
+ @DeveloperApi
+ def startWithContext(sqlContext: SQLContext): HiveThriftServer2 = {
+ startWithContext(sqlContext, exitOnError)
Review Comment:
Oh, could you remove `exitOnError`, @dragqueen95 ? This seems to be a typo.
```
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
```
--
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
Re: [PR] [SPARK-46575][SQL][FOLLOWUP] Add back `HiveThriftServer2.startWithContext(SQLContext)` method for compatibility [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #45727:
URL: https://github.com/apache/spark/pull/45727#discussion_r1540457438
##########
sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/HiveThriftServer2.scala:
##########
@@ -75,6 +76,18 @@ object HiveThriftServer2 extends Logging {
server
}
+ /**
+ * :: DeveloperApi ::
+ * Starts a new thrift server with the given context.
+ *
+ * @param sqlContext SQLContext to use for the server
+ */
+ @Since("2.0.0")
+ @DeveloperApi
+ def startWithContext(sqlContext: SQLContext): HiveThriftServer2 = {
+ startWithContext(sqlContext, exitOnError)
Review Comment:
```suggestion
startWithContext(sqlContext, exitOnError = true)
```
--
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