You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by keypointt <gi...@git.apache.org> on 2016/04/27 23:54:45 UTC
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
GitHub user keypointt opened a pull request:
https://github.com/apache/spark/pull/12744
[SPARK-14935][CORE] DistributedSuite "local-cluster format" shouldn't actually launch clusters
https://issues.apache.org/jira/browse/SPARK-14935
## What changes were proposed in this pull request?
In DistributedSuite, the "local-cluster format" test actually launches a bunch of clusters, but this doesn't seem necessary for what should just be a unit test of a regex. We should clean up the code so that this is testable without actually launching a cluster, which should buy us about 20 seconds per build.
## How was this patch tested?
Passed unit test on my local machine
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/keypointt/spark SPARK-14935
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12744.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #12744
----
commit 075a749b0b0ff5c951d6b61cba3f5406fe063d7f
Author: Xin Ren <ia...@126.com>
Date: 2016-04-27T21:49:00Z
[SPARK-14935] check local-cluster master name with regex, no cluster launching
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by keypointt <gi...@git.apache.org>.
Github user keypointt commented on a diff in the pull request:
https://github.com/apache/spark/pull/12744#discussion_r61365044
--- Diff: core/src/test/scala/org/apache/spark/DistributedSuite.scala ---
@@ -51,18 +51,21 @@ class DistributedSuite extends SparkFunSuite with Matchers with LocalSparkContex
}
test("local-cluster format") {
- sc = new SparkContext("local-cluster[2,1,1024]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
- sc = new SparkContext("local-cluster[2 , 1 , 1024]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
- sc = new SparkContext("local-cluster[2, 1, 1024]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
- sc = new SparkContext("local-cluster[ 2, 1, 1024 ]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
+ import SparkMasterRegex._
+
+ val masterStrings = Seq(
+ "local-cluster[2,1,1024]",
+ "local-cluster[2 , 1 , 1024]",
+ "local-cluster[2, 1, 1024]",
+ "local-cluster[ 2, 1, 1024 ]"
+ )
+
+ masterStrings.map(x => x match {
--- End diff --
oh yes you are right, fixing it now
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/12744#discussion_r61470214
--- Diff: core/src/test/scala/org/apache/spark/DistributedSuite.scala ---
@@ -51,18 +51,21 @@ class DistributedSuite extends SparkFunSuite with Matchers with LocalSparkContex
}
test("local-cluster format") {
- sc = new SparkContext("local-cluster[2,1,1024]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
- sc = new SparkContext("local-cluster[2 , 1 , 1024]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
- sc = new SparkContext("local-cluster[2, 1, 1024]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
- sc = new SparkContext("local-cluster[ 2, 1, 1024 ]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
+ import SparkMasterRegex._
+
+ val masterStrings = Seq(
+ "local-cluster[2,1,1024]",
+ "local-cluster[2 , 1 , 1024]",
+ "local-cluster[2, 1, 1024]",
+ "local-cluster[ 2, 1, 1024 ]"
+ )
+
+ masterStrings.foreach(x => x match {
--- End diff --
There was a second part to my earlier style suggestion:
You don't need the extra set of redundant braces in order to be able to do a match here. Instead, you could just write
```
masterStrings.foreach {
case LOCAL_CLUSTER_REGEX(numSlaves, coresPerSlave, memoryPerSlave) =>
..
}
```
Sorry for being picky about this, but I'd rather get it as clean as possible now instead of having to deal with patches suggesting cleanup later. I'll just fix this myself on merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12744#issuecomment-215280135
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57190/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12744#issuecomment-215268169
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57180/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/12744#discussion_r61350681
--- Diff: core/src/test/scala/org/apache/spark/DistributedSuite.scala ---
@@ -17,6 +17,7 @@
package org.apache.spark
+import SparkMasterRegex._
--- End diff --
I'd move this import inside of the `test("local-cluster format")` to avoid complaints about the import ordering. Otherwise, this looks good to me. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12744#issuecomment-215304423
**[Test build #57202 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57202/consoleFull)** for PR 12744 at commit [`0f14ad2`](https://github.com/apache/spark/commit/0f14ad2698bb9b2fbf582c44e3bb8f796b873750).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12744#issuecomment-215261144
**[Test build #57190 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57190/consoleFull)** for PR 12744 at commit [`d2ec609`](https://github.com/apache/spark/commit/d2ec609b40b457a8989f87026543def05b532e51).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12744#issuecomment-215288071
**[Test build #57202 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57202/consoleFull)** for PR 12744 at commit [`0f14ad2`](https://github.com/apache/spark/commit/0f14ad2698bb9b2fbf582c44e3bb8f796b873750).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12744#issuecomment-215304507
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57202/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12744#issuecomment-215268166
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12744#issuecomment-215280131
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12744#issuecomment-215242387
**[Test build #57180 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57180/consoleFull)** for PR 12744 at commit [`075a749`](https://github.com/apache/spark/commit/075a749b0b0ff5c951d6b61cba3f5406fe063d7f).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12744#issuecomment-215280010
**[Test build #57190 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57190/consoleFull)** for PR 12744 at commit [`d2ec609`](https://github.com/apache/spark/commit/d2ec609b40b457a8989f87026543def05b532e51).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by JoshRosen <gi...@git.apache.org>.
Github user JoshRosen commented on a diff in the pull request:
https://github.com/apache/spark/pull/12744#discussion_r61362822
--- Diff: core/src/test/scala/org/apache/spark/DistributedSuite.scala ---
@@ -51,18 +51,21 @@ class DistributedSuite extends SparkFunSuite with Matchers with LocalSparkContex
}
test("local-cluster format") {
- sc = new SparkContext("local-cluster[2,1,1024]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
- sc = new SparkContext("local-cluster[2 , 1 , 1024]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
- sc = new SparkContext("local-cluster[2, 1, 1024]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
- sc = new SparkContext("local-cluster[ 2, 1, 1024 ]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
+ import SparkMasterRegex._
+
+ val masterStrings = Seq(
+ "local-cluster[2,1,1024]",
+ "local-cluster[2 , 1 , 1024]",
+ "local-cluster[2, 1, 1024]",
+ "local-cluster[ 2, 1, 1024 ]"
+ )
+
+ masterStrings.map(x => x match {
--- End diff --
Actually, spotted another style problem: this should be `forEach` and not `map` since it's being executed only for side-effects.
You can just write
```
masterStrings.forEach {
case ...
}
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12744#issuecomment-215304506
Merged build finished. Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12744
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by keypointt <gi...@git.apache.org>.
Github user keypointt commented on the pull request:
https://github.com/apache/spark/pull/12744#issuecomment-215510212
Thanks a lot for the review
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by keypointt <gi...@git.apache.org>.
Github user keypointt commented on a diff in the pull request:
https://github.com/apache/spark/pull/12744#discussion_r61472405
--- Diff: core/src/test/scala/org/apache/spark/DistributedSuite.scala ---
@@ -51,18 +51,21 @@ class DistributedSuite extends SparkFunSuite with Matchers with LocalSparkContex
}
test("local-cluster format") {
- sc = new SparkContext("local-cluster[2,1,1024]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
- sc = new SparkContext("local-cluster[2 , 1 , 1024]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
- sc = new SparkContext("local-cluster[2, 1, 1024]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
- sc = new SparkContext("local-cluster[ 2, 1, 1024 ]", "test")
- assert(sc.parallelize(1 to 2, 2).count() == 2)
- resetSparkContext()
+ import SparkMasterRegex._
+
+ val masterStrings = Seq(
+ "local-cluster[2,1,1024]",
+ "local-cluster[2 , 1 , 1024]",
+ "local-cluster[2, 1, 1024]",
+ "local-cluster[ 2, 1, 1024 ]"
+ )
+
+ masterStrings.foreach(x => x match {
--- End diff --
Oh sorry my fault, fixing it now :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14935][CORE] DistributedSuite "local-cl...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12744#issuecomment-215268021
**[Test build #57180 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57180/consoleFull)** for PR 12744 at commit [`075a749`](https://github.com/apache/spark/commit/075a749b0b0ff5c951d6b61cba3f5406fe063d7f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org