-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-2960][Deploy] Support executing Spark from symlinks (reopen) #8669
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
Changes from 6 commits
9cb7ab8
1c08d80
350bec9
aa470fe
c796397
41511b4
28fd58d
5795800
16ea884
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,11 +17,13 @@ | |
| # limitations under the License. | ||
| # | ||
|
|
||
| FWDIR="$(cd "`dirname "$0"`"/..; pwd)" | ||
| export SPARK_HOME="$FWDIR" | ||
| EXAMPLES_DIR="$FWDIR"/examples | ||
| if [ -z "${SPARK_HOME}" ]; then | ||
| export SPARK_HOME="$(cd "`dirname "$0"`"/..; pwd)" | ||
| fi | ||
|
|
||
| EXAMPLES_DIR="${SPARK_HOME}"/examples | ||
|
|
||
| . "$FWDIR"/bin/load-spark-env.sh | ||
| . "${SPARK_HOME}"/bin/load-spark-env.sh | ||
|
|
||
| if [ -n "$1" ]; then | ||
| EXAMPLE_CLASS="$1" | ||
|
|
@@ -34,8 +36,8 @@ else | |
| exit 1 | ||
| fi | ||
|
|
||
| if [ -f "$FWDIR/RELEASE" ]; then | ||
| JAR_PATH="${FWDIR}/lib" | ||
| if [ -f "${SPARK_HOME}/RELEASE" ]; then | ||
| JAR_PATH="${SPARK_HOME}/lib" | ||
| else | ||
| JAR_PATH="${EXAMPLES_DIR}/target/scala-${SPARK_SCALA_VERSION}" | ||
| fi | ||
|
|
@@ -44,7 +46,7 @@ JAR_COUNT=0 | |
|
|
||
| for f in "${JAR_PATH}"/spark-examples-*hadoop*.jar; do | ||
| if [[ ! -e "$f" ]]; then | ||
| echo "Failed to find Spark examples assembly in $FWDIR/lib or $FWDIR/examples/target" 1>&2 | ||
| echo "Failed to find Spark examples assembly in ${SPARK_HOME}/lib or $SPARK_HOME/examples/target" 1>&2 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A bit of a roughness here: no curly braces for the second usage of
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks @patrungel , I will change it. BTW, what do you think of this solution, compared to the previous code using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @jerryshao , the solution with honouring |
||
| echo "You need to build Spark before running this program" 1>&2 | ||
| exit 1 | ||
| fi | ||
|
|
@@ -67,7 +69,7 @@ if [[ ! $EXAMPLE_CLASS == org.apache.spark.examples* ]]; then | |
| EXAMPLE_CLASS="org.apache.spark.examples.$EXAMPLE_CLASS" | ||
| fi | ||
|
|
||
| exec "$FWDIR"/bin/spark-submit \ | ||
| exec "${SPARK_HOME}"/bin/spark-submit \ | ||
| --master $EXAMPLE_MASTER \ | ||
| --class $EXAMPLE_CLASS \ | ||
| "$SPARK_EXAMPLES_JAR" \ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,20 +19,11 @@ | |
| # should not be executable directly | ||
| # also should not be passed any arguments, since we need original $* | ||
|
|
||
| # resolve links - $0 may be a softlink | ||
| this="${BASH_SOURCE:-$0}" | ||
| common_bin="$(cd -P -- "$(dirname -- "$this")" && pwd -P)" | ||
| script="$(basename -- "$this")" | ||
| this="$common_bin/$script" | ||
| # symlink and absolute path should rely on SPARK_HOME to resolve | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have trouble parsing what this script was doing before, but the desired outcome seems clear. The only change here is that
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This bunch of code is try to solve symlink, relative path of this file, since we now honor the SPARK_HOME to solve this issue, from my understanding the original code is not necessary. |
||
| if [ -z "${SPARK_HOME}" ]; then | ||
| export SPARK_HOME="$(cd "`dirname "$0"`"/..; pwd)" | ||
| fi | ||
|
|
||
| # convert relative path to absolute path | ||
| config_bin="`dirname "$this"`" | ||
| script="`basename "$this"`" | ||
| config_bin="`cd "$config_bin"; pwd`" | ||
| this="$config_bin/$script" | ||
|
|
||
| export SPARK_PREFIX="`dirname "$this"`"/.. | ||
| export SPARK_HOME="${SPARK_PREFIX}" | ||
| export SPARK_CONF_DIR="${SPARK_CONF_DIR:-"$SPARK_HOME/conf"}" | ||
| # Add the PySpark classes to the PYTHONPATH: | ||
| export PYTHONPATH="$SPARK_HOME/python:$PYTHONPATH" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I very much like how this has standardized everything to use
SPARK_HOMEinstead ofFWDIR, and doesn't overwrite the value if already set. (Nit: all of the occurrences of this line have a 4-space indent instead of 2)LGTM; does anyone see a reason this isn't a good idea? I suppose now
SPARK_HOME, if set, has an effect everywhere, but it looks like the desired effect. Docs also make reference toSPARK_HOMEas if it has this effect.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @srowen for your comments, I will change to the 2-space indent.