You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "AngersZhuuuu (via GitHub)" <gi...@apache.org> on 2023/03/07 11:11:33 UTC
[GitHub] [spark] AngersZhuuuu opened a new pull request, #40315: [SPARK-42699][CONNECTOR] SparkConnectServer should make client and AM same exit code
AngersZhuuuu opened a new pull request, #40315:
URL: https://github.com/apache/spark/pull/40315
### What changes were proposed in this pull request?
Since in https://github.com/apache/spark/pull/35594 we support pass a exit code to AM,
when SparkConnectServer exit with -1, need pass this to AM too.
### Why are the changes needed?
Keep same exit code
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
--
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
[GitHub] [spark] AngersZhuuuu commented on pull request #40315: [SPARK-42699][CONNECTOR] SparkConnectServer should make client and AM same exit code
Posted by "AngersZhuuuu (via GitHub)" <gi...@apache.org>.
AngersZhuuuu commented on PR #40315:
URL: https://github.com/apache/spark/pull/40315#issuecomment-1457984092
ping @HyukjinKwon
--
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
[GitHub] [spark] AngersZhuuuu commented on pull request #40315: [SPARK-42699][CONNECT] SparkConnectServer should make client and AM same exit code
Posted by "AngersZhuuuu (via GitHub)" <gi...@apache.org>.
AngersZhuuuu commented on PR #40315:
URL: https://github.com/apache/spark/pull/40315#issuecomment-1500104571
@amaliujia Like current? also ping @HyukjinKwon
--
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
[GitHub] [spark] AngersZhuuuu commented on pull request #40315: [SPARK-42699][CONNECT] SparkConnectServer should make client and AM same exit code
Posted by "AngersZhuuuu (via GitHub)" <gi...@apache.org>.
AngersZhuuuu commented on PR #40315:
URL: https://github.com/apache/spark/pull/40315#issuecomment-1461590509
ping @HyukjinKwon
--
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
[GitHub] [spark] hvanhovell commented on a diff in pull request #40315: [SPARK-42699][CONNECTOR] SparkConnectServer should make client and AM same exit code
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #40315:
URL: https://github.com/apache/spark/pull/40315#discussion_r1127848561
##########
sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala:
##########
@@ -736,13 +736,15 @@ class SparkSession private(
}
// scalastyle:on
+ def stop(): Unit = stop(0)
+
/**
* Stop the underlying `SparkContext`.
*
* @since 2.0.0
*/
- def stop(): Unit = {
- sparkContext.stop()
+ def stop(exitCode: Int = 0): Unit = {
Review Comment:
@AngersZhuuuu this is a breaking change. You will need to define two stop methods, one with the status code, and one without.
--
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
[GitHub] [spark] github-actions[bot] commented on pull request #40315: [SPARK-42699][CONNECT] SparkConnectServer should make client and AM same exit code
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #40315:
URL: https://github.com/apache/spark/pull/40315#issuecomment-1637231272
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!
--
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
[GitHub] [spark] amaliujia commented on a diff in pull request #40315: [SPARK-42699][CONNECT] SparkConnectServer should make client and AM same exit code
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on code in PR #40315:
URL: https://github.com/apache/spark/pull/40315#discussion_r1131780390
##########
sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala:
##########
@@ -737,12 +737,19 @@ class SparkSession private(
// scalastyle:on
/**
- * Stop the underlying `SparkContext`.
+ * Stop the underlying `SparkContext` with default exit code 0.
*
* @since 2.0.0
*/
- def stop(): Unit = {
- sparkContext.stop()
+ def stop(): Unit = stop(0)
Review Comment:
I think it is better to not change this line. This line builds on `sparkContext.stop()`. Even though the underlying implementation is `SparkContext.stop(0)`, I think we'd better do not make that assumption that it will always be.
--
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
[GitHub] [spark] AngersZhuuuu commented on a diff in pull request #40315: [SPARK-42699][CONNECT] SparkConnectServer should make client and AM same exit code
Posted by "AngersZhuuuu (via GitHub)" <gi...@apache.org>.
AngersZhuuuu commented on code in PR #40315:
URL: https://github.com/apache/spark/pull/40315#discussion_r1128888640
##########
sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala:
##########
@@ -736,13 +736,15 @@ class SparkSession private(
}
// scalastyle:on
+ def stop(): Unit = stop(0)
+
/**
* Stop the underlying `SparkContext`.
*
* @since 2.0.0
Review Comment:
How about current?
--
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
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #40315: [SPARK-42699][CONNECT] SparkConnectServer should make client and AM same exit code
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #40315:
URL: https://github.com/apache/spark/pull/40315#discussion_r1128822955
##########
sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala:
##########
@@ -736,13 +736,15 @@ class SparkSession private(
}
// scalastyle:on
+ def stop(): Unit = stop(0)
+
/**
* Stop the underlying `SparkContext`.
*
* @since 2.0.0
*/
- def stop(): Unit = {
- sparkContext.stop()
+ def stop(exitCode: Int = 0): Unit = {
Review Comment:
Can you remove the default value here?
--
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
[GitHub] [spark] HyukjinKwon commented on a diff in pull request #40315: [SPARK-42699][CONNECT] SparkConnectServer should make client and AM same exit code
Posted by "HyukjinKwon (via GitHub)" <gi...@apache.org>.
HyukjinKwon commented on code in PR #40315:
URL: https://github.com/apache/spark/pull/40315#discussion_r1128823249
##########
sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala:
##########
@@ -736,13 +736,15 @@ class SparkSession private(
}
// scalastyle:on
+ def stop(): Unit = stop(0)
+
/**
* Stop the underlying `SparkContext`.
*
* @since 2.0.0
Review Comment:
You should have the new docs for the one for `exitCode` with new `@since`
--
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
[GitHub] [spark] amaliujia commented on pull request #40315: [SPARK-42699][CONNECT] SparkConnectServer should make client and AM same exit code
Posted by "amaliujia (via GitHub)" <gi...@apache.org>.
amaliujia commented on PR #40315:
URL: https://github.com/apache/spark/pull/40315#issuecomment-1500531162
LGTM
--
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
[GitHub] [spark] github-actions[bot] closed pull request #40315: [SPARK-42699][CONNECT] SparkConnectServer should make client and AM same exit code
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #40315: [SPARK-42699][CONNECT] SparkConnectServer should make client and AM same exit code
URL: https://github.com/apache/spark/pull/40315
--
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