[SPARK-16055][SPARKR] warning added while using sparkPackages with spark-submit by krishnakalyan3 · Pull Request #14179 · apache/spark

krishnakalyan3 · 2016-07-13T09:01:10Z

What changes were proposed in this pull request?

https://issues.apache.org/jira/browse/SPARK-16055
sparkPackages - argument is passed and we detect that we are in the R script mode, we should print some warning like --packages flag should be used with with spark-submit

How was this patch tested?

In my system locally

krishnakalyan3 · 2016-07-13T09:06:43Z

cc @shivaram

shivaram · 2016-07-13T16:39:17Z

 existingPort <- Sys.getenv("EXISTING_SPARKR_BACKEND_PORT", "")
 if (existingPort != "") {
+ if(sparkPackages != ""){
+ warning("--packages flag should be used with with spark-submit")


two space indent here

And "spark-submit or sparkR shell"

@felixcheung the sparkPackages argument should work from the SparkR shell ? Not sure we should add that in the warning message

@shivaram @felixcheung how about something like sparkPackages cannot be used as an argument within sparkR.init please use the --packages flag while using spark-submit or sparkR shell

@shivaram maybe it should but sparkR.session() is already called in sparkR shell, and calling SparkSession again with the sparkPackages does nothing:
> sparkR.session(sparkPackages = "com.databricks:spark-avro_2.10:2.0.1") Java ref type org.apache.spark.sql.SparkSession id 1 > read.df("", source = "avro") 16/07/14 23:55:43 ERROR RBackendHandler: loadDF on org.apache.spark.sql.api.r.SQLUtils failed Error in invokeJava(isStatic = TRUE, className, methodName, ...) : org.apache.spark.sql.AnalysisException: Failed to find data source: avro. Please use Spark package http://spark-packages.org/package/databricks/spark-avro;
It's because it is passed only when creating the SparkContext initially.
https://github.com/apache/spark/blob/master/R/pkg/R/sparkR.R#L164
@krishnakalyan3 something like "sparkPackages has no effect when using spark-submit or sparkR shell, please use the --packages commandline instead"

shivaram · 2016-07-13T16:39:46Z

Jenkins, ok to test

SparkQA · 2016-07-13T16:45:24Z

Test build #62254 has finished for PR 14179 at commit 3d5d49b.

This patch fails R style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-07-14T18:35:40Z

Test build #62338 has finished for PR 14179 at commit 259c4af.

This patch fails SparkR unit tests.
This patch merges cleanly.
This patch adds no public classes.

krishnakalyan3 · 2016-07-15T18:12:31Z

@shivaram @felixcheung My patch fails sparkR unit test. (./R/run-tests.sh)
Logs https://gist.github.com/krishnakalyan3/6585a1007b731e82fede1b942ea00bec
I am not sure how to go about resolving this.

SparkQA · 2016-07-15T18:21:04Z

Test build #62385 has finished for PR 14179 at commit 76fa14b.

This patch fails R style tests.
This patch merges cleanly.
This patch adds no public classes.

felixcheung · 2016-07-16T05:21:08Z

The error in the link you provided (https://gist.github.com/krishnakalyan3/6585a1007b731e82fede1b942ea00bec) are odd, I have not seen them. What is the version of testthat you have installed? As for build failure, they look more straightforward: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62385/console R/pkg/R/group.R' lines should not be more than 100 characters. warning("sparkPackages has no effect when using spark-submit or sparkR shell, please use the --packages commandline instead") ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ lintr checks failed.

krishnakalyan3 · 2016-07-16T17:38:00Z

@felixcheung my local unit test still fail, anyway thanks for the clarification.

SparkQA · 2016-07-16T17:40:11Z

Test build #62409 has finished for PR 14179 at commit c0dca31.

This patch fails R style tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-07-16T20:05:59Z

Test build #62410 has finished for PR 14179 at commit 204fa25.

This patch fails SparkR unit tests.
This patch merges cleanly.
This patch adds no public classes.

krishnakalyan3 · 2016-07-16T20:28:04Z

@felixcheung @shivaram I am not sure if the warning message is clear enough. I did the best I could with character limit of 100. I am not sure which SparkR unit tests fail from the logs below
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/62410/console

shivaram · 2016-07-16T20:38:12Z

@krishnakalyan3 We don't need to modify the message. You can keep the original message and just split it across two lines with something like the paste function used in

spark/R/pkg/inst/tests/testthat/test_sparkSQL.R

Line 1599 in 5ec0d69

 paste("The following column name: name_y occurs more than once in the 'DataFrame'.", 

Regarding the test error, it looks like there is some unit test that is running into this warning and that warning isn't expected, so it gets promoted to an error. We should first track down which test is causing this.

SparkQA · 2016-07-16T21:48:29Z

Test build #62414 has finished for PR 14179 at commit b425540.

This patch fails SparkR unit tests.
This patch merges cleanly.
This patch adds no public classes.

felixcheung · 2016-07-16T23:41:23Z

A couple of test files are reusing an existing SparkSession/SparkContext by calling sparkR.sparkContext I think they are now hitting the warning statement that is added in this PR, somehow. I'm not sure which ones are calling sparkR.sparkContext with sparkPackages set. If you find them you are add suppressWarnings() around it.

SerDe functionality : Error in sparkR.sparkContext(master, appName, sparkHome, sparkConfigMap, : (converted from warning) sparkPackages has no effect when using spark-submit or sparkR shell,please use the --packages commandline instead
Calls: test_package ... eval -> eval -> sparkR.session -> sparkR.sparkContext

shivaram · 2016-07-17T00:02:04Z


 existingPort <- Sys.getenv("EXISTING_SPARKR_BACKEND_PORT", "")
 if (existingPort != "") {
+ if (length(sparkPackages) != 0) {


I looked into this and we get the unit test error because the check here isn't correct. So we get sparkPackages as "" and in R the length of empty string is not 0 but 1.
> length("") [1] 1
I think we should instead check the length of packages to be zero ? (packages is a list created by splitting the input in processSparkPackages)

checking
if (length(packages) != 0)
sounds like a much better idea!

@shivaram yes you are right, thanks. @felixcheung will make the change.

SparkQA · 2016-07-17T06:37:45Z

Test build #62424 has finished for PR 14179 at commit aefdb45.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-07-17T06:47:02Z

Test build #62425 has finished for PR 14179 at commit 12bd700.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

krishnakalyan3 · 2016-07-17T10:43:45Z

@felixcheung @shivaram Is the current state okay?

shivaram · 2016-07-17T20:30:49Z

LGTM.

felixcheung · 2016-07-18T06:48:30Z

LGTM.

btw, for future references, you could either

# sep defaults to " "
paste("sparkPackages has no effect when using spark-submit or sparkR shell,",
"please use the --packages commandline instead")

or

paste0("sparkPackages has no effect when using spark-submit or sparkR shell, ",
"please use the --packages commandline instead")

…ark-submit ## What changes were proposed in this pull request? https://issues.apache.org/jira/browse/SPARK-16055 sparkPackages - argument is passed and we detect that we are in the R script mode, we should print some warning like --packages flag should be used with with spark-submit ## How was this patch tested? In my system locally Author: krishnakalyan3 <krishnakalyan3@gmail.com> Closes#14179 from krishnakalyan3/spark-pkg. (cherry picked from commit 8ea3f4e) Signed-off-by: Shivaram Venkataraman <shivaram@cs.berkeley.edu>

krishnakalyan3 · 2016-07-20T08:16:51Z

@shivaram @felixcheung thanks for the reviews. Will keep the feedbacks in mind.

warning added while using sparkPackages with spark-submit
3d5d49b

shivaram reviewed Jul 13, 2016
View reviewed changes

shivaram mentioned this pull request Jul 13, 2016
[SPARKR][DOCS][MINOR] R programming guide to include csv data source example #14178
Closed

style change and improve warning message
259c4af

Improve warning message
76fa14b

character limit
c0dca31

character limit
204fa25

krishnakalyan3 added 2 commits July 16, 2016 23:24

fix warning
1f01365

fix warning
b425540

shivaram reviewed Jul 17, 2016
View reviewed changes

krishnakalyan3 added 3 commits July 17, 2016 08:09

using packages to check length
28245ec

using packages to check length
aefdb45

fix space
12bd700

asfgit closed this in 8ea3f4eJul 18, 2016

krishnakalyan3 deleted the spark-pkg branch July 20, 2016 08:13

Uh oh!

Conversation

krishnakalyan3 commented Jul 13, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

krishnakalyan3 commented Jul 13, 2016

Uh oh!

shivaramJul 13, 2016

Choose a reason for hiding this comment

Uh oh!

felixcheungJul 13, 2016

Choose a reason for hiding this comment

Uh oh!

shivaramJul 14, 2016

Choose a reason for hiding this comment

Uh oh!

krishnakalyan3Jul 14, 2016

Choose a reason for hiding this comment

Uh oh!

felixcheungJul 15, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shivaram commented Jul 13, 2016

Uh oh!

SparkQA commented Jul 13, 2016

Uh oh!

SparkQA commented Jul 14, 2016

Uh oh!

krishnakalyan3 commented Jul 15, 2016• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented Jul 15, 2016

Uh oh!

felixcheung commented Jul 16, 2016 via email

Uh oh!

krishnakalyan3 commented Jul 16, 2016

Uh oh!

SparkQA commented Jul 16, 2016

Uh oh!

SparkQA commented Jul 16, 2016

Uh oh!

krishnakalyan3 commented Jul 16, 2016

Uh oh!

shivaram commented Jul 16, 2016

Uh oh!

SparkQA commented Jul 16, 2016

Uh oh!

felixcheung commented Jul 16, 2016

Uh oh!

shivaramJul 17, 2016

Choose a reason for hiding this comment

Uh oh!

felixcheungJul 17, 2016

Choose a reason for hiding this comment

Uh oh!

krishnakalyan3Jul 17, 2016

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 17, 2016

Uh oh!

SparkQA commented Jul 17, 2016

Uh oh!

krishnakalyan3 commented Jul 17, 2016

Uh oh!

shivaram commented Jul 17, 2016

Uh oh!

felixcheung commented Jul 18, 2016

Uh oh!

krishnakalyan3 commented Jul 20, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

krishnakalyan3 commented Jul 13, 2016•
edited
Loading

felixcheungJul 15, 2016•
edited
Loading

krishnakalyan3 commented Jul 15, 2016•
edited
Loading