search

apache-spark-on-k8s / spark

Apache Spark enhanced with native Kubernetes scheduler back-end: NOTE this repository is being ARCHIVED as all new development for the kubernetes scheduler back-end is now on https://github.com/apache/spark/
https://spark.apache.org/
Apache License 2.0
612 stars 118 forks source link

SparkR Support #507

Closed ifilonenko closed 7 years ago

ifilonenko commented 7 years ago

What changes were proposed in this pull request?

Initial Spark R support

How was this patch tested?

ifilonenko commented 7 years ago

Upon merging, this PR closes #506

ifilonenko commented 7 years ago

To run integration tests in a proper R environment, it is required that R_HOME is defined in the testing environment which means everyone would need to install R. Is that something that would be an issue or something that can be assumed if someone is building out a full dev environment for Spark? @foxish @erikerlandson

foxish commented 7 years ago

Hmm, interesting. The submitting node has an R dependency? or the driver?

ifilonenko commented 7 years ago

The R dependency is because there is a need to mimic the make-distribution environment in target/docker/R so that when we run ADD R opt/spark/R it is already packaged in the Docker environment; this is similar to how we setup PySpark.

ifilonenko commented 7 years ago

@ssuchter @varunkatta Integration test will pass after R is installed and R_HOME is defined in the jenkins environment.

PR is otherwise ready for review

ifilonenko commented 7 years ago

rerun integration tests please

ifilonenko commented 7 years ago

rerun integration tests please

ifilonenko commented 7 years ago

rerun integration tests please

ifilonenko commented 7 years ago

PR is ready for review @foxish @erikerlandson

liyinan926 commented 7 years ago

@ifilonenko you also need to update sbin/build-push-docker-images.sh to add the new images.

ifilonenko commented 7 years ago

rerun unit tests please

ifilonenko commented 7 years ago

Ready for merging to branch-2.2 unless any other concerns @erikerlandson @foxish @liyinan926

liyinan926 commented 7 years ago

LGTM. Thanks for the work!

ifilonenko commented 7 years ago

rerun unit tests please

ifilonenko commented 7 years ago

Unless there are any further comments, I think this is ready to merge

ifilonenko commented 7 years ago

All set to merge? @foxish @erikerlandson @liyinan926

liyinan926 commented 7 years ago

I think it's all good.

felixcheung commented 7 years ago

hi - what's left for this work? this https://github.com/apache-spark-on-k8s/spark/pull/507#discussion_r143595440?

mccheah commented 7 years ago

I left some comments. Also would like to see this tested in a production environment, but maybe we can just merge it and follow up as feedback comes in.

ifilonenko commented 7 years ago

@mccheah as per your last comment. Is this okay to merge then?

mccheah commented 7 years ago

Can merge when CI passes - I just updated the branch.

ifilonenko commented 7 years ago

ready for merging: @foxish

foxish commented 7 years ago

Nvm. Please ignore last comment. That was in the merge commit.