You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by felixcheung <gi...@git.apache.org> on 2017/01/27 19:12:13 UTC
[GitHub] spark pull request #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
GitHub user felixcheung opened a pull request:
https://github.com/apache/spark/pull/16720
[SPARK-19387][SPARKR] Tests do not run with SparkR source package in CRAN check
## What changes were proposed in this pull request?
- this is cause by changes in SPARK-18444, SPARK-18643 that we no longer install Spark when `master = ""` (default), but also related to SPARK-18449 since the real `master` value is not known at the time the R code in `sparkR.session` is run. (`master` cannot default to "local" since it could be overridden by spark-submit commandline or spark config)
- as a result, while running SparkR as a package in IDE is working fine, CRAN check is not as it is launching it via non-interactive script
-
## How was this patch tested?
Tested this by reverting version to 2.1, since it needs to download the release jar with matching version. But since there are changes in 2.2 (specifically around SparkR ML) that are incompatible with 2.1, some tests are failing in this config. Will need to port this to branch-2.1 and retest with 2.1 release jar.
manually as:
```
# modify DESCRIPTION to revert version to 2.1.0
SPARK_HOME=/usr/spark R CMD build pkg
R CMD check --as-cran SparkR_2.1.0.tar.gz
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/felixcheung/spark rcranchecktest
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/16720.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 #16720
----
commit 318ecc876c64b7085df46618278861c2ea9ff422
Author: Felix Cheung <fe...@hotmail.com>
Date: 2017-01-27T19:03:15Z
make sure Spark is installed
----
---
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 #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:
https://github.com/apache/spark/pull/16720#discussion_r98833756
--- Diff: R/pkg/vignettes/sparkr-vignettes.Rmd ---
@@ -27,6 +27,9 @@ library(SparkR)
We use default settings in which it runs in local mode. It auto downloads Spark package in the background if no previous installation is found. For more details about setup, see [Spark Session](#SetupSparkSession).
+```{r, include=FALSE}
+SparkR:::sparkCheckInstall()
--- End diff --
Is it ok to include a `:::` function in the vignette ?
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16720
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72103/
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/16720
yes, that is described in the PR description:
"
fix is to add check to the beginning of each test and vignettes; the same would also work by changing `sparkR.session()` to `sparkR.session(master = "local")` in tests, but I think being more explicit is better.
"
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16720
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 #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/16720#discussion_r98837222
--- Diff: R/pkg/inst/tests/testthat/test_utils.R ---
@@ -17,6 +17,9 @@
context("functions in utils.R")
+# Ensure Spark is installed
+sparkCheckInstall()
--- End diff --
hmm, we could put it in https://github.com/apache/spark/blob/master/R/pkg/tests/run-all.R?
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16720
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 #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:
https://github.com/apache/spark/pull/16720#discussion_r98833957
--- Diff: R/pkg/inst/tests/testthat/test_utils.R ---
@@ -17,6 +17,9 @@
context("functions in utils.R")
+# Ensure Spark is installed
+sparkCheckInstall()
--- End diff --
What I had in mind was to combine the `sparkR.session` and this `sparkCheckInstall` into one function so its easy to remember for a new test file. Any thoughts on this ?
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16720
**[Test build #72103 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72103/testReport)** for PR 16720 at commit [`f51f504`](https://github.com/apache/spark/commit/f51f504acb3a64da27bf0bddbb156c68d62d89bb).
* 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 #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/16720
---
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 #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:
https://github.com/apache/spark/pull/16720#discussion_r98838566
--- Diff: R/pkg/inst/tests/testthat/test_utils.R ---
@@ -17,6 +17,9 @@
context("functions in utils.R")
+# Ensure Spark is installed
+sparkCheckInstall()
--- End diff --
Ah thats a great idea - Can you see if that works (unfortunately it needs manual verification) ?
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16720
**[Test build #72793 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72793/testReport)** for PR 16720 at commit [`cd1394a`](https://github.com/apache/spark/commit/cd1394a8aab39b20c9c747b5ddfe5a003081510a).
---
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 #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:
https://github.com/apache/spark/pull/16720#discussion_r99975077
--- Diff: R/pkg/vignettes/sparkr-vignettes.Rmd ---
@@ -27,6 +27,9 @@ library(SparkR)
We use default settings in which it runs in local mode. It auto downloads Spark package in the background if no previous installation is found. For more details about setup, see [Spark Session](#SetupSparkSession).
+```{r, include=FALSE}
+SparkR:::sparkCheckInstall()
--- End diff --
FWIW These vignette changes are still needed even if we update run-all.R
---
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 #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:
https://github.com/apache/spark/pull/16720#discussion_r99456349
--- Diff: R/pkg/inst/tests/testthat/test_utils.R ---
@@ -17,6 +17,9 @@
context("functions in utils.R")
+# Ensure Spark is installed
+sparkCheckInstall()
--- End diff --
Any luck testing this 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 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 #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:
https://github.com/apache/spark/pull/16720#discussion_r98836638
--- Diff: R/pkg/inst/tests/testthat/test_utils.R ---
@@ -17,6 +17,9 @@
context("functions in utils.R")
+# Ensure Spark is installed
+sparkCheckInstall()
--- End diff --
Sure - that sounds fine. I was looking to see if `testthat` had any support for writing a `setup` that gets called before each test - Doesn't look like it has that
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/16720
Sure, I've simplified it.
Good point on the ordering - digging into it looks like it's just file system search order, which really is not reliable.
We could certainly add a test util - though seems like some tests are different though, for example test_context doesn't need a SparkSession.
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16720
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16720
**[Test build #72103 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72103/testReport)** for PR 16720 at commit [`f51f504`](https://github.com/apache/spark/commit/f51f504acb3a64da27bf0bddbb156c68d62d89bb).
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on the issue:
https://github.com/apache/spark/pull/16720
LGTM. I patched this again on top of 2.1.0 and `R CMD check --as-cran` passes now. Merging this to master and branch-2.1
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16720
**[Test build #72082 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72082/testReport)** for PR 16720 at commit [`318ecc8`](https://github.com/apache/spark/commit/318ecc876c64b7085df46618278861c2ea9ff422).
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on the issue:
https://github.com/apache/spark/pull/16720
Hmm - another fix could be that in the test cases whenever we create `spark.session` we always pass in `master=local` ?
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16720
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72793/
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 #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/16720#discussion_r98834935
--- Diff: R/pkg/vignettes/sparkr-vignettes.Rmd ---
@@ -27,6 +27,9 @@ library(SparkR)
We use default settings in which it runs in local mode. It auto downloads Spark package in the background if no previous installation is found. For more details about setup, see [Spark Session](#SetupSparkSession).
+```{r, include=FALSE}
+SparkR:::sparkCheckInstall()
--- End diff --
this has `include=FALSE` so it will run but the code and output will not be included in the vignettes text
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16720
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/72082/
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/16720
I think better would be the approach being taken in PR 16330 - has a first run test that prepare and run these kind of thing https://github.com/apache/spark/pull/16330/files#diff-5ff1ba5d1751f3b1cc96a567e9ab25ff
---
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 #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung closed the pull request at:
https://github.com/apache/spark/pull/16720
---
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 #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
Posted by felixcheung <gi...@git.apache.org>.
GitHub user felixcheung reopened a pull request:
https://github.com/apache/spark/pull/16720
[SPARK-19387][SPARKR] Tests do not run with SparkR source package in CRAN check
## What changes were proposed in this pull request?
- this is cause by changes in SPARK-18444, SPARK-18643 that we no longer install Spark when `master = ""` (default), but also related to SPARK-18449 since the real `master` value is not known at the time the R code in `sparkR.session` is run. (`master` cannot default to "local" since it could be overridden by spark-submit commandline or spark config)
- as a result, while running SparkR as a package in IDE is working fine, CRAN check is not as it is launching it via non-interactive script
- fix is to add check to the beginning of each test and vignettes; the same would also work by changing `sparkR.session()` to `sparkR.session(master = "local")` in tests, but I think being more explicit is better.
## How was this patch tested?
Tested this by reverting version to 2.1, since it needs to download the release jar with matching version. But since there are changes in 2.2 (specifically around SparkR ML) that are incompatible with 2.1, some tests are failing in this config. Will need to port this to branch-2.1 and retest with 2.1 release jar.
manually as:
```
# modify DESCRIPTION to revert version to 2.1.0
SPARK_HOME=/usr/spark R CMD build pkg
# run cran check without SPARK_HOME
R CMD check --as-cran SparkR_2.1.0.tar.gz
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/felixcheung/spark rcranchecktest
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/16720.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 #16720
----
commit 318ecc876c64b7085df46618278861c2ea9ff422
Author: Felix Cheung <fe...@hotmail.com>
Date: 2017-01-27T19:03:15Z
make sure Spark is installed
commit f51f504acb3a64da27bf0bddbb156c68d62d89bb
Author: Felix Cheung <fe...@hotmail.com>
Date: 2017-01-28T06:55:54Z
simplify
commit cd1394a8aab39b20c9c747b5ddfe5a003081510a
Author: Felix Cheung <fe...@hotmail.com>
Date: 2017-02-13T00:11:18Z
change install.spark
----
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16720
**[Test build #72793 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72793/testReport)** for PR 16720 at commit [`cd1394a`](https://github.com/apache/spark/commit/cd1394a8aab39b20c9c747b5ddfe5a003081510a).
* 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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/16720
Great! Sure will do.
I agree, I think we should consider integrating this into install.spark although that would be a change in behavior (for the better?)
(Btw I thought I was behind you at checkin a hour ago)
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on the issue:
https://github.com/apache/spark/pull/16720
I am not sure tests are ever meant to run on a cluster (see the number of uses of LocalSparkContext in core/src/test/scala) -- The main reason I dont want to introduce the 'first test' approach is that we are then relying too much on test names not clashing / getting in front of each other which seems fragile.
The other thing that might be good is to create a test util function like `initializeTestSparkContext` and inside that we put both the session start and install stuff.
---
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 #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/16720#discussion_r99462457
--- Diff: R/pkg/inst/tests/testthat/test_utils.R ---
@@ -17,6 +17,9 @@
context("functions in utils.R")
+# Ensure Spark is installed
+sparkCheckInstall()
--- End diff --
sorry, I"m really swamped, haven't had the chance to test that out yet
---
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 #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:
https://github.com/apache/spark/pull/16720#discussion_r98834936
--- Diff: R/pkg/inst/tests/testthat/test_utils.R ---
@@ -17,6 +17,9 @@
context("functions in utils.R")
+# Ensure Spark is installed
+sparkCheckInstall()
--- End diff --
I understand that, but as pointed out https://github.com/apache/spark/pull/16720#issuecomment-275832013, some tests don't need SparkSession, and some tests will create/stop one as needed, and to have a function that does that all just mean more complicity?
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16720
**[Test build #72082 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/72082/testReport)** for PR 16720 at commit [`318ecc8`](https://github.com/apache/spark/commit/318ecc876c64b7085df46618278861c2ea9ff422).
* 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 #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:
https://github.com/apache/spark/pull/16720#discussion_r99975068
--- Diff: R/pkg/inst/tests/testthat/test_utils.R ---
@@ -17,6 +17,9 @@
context("functions in utils.R")
+# Ensure Spark is installed
+sparkCheckInstall()
--- End diff --
I just tested this by putting SparkR:::checkInstall in run-all.R (before calling test_package) and that seems to do the trick on a custom 2.1.0 build !
@felixchueng when you get a chance can you update the PR with that ? The only thing that I'm concerned about is calling a private function from `run-all.R` - We could either export this function or move some of this functionality into `install.spark`
---
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 #16720: [SPARK-19387][SPARKR] Tests do not run with Spark...
Posted by shivaram <gi...@git.apache.org>.
Github user shivaram commented on a diff in the pull request:
https://github.com/apache/spark/pull/16720#discussion_r98837066
--- Diff: R/pkg/vignettes/sparkr-vignettes.Rmd ---
@@ -27,6 +27,9 @@ library(SparkR)
We use default settings in which it runs in local mode. It auto downloads Spark package in the background if no previous installation is found. For more details about setup, see [Spark Session](#SetupSparkSession).
+```{r, include=FALSE}
+SparkR:::sparkCheckInstall()
--- End diff --
Is the Rmd file a part of the install that the users see ? I just dont want to put in any code that people might copy-paste etc. Is it not good enough to pass in `master=local[*]` 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 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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/16720
@shivaram
---
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 issue #16720: [SPARK-19387][SPARKR] Tests do not run with SparkR sourc...
Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:
https://github.com/apache/spark/pull/16720
found another issue, opened SPARK-19568
---
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