Skip to content

[SPARK-2960] Support executing Spark from symlinks #1875

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

[SPARK-2960] Support executing Spark from symlinks #1875

wants to merge 1 commit into from

Conversation

roji
Copy link

@roji roji commented Aug 10, 2014

The current scripts (e.g. pyspark) fail to run when they are executed via symlinks. A common Linux scenario would be to have Spark installed somewhere (e.g. /opt) and have a symlink to it in /usr/bin.

Fixed the scripts to traverse symlinks until reaching the actual binary.

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@mateiz
Copy link
Contributor

mateiz commented Aug 10, 2014

Jenkins, this is ok to test

@mateiz
Copy link
Contributor

mateiz commented Aug 10, 2014

@roji mind opening a JIRA issue for this on https://issues.apache.org/jira/browse/SPARK and adding it in the pull request's title?

@SparkQA
Copy link

SparkQA commented Aug 10, 2014

QA tests have started for PR 1875. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18290/consoleFull

@SparkQA
Copy link

SparkQA commented Aug 10, 2014

QA results for PR 1875:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18290/consoleFull

@roji roji changed the title Support executing Spark from symlinks [SPARK-2960] Support executing Spark from symlinks Aug 11, 2014
@roji
Copy link
Author

roji commented Aug 11, 2014

@mateiz, thanks for the attention. Have opened an issue for this.

May I suggest you guys update the contribution guide to specify that an issue be opened alongside pull requests...

@roji
Copy link
Author

roji commented Aug 11, 2014

Have just noticed the additional scripts under sbin, which also require treatment. Just before I go ahead and work on that, can you confirm this is a desirable PR?

@mateiz
Copy link
Contributor

mateiz commented Aug 13, 2014

Yup, please fix those too. I think this is useful to have.

@roji
Copy link
Author

roji commented Aug 20, 2014

@mateiz, added a commit for the sbin scripts.

In general the scripts could use a bit of cleanup - the bin and sbin scripts work a bit differently, the sbin-specific spark-config.sh doesn't do much anymore, SPARK_HOME and SPARK_PREFIX are used interchangeably... But it isn't very important.

@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA tests have started for PR 1875. This patch DID NOT merge cleanly!
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18953/consoleFull

@SparkQA
Copy link

SparkQA commented Aug 20, 2014

QA results for PR 1875:
- This patch FAILED unit tests.

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18953/consoleFull

@roji
Copy link
Author

roji commented Aug 25, 2014

Um, am not sure, did something actually go wrong here?

@mateiz
Copy link
Contributor

mateiz commented Aug 25, 2014

Yes, your patch no longer merges on master. Please rebase it onto the master branch.

The current scripts (e.g. pyspark) fail to run when they are
executed via symlinks. A common Linux scenario would be to have Spark
installed somewhere (e.g. /opt) and have a symlink to it in /usr/bin.

Fixed the scripts to traverse symlinks until reaching the actual binary.
@roji
Copy link
Author

roji commented Aug 27, 2014

Rebased on master and squashed to a single commit, hope all is well now.

@mateiz
Copy link
Contributor

mateiz commented Aug 27, 2014

Jenkins, test this please

@SparkQA
Copy link

SparkQA commented Aug 27, 2014

QA tests have started for PR 1875 at commit ccbc6e5.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Aug 27, 2014

QA tests have finished for PR 1875 at commit ccbc6e5.

  • This patch passes unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@mateiz
Copy link
Contributor

mateiz commented Aug 27, 2014

Hey @roji, actually I looked at this and I notice that -h on the [ command is deprecated and replaced by -L. Mind changing that in the code? Otherwise it looks good.

@mateiz
Copy link
Contributor

mateiz commented Aug 27, 2014

Actually I spoke too soon, this fix doesn't seem to work with symlinks to relative paths. You can do

cd spark
ln -s bin/spark-shell spark-shell

Then it will fail to run, thinking that the path is just "bin" or "/". Maybe it's because you add a / in front.

@mateiz
Copy link
Contributor

mateiz commented Aug 27, 2014

It would be nice if you found some best practice way of doing this.

@mateiz
Copy link
Contributor

mateiz commented Aug 27, 2014

Actually I notice that there's code for this in our sbt/sbt script, though it may be specific for finding directories. Take a look at that.

@SparkQA
Copy link

SparkQA commented Sep 5, 2014

Can one of the admins verify this patch?

@marmbrus
Copy link
Contributor

marmbrus commented Dec 2, 2014

Any update here? I think this would be a great feature to have, but perhaps we should close this issue until it is ready to review (to make the size of the PR queue a little more manageable).

@roji
Copy link
Author

roji commented Dec 2, 2014

Sorry for dropping out, was involved in other things. @mateiz, I'll take a look at your suggestions in the coming week.

@mateiz
Copy link
Contributor

mateiz commented Dec 4, 2014

Alright, let me know when you've had a chance.

@asfgit asfgit closed this in 3cdae03 Dec 4, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants