[GitHub] spark issue #19802: [SPARK-22594][CORE] Handling spark-submit and master ver...
Github user vanzin commented on the issue: https://github.com/apache/spark/pull/19802 @Jiri-Kremser if you're not planning to address the feedback you should probably close this PR. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19802: [SPARK-22594][CORE] Handling spark-submit and master ver...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19802 Can one of the admins verify this patch? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19802: [SPARK-22594][CORE] Handling spark-submit and master ver...
Github user miacobv commented on the issue: https://github.com/apache/spark/pull/19802 I get this when I start the master and worker, without running spark-submit on both 2.2.0 and 2.2.1 --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19802: [SPARK-22594][CORE] Handling spark-submit and master ver...
Github user vanzin commented on the issue: https://github.com/apache/spark/pull/19802 I think this is the wrong place for the fix, if one is desired. The transport library doesn't know anything about serialization, that's all handled by the code using it. In this case, the 'RpcEnv' stuff in core. So I don't think it's correct to handle those errors in the transport library. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19802: [SPARK-22594][CORE] Handling spark-submit and master ver...
Github user srowen commented on the issue: https://github.com/apache/spark/pull/19802 @vanzin do you have thoughts on this one? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19802: [SPARK-22594][CORE] Handling spark-submit and master ver...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19802 **[Test build #4086 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4086/testReport)** for PR 19802 at commit [`4f79632`](https://github.com/apache/spark/commit/4f79632d22b67128a6be8a285f4fc1fec0d5f12f). * This patch passes all tests. * This patch merges cleanly. * This patch adds no public classes. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19802: [SPARK-22594][CORE] Handling spark-submit and master ver...
Github user SparkQA commented on the issue: https://github.com/apache/spark/pull/19802 **[Test build #4086 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/4086/testReport)** for PR 19802 at commit [`4f79632`](https://github.com/apache/spark/commit/4f79632d22b67128a6be8a285f4fc1fec0d5f12f). --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19802: [SPARK-22594][CORE] Handling spark-submit and master ver...
Github user Jiri-Kremser commented on the issue: https://github.com/apache/spark/pull/19802 @srowen was Jenkins ok with the change? I can't see the results. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19802: [SPARK-22594][CORE] Handling spark-submit and master ver...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19802 Can one of the admins verify this patch? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19802: [SPARK-22594][CORE] Handling spark-submit and master ver...
Github user AmplabJenkins commented on the issue: https://github.com/apache/spark/pull/19802 Can one of the admins verify this patch? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19802: [SPARK-22594][CORE] Handling spark-submit and master ver...
Github user srowen commented on the issue: https://github.com/apache/spark/pull/19802 I see, yeah that's a fine point. Don't rethrow then. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19802: [SPARK-22594][CORE] Handling spark-submit and master ver...
Github user Jiri-Kremser commented on the issue:
https://github.com/apache/spark/pull/19802
> I think it's probably OK if you also re-throw that exception for now.
You're just giving more info then.
Hmm, In the original code the exception wasn't re-thrown. Do you really
think it is a good idea? The `InvalidClassException` is a checked exception so
the signature of `processOneWayMessage()` method would need to be change as
well.
this is the original catch-them-all code :)
```java
} catch (Exception e) {
logger.error("Error while invoking RpcHandler#receive() for one-way
message.", e);
}
```
I think, I addressed all your inline other comments, thanks for the review.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19802: [SPARK-22594][CORE] Handling spark-submit and master ver...
Github user Jiri-Kremser commented on the issue: https://github.com/apache/spark/pull/19802 @srowen is there anything I can improve in the PR? --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark issue #19802: [SPARK-22594][CORE] Handling spark-submit and master ver...
Github user Jiri-Kremser commented on the issue: https://github.com/apache/spark/pull/19802 I've also added tests and removed "WIP" from the label? Do you guys want the change or would you like to have something more elaborated, like an initial handshake? One note about the "handshake approach". If there is new initial message that would be sent from client to server in which the client sends its version and server decides if it's backward compatible or not with this particular client version, this approach itself wouldn't solve the issue when old client (that doesn't know anything about it) tries to talk with new server. This PR solves this scenario. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
