[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[asfgit]

Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/10289


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[srowen]

Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-167114060
  
Merged to master


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166856069
  
**[Test build #2251 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2251/consoleFull)**
 for PR 10289 at commit 
[`cb6a862`](https://github.com/apache/spark/commit/cb6a8626e9c29d8b4a1f4bd2f0163784cf306bb5).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166878686
  
**[Test build #2251 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2251/consoleFull)**
 for PR 10289 at commit 
[`cb6a862`](https://github.com/apache/spark/commit/cb6a8626e9c29d8b4a1f4bd2f0163784cf306bb5).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:\n  * 
`class SecurityManagerSuite extends SparkFunSuite with ResetSystemProperties `\n


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[srowen]

Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166881220
  
I'm going to go ahead and merge this today


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166644123
  
**[Test build #48202 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48202/consoleFull)**
 for PR 10289 at commit 
[`cb6a862`](https://github.com/apache/spark/commit/cb6a8626e9c29d8b4a1f4bd2f0163784cf306bb5).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166709517
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48202/
Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166709431
  
**[Test build #48202 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48202/consoleFull)**
 for PR 10289 at commit 
[`cb6a862`](https://github.com/apache/spark/commit/cb6a8626e9c29d8b4a1f4bd2f0163784cf306bb5).
 * This patch **fails from timeout after a configured wait of \`250m\`**.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:\n  * 
`class SecurityManagerSuite extends SparkFunSuite with ResetSystemProperties `\n


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166709515
  
Merged build finished. Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[kiszk]

Github user kiszk commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166765714
  
Can I retest this?  Timeout may occur in pyspark.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[kiszk]

Github user kiszk commented on a diff in the pull request:

https://github.com/apache/spark/pull/10289#discussion_r48238449
  
--- Diff: 
yarn/src/test/scala/org/apache/spark/deploy/yarn/BaseYarnClusterSuite.scala ---
@@ -116,7 +120,7 @@ abstract class BaseYarnClusterSuite
 
   override def afterAll() {
 yarnCluster.stop()
-System.clearProperty("SPARK_YARN_MODE")
+System.setProperties(oldProperties)
 super.afterAll()
--- End diff --

I see, I will use `try finally` here


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[kiszk]

Github user kiszk commented on a diff in the pull request:

https://github.com/apache/spark/pull/10289#discussion_r48238546
  
--- Diff: 
yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala ---
@@ -43,12 +45,21 @@ import org.apache.spark.util.Utils
 
 class ClientSuite extends SparkFunSuite with Matchers with 
BeforeAndAfterAll {
 
+  var oldProperties: Properties = null
--- End diff --

Sorry, I made a mistake when I check all of the changes in this file. I 
will use `ResetSystemProperties` here.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/10289#discussion_r48234885
  
--- Diff: 
yarn/src/test/scala/org/apache/spark/deploy/yarn/BaseYarnClusterSuite.scala ---
@@ -116,7 +120,7 @@ abstract class BaseYarnClusterSuite
 
   override def afterAll() {
 yarnCluster.stop()
-System.clearProperty("SPARK_YARN_MODE")
+System.setProperties(oldProperties)
 super.afterAll()
--- End diff --

Yes, your change is adding this call though right? Ok to do it here too


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/10289#discussion_r48234838
  
--- Diff: 
yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala ---
@@ -43,12 +45,21 @@ import org.apache.spark.util.Utils
 
 class ClientSuite extends SparkFunSuite with Matchers with 
BeforeAndAfterAll {
 
+  var oldProperties: Properties = null
--- End diff --

That's a fair point -- yes it's called two lines below, right? I think you 
could use the standard mechanism in the 2 places you're manually copying the 
sys properties?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[andrewor14]

Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166388741
  
Yeah, this looks fine. I wonder if there's something safer that we can do 
in the future. What I want is the following but I don't think scalastyle is 
powerful enough to support it:
```
// Allow
override def beforeAll(): Unit = {
  super.beforeAll()
  ...
}

// Disallow
override def beforeAll(): Unit = {
  // no super.beforeAll() here
  ...
}
```


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[andrewor14]

Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166440250
  
Hm, I suppose we could do something in `SparkFunSuite`:

```
final def beforeAll(): Unit = {
  super.beforeAll()
  doBeforeAll()
}

protected override def doBeforeAll(): Unit
```
and then we'll use scalastyle to ban direct usages of `BeforeAndAfterAll`. 
Would that work? (I'm suggesting for a future patch)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[srowen]

Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166441235
  
Yeah, though then you can't fix this for test classes that extend test 
classes... not sure if that exists today but could. My gut is that it's not 
worth it but yes that could help too.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[andrewor14]

Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/10289#discussion_r48200103
  
--- Diff: 
yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala ---
@@ -43,12 +45,21 @@ import org.apache.spark.util.Utils
 
 class ClientSuite extends SparkFunSuite with Matchers with 
BeforeAndAfterAll {
 
+  var oldProperties: Properties = null
--- End diff --

why not use `ResetSystemProperties` here?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[andrewor14]

Github user andrewor14 commented on a diff in the pull request:

https://github.com/apache/spark/pull/10289#discussion_r48199984
  
--- Diff: 
yarn/src/test/scala/org/apache/spark/deploy/yarn/BaseYarnClusterSuite.scala ---
@@ -116,7 +120,7 @@ abstract class BaseYarnClusterSuite
 
   override def afterAll() {
 yarnCluster.stop()
-System.clearProperty("SPARK_YARN_MODE")
+System.setProperties(oldProperties)
 super.afterAll()
--- End diff --

this one needs to be in `try finally`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[srowen]

Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166439578
  
I suppose the superclass could catch the case where beforeAll wasn't called 
but afterAll was, but not the latter. I also don't know how to implement this 
without significant cleverness, which doesn't seem worth it.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[kiszk]

Github user kiszk commented on a diff in the pull request:

https://github.com/apache/spark/pull/10289#discussion_r48209450
  
--- Diff: 
yarn/src/test/scala/org/apache/spark/deploy/yarn/ClientSuite.scala ---
@@ -43,12 +45,21 @@ import org.apache.spark.util.Utils
 
 class ClientSuite extends SparkFunSuite with Matchers with 
BeforeAndAfterAll {
 
+  var oldProperties: Properties = null
--- End diff --

This is because System.setProperty() is not used in this file.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[kiszk]

Github user kiszk commented on a diff in the pull request:

https://github.com/apache/spark/pull/10289#discussion_r48209783
  
--- Diff: 
yarn/src/test/scala/org/apache/spark/deploy/yarn/BaseYarnClusterSuite.scala ---
@@ -116,7 +120,7 @@ abstract class BaseYarnClusterSuite
 
   override def afterAll() {
 yarnCluster.stop()
-System.clearProperty("SPARK_YARN_MODE")
+System.setProperties(oldProperties)
 super.afterAll()
--- End diff --

I agree. Since the original code did not use `try finally`, I did not use 
it, too. Should I use `try finally`?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[srowen]

Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166280174
  
OK, this looks good to me. It's fixing the original problem and fixing 
potential problems of the same form elsewhere.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166118389
  
**[Test build #48072 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48072/consoleFull)**
 for PR 10289 at commit 
[`43d1854`](https://github.com/apache/spark/commit/43d185493c8639a39dbdd54b54f8fe1285a89364).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166131447
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166131449
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48072/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-166131397
  
**[Test build #48072 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48072/consoleFull)**
 for PR 10289 at commit 
[`43d1854`](https://github.com/apache/spark/commit/43d185493c8639a39dbdd54b54f8fe1285a89364).
 * 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[kiszk]

Github user kiszk commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165960572
  
I will fix this merge issue this weekend.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165721057
  
**[Test build #47995 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47995/consoleFull)**
 for PR 10289 at commit 
[`49b15f8`](https://github.com/apache/spark/commit/49b15f8745eee783b6fe6cafc476e057466c9962).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165748813
  
Test PASSed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47995/
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165748810
  
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165748707
  
**[Test build #47995 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47995/consoleFull)**
 for PR 10289 at commit 
[`49b15f8`](https://github.com/apache/spark/commit/49b15f8745eee783b6fe6cafc476e057466c9962).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:\n  * 
`class SecurityManagerSuite extends SparkFunSuite with ResetSystemProperties `\n


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[srowen]

Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165800366
  
@kiszk your last merge to this branch seems to have gone wrong. This has a 
lot of unrelated changes now. Can you try backing out the bad commits and force 
pushing? or getting the right commits to a new PR?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165947512
  
**[Test build #48047 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48047/consoleFull)**
 for PR 10289 at commit 
[`7be29b1`](https://github.com/apache/spark/commit/7be29b179dc5a518e48372658ee965d3292b3021).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165947641
  
Build finished. Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165947639
  
**[Test build #48047 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48047/consoleFull)**
 for PR 10289 at commit 
[`7be29b1`](https://github.com/apache/spark/commit/7be29b179dc5a518e48372658ee965d3292b3021).
 * This patch **fails R style tests**.
 * This patch **does not merge cleanly**.
 * This patch adds the following public classes _(experimental)_:\n  * 
`class SecurityManagerSuite extends SparkFunSuite with ResetSystemProperties 
`\n  * `class CheckpointInputDStream(ssc_ : StreamingContext) extends 
InputDStream[Int](ssc_) `\n  * `  class FileInputDStreamCheckpointData extends 
DStreamCheckpointData(this) `\n


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165947642
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/48047/
Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[kiszk]

Github user kiszk commented on a diff in the pull request:

https://github.com/apache/spark/pull/10289#discussion_r47920442
  
--- Diff: 
core/src/test/scala/org/apache/spark/util/collection/ExternalSorterSuite.scala 
---
@@ -235,7 +235,7 @@ class ExternalSorterSuite extends SparkFunSuite with 
LocalSparkContext {
   it.next()
 }
   }
-
+*/
--- End diff --

Sorry, this is my mistake. This file should not be modified.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/10289#discussion_r47917288
  
--- Diff: 
core/src/test/scala/org/apache/spark/util/collection/ExternalSorterSuite.scala 
---
@@ -235,7 +235,7 @@ class ExternalSorterSuite extends SparkFunSuite with 
LocalSparkContext {
   it.next()
 }
   }
-
+*/
--- End diff --

Why is this big section commented out?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/10289#discussion_r47917308
  
--- Diff: 
mllib/src/test/scala/org/apache/spark/mllib/util/MLlibTestSparkContext.scala ---
@@ -38,12 +38,15 @@ trait MLlibTestSparkContext extends BeforeAndAfterAll { 
self: Suite =>
   }
 
   override def afterAll() {
-sqlContext = null
-SQLContext.clearActive()
-if (sc != null) {
-  sc.stop()
-}
+try {
+  sqlContext = null
+  SQLContext.clearActive()
+  if (sc != null) {
+sc.stop()
+  }
 sc = null
-super.afterAll()
+} finally {
--- End diff --

I think the line above needs an indent?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[kiszk]

Github user kiszk commented on a diff in the pull request:

https://github.com/apache/spark/pull/10289#discussion_r47922285
  
--- Diff: 
mllib/src/test/scala/org/apache/spark/mllib/util/MLlibTestSparkContext.scala ---
@@ -38,12 +38,15 @@ trait MLlibTestSparkContext extends BeforeAndAfterAll { 
self: Suite =>
   }
 
   override def afterAll() {
-sqlContext = null
-SQLContext.clearActive()
-if (sc != null) {
-  sc.stop()
-}
+try {
+  sqlContext = null
+  SQLContext.clearActive()
+  if (sc != null) {
+sc.stop()
+  }
 sc = null
-super.afterAll()
+} finally {
--- End diff --

Yes, I will insert two spaces for an indent


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[kiszk]

Github user kiszk commented on a diff in the pull request:

https://github.com/apache/spark/pull/10289#discussion_r47920869
  
--- Diff: 
yarn/src/test/scala/org/apache/spark/deploy/yarn/BaseYarnClusterSuite.scala ---
@@ -116,7 +120,7 @@ abstract class BaseYarnClusterSuite
 
   override def afterAll() {
 yarnCluster.stop()
-System.clearProperty("SPARK_YARN_MODE")
+System.setProperties(oldProperties)
--- End diff --

I am afraid that another property may be changes somewhere. I will use the 
original code since it works well 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[srowen]

Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165482323
  
@kiszk I think it's looking good, minus some style problem that's making 
the build fail, and a few minor comments. I think it's pretty nice to clean up 
the original bug but also standardize and correct the behavior of these test 
lifecycle methods. It's worth a change that touches lots of files.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/10289#discussion_r47917486
  
--- Diff: 
yarn/src/test/scala/org/apache/spark/deploy/yarn/BaseYarnClusterSuite.scala ---
@@ -116,7 +120,7 @@ abstract class BaseYarnClusterSuite
 
   override def afterAll() {
 yarnCluster.stop()
-System.clearProperty("SPARK_YARN_MODE")
+System.setProperties(oldProperties)
--- End diff --

This changes the logic of the test a bit; is this needed?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[srowen]

Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/10289#discussion_r47922105
  
--- Diff: 
yarn/src/test/scala/org/apache/spark/deploy/yarn/BaseYarnClusterSuite.scala ---
@@ -116,7 +120,7 @@ abstract class BaseYarnClusterSuite
 
   override def afterAll() {
 yarnCluster.stop()
-System.clearProperty("SPARK_YARN_MODE")
+System.setProperties(oldProperties)
--- End diff --

It's probably a decent idea. I think you can leave the change.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165521411
  
Merged build finished. Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165523301
  
**[Test build #2224 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2224/consoleFull)**
 for PR 10289 at commit 
[`9b99df9`](https://github.com/apache/spark/commit/9b99df973f707e42e83926bfaf2f71a03538e65d).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165521415
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47933/
Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165566493
  
**[Test build #2224 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/2224/consoleFull)**
 for PR 10289 at commit 
[`9b99df9`](https://github.com/apache/spark/commit/9b99df973f707e42e83926bfaf2f71a03538e65d).
 * This patch passes all tests.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:\n  * 
`class SecurityManagerSuite extends SparkFunSuite with ResetSystemProperties `\n


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165319314
  
**[Test build #47884 has 
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47884/consoleFull)**
 for PR 10289 at commit 
[`b6deb2f`](https://github.com/apache/spark/commit/b6deb2f59ad2245847cc15fefae8697649618cb9).
 * This patch **fails Scala style tests**.
 * This patch merges cleanly.
 * This patch adds the following public classes _(experimental)_:\n  * 
`class SecurityManagerSuite extends SparkFunSuite with ResetSystemProperties `\n


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[SparkQA]

Github user SparkQA commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165319124
  
**[Test build #47884 has 
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47884/consoleFull)**
 for PR 10289 at commit 
[`b6deb2f`](https://github.com/apache/spark/commit/b6deb2f59ad2245847cc15fefae8697649618cb9).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165319317
  
Merged build finished. Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165319320
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47884/
Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165330330
  
Merged build finished. Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165330331
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47894/
Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[andrewor14]

Github user andrewor14 commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165317600
  
ok to test


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165328555
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47893/
Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165328554
  
Merged build finished. Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165331170
  
Merged build finished. Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-165331171
  
Test FAILed.
Refer to this link for build results (access rights to CI server needed): 
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/47895/
Test FAILed.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[3ourroom]

Github user 3ourroom commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-164742037
  

NAVER - http://www.naver.com/


3ourr...@naver.com 님께 보내신 메일  이 다음과 같은 이유
로 전송 실패했습니다.



받는 사람이 회원님의 메일을 수신차단 하였습니다. 






---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[srowen]

Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-164740892
  
@holdenk yeah I think that explains the problem in `BlockManagerSuite`, 
good catch. I'm not sure why the other one is a problem though; not overriding 
`afterEach` is fine as it means the superclass method does execute. Maybe that 
one isn't actually a problem @kiszk?

The general form of these overrides should be to run superclass init first, 
and superclass cleanup after:

```
override def beforeEach(): Unit = {
  super.beforeEach()
  ... init here ...
}

override def afterEach(): Unit = {
  try {
... cleanup here ...
  } finally {
super.afterEach()
  }
}
```

Same for `beforeAll()` and `afterAll()`. _The `try-finally` isn't necessary 
if the cleanup really can't generate an exception._

We have a number of instances where this isn't done quite right across the 
code base,. AFAICT most don't matter. It's not that many though. @kiszk what do 
you think about correcting this pattern everywhere to head off potential bugs 
of this form later?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[kiszk]

Github user kiszk commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-164779119
  
@srowen, OK, I will correct this pattern everywhere. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[kiszk]

Github user kiszk commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-164802842
  
There are two potential bugs
1. A method of the super class is not called in `beforeEach()`, 
`afterEach()`, `beforeAll()`, and `afterAll()`
2. Although `System.setProperty()` is called, `ResetSystemProperties` is 
not mixed-in. 
I will correct them


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[holdenk]

Github user holdenk commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-164632168
  
@JoshRosen neither of those suites override the afterAll , but neither of 
them call the super's afterEach function (which they both override and that is 
where the ResetSystemProperties trait has it hook).


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[srowen]

Github user srowen commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-164442285
  
Ah, I knew I was forgetting something. I agree this should already be 
handled. @kiszk are you sure it's actually not working or just thought it 
looked like it didn't?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[JoshRosen]

Github user JoshRosen commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-164373129
  
I think that this was supposed to be handled by the `ResetSystemProperties` 
mixin; do either of these suites fail to call the superclass's `afterEach()` or 
`afterAll()` methods, by chance?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[AmplabJenkins]

Github user AmplabJenkins commented on the pull request:

https://github.com/apache/spark/pull/10289#issuecomment-164369953
  
Can one of the admins verify this patch?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org

[GitHub] spark pull request: [SPARK-12311][CORE] Restore previous value of ...

[kiszk]

GitHub user kiszk opened a pull request:

https://github.com/apache/spark/pull/10289

[SPARK-12311][CORE] Restore previous value of "os.arch" property in test 
suites after forcing to set specific value to "os.arch" property

Restore the original value of os.arch property after each test

Since some of tests forced to set the specific value to os.arch property, 
we need to set the original value.

You can merge this pull request into a Git repository by running:

$ git pull https://github.com/kiszk/spark SPARK-12311

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/10289.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 #10289


commit df0848c94e91333d398b6f793d82c2f2f80d07a0
Author: Kazuaki Ishizaki 
Date:   2015-12-14T07:32:52Z

restore the original value of os.arch property after each test

Since some of tests forced to set the specific value to os.arch property, 
we need to set the original value.




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org