[SPARK-4504][Examples] fix run-example failure if multiple example jars present #3377

gvramana · 2014-11-20T04:53:41Z

Fix run-examples script error during multiple jars.
Modified to support failing fast similar to assembly.jar

Author: Venkata Ramana G [email protected]

AmplabJenkins · 2014-11-20T04:57:10Z

Can one of the admins verify this patch?

vanzin · 2014-11-21T00:38:16Z

bin/run-example

-  export SPARK_EXAMPLES_JAR="`ls "$FWDIR"/lib/spark-examples-*hadoop*.jar`"
-elif [ -e "$EXAMPLES_DIR"/target/scala-$SPARK_SCALA_VERSION/spark-examples-*hadoop*.jar ]; then
-  export SPARK_EXAMPLES_JAR="`ls "$EXAMPLES_DIR"/target/scala-$SPARK_SCALA_VERSION/spark-examples-*hadoop*.jar`"
+  export SPARK_EXAMPLES_JAR="`ls -t "$FWDIR"/lib/spark-examples-*hadoop*.jar 2>>/dev/null | head -1`"


How do you know this will pick up the right jar?

More generally, I think the only situation someone will run into this is when switching between branches and rebuilding without cleaning. Not sure it's worth it to add workarounds in the code for that.

You'll get a similar error from compute-classpath.sh if multiple assembly jars exist. The same behavior should be just fine for the examples.

I understand that required jar will not always be latest. But failing with script error confuses user. We can at least correct to give a proper error message.

I'd prefer to fail fast in this case like we do for the assembly jar.

gvramana · 2014-11-27T05:26:15Z

Modified to support failing fast similar to assembly.jar

gvramana · 2014-12-01T03:31:54Z

Please review the patch, thanks

JoshRosen · 2014-12-24T22:42:38Z

This looks pretty good, but I've noticed that there are some small differences between the code here and the analogous code in compute-classpath.sh. We should keep them as similar as possible.

JoshRosen · 2014-12-24T22:43:32Z

bin/run-example

  echo "Failed to find Spark examples assembly in $FWDIR/lib or $FWDIR/examples/target" 1>&2
  echo "You need to build Spark before running this program" 1>&2
  exit 1
 fi

+if [ "$JAR_COUNT" -gt "1" ]; then
+  JARS_LIST="`ls -t ${JAR_PATH}/spark-examples-*hadoop*.jar`"


For instance, compute-classpath uses

jars_list=$(ls "$assembly_folder" | grep "spark-assembly.*hadoop.*.jar$")

for this; we should match this style rather than introducing a new one using backticks, ls - t, etc.

JoshRosen · 2014-12-24T22:45:40Z

@nchammas, do you want to chime in here regarding bash style?

nchammas · 2014-12-24T23:01:01Z

The style looks good to me, with a couple of exceptions: In general we should use $(...) in place of backticks, and we should also avoid parsing the output of ls.

nchammas · 2014-12-24T23:10:55Z

bin/run-example

 fi

-if [[ -z "$SPARK_EXAMPLES_JAR" ]]; then
+JAR_COUNT="`ls ${JAR_PATH}/spark-examples-*hadoop*.jar 2>>/dev/null | wc -l`"


I know that the old code used this style, but it would be great if we could replace all occurrences where we capture the output of ls with one of the recommended patterns in either of these two pages (e.g. using for f in glob_expression or using find).

Even better would be to find a way to avoid having to capture this output at all

srowen · 2015-01-05T13:17:22Z

There was already a PR for this: #3069
But it seems to be fixing a different root cause, that the assembly generated by SBT and Maven are named differently. I am not sure if this also addresses this problem?

JoshRosen · 2015-01-05T19:49:34Z

@srowen You can still end up with multiple examples assemblies if you're switching between branches without performing cleans, e.g. switch from branch-1.2 to master.

srowen · 2015-01-05T19:57:00Z

@JoshRosen Yeah I get that; I think there is another different issue lurking here, though I haven't really looked at it, and that is that the glob pattern does not match what SBT builds. The other PR might also be worth merging.

gvramana · 2015-01-11T05:15:06Z

@nchammas @JoshRosen Thanks for your comments. I have updated the same. please review.

nchammas · 2015-01-11T07:26:59Z

Updated Bash style looks good to me.

JoshRosen · 2015-01-15T19:36:30Z

Jenkins, this is ok to test.

SparkQA · 2015-01-15T19:37:45Z

Test build #25611 has started for PR 3377 at commit fa7f481.

This patch merges cleanly.

SparkQA · 2015-01-15T20:41:50Z

Test build #25611 has finished for PR 3377 at commit fa7f481.

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

AmplabJenkins · 2015-01-15T20:41:53Z

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/25611/
Test PASSed.

JoshRosen · 2015-01-19T19:53:23Z

LGTM. Thanks @nchammas for looking this over. I'm going to merge this into master (1.3.0), branch-1.2 (1.2.1), and branch-1.1 (1.1.2).

…ars exist Fix run-example script to fail fast with useful error message if multiple example assembly JARs are present. Author: Venkata Ramana Gollamudi <[email protected]> Closes #3377 from gvramana/run-example_fails and squashes the following commits: fa7f481 [Venkata Ramana Gollamudi] Fixed review comments, avoiding ls output scanning. 6aa1ab7 [Venkata Ramana Gollamudi] Fix run-examples script error during multiple jars (cherry picked from commit 74de94e) Signed-off-by: Josh Rosen <[email protected]> Conflicts: bin/compute-classpath.sh

…ars exist Fix run-example script to fail fast with useful error message if multiple example assembly JARs are present. Author: Venkata Ramana Gollamudi <[email protected]> Closes #3377 from gvramana/run-example_fails and squashes the following commits: fa7f481 [Venkata Ramana Gollamudi] Fixed review comments, avoiding ls output scanning. 6aa1ab7 [Venkata Ramana Gollamudi] Fix run-examples script error during multiple jars (cherry picked from commit 74de94e) Signed-off-by: Josh Rosen <[email protected]> Conflicts: bin/compute-classpath.sh bin/run-example

nchammas · 2015-02-11T22:44:06Z

bin/compute-classpath.sh

-fi
+num_jars=0
+
+for f in ${assembly_folder}/spark-assembly*hadoop*.jar; do


There is a word splitting bug here. My bad for missing this during review.

gvramana · 2015-02-12T09:34:23Z

Oh, basic mistake :( . Sorry I missed it completely.
Fixed the same in PR #4561
@nchammas Thanks for identifying.

vanzin reviewed Nov 21, 2014
View reviewed changes

gvramana force-pushed the run-example_fails branch from 81d659b to 2878139 Compare November 27, 2014 05:24

JoshRosen reviewed Dec 24, 2014
View reviewed changes

nchammas reviewed Dec 24, 2014
View reviewed changes

srowen mentioned this pull request Jan 5, 2015

[SPARK-4504] Minor bug fixes in bin/run-example #3069

Closed

gvramana added 2 commits January 7, 2015 06:56

Fix run-examples script error during multiple jars

6aa1ab7

Fixed review comments, avoiding ls output scanning.

fa7f481

gvramana force-pushed the run-example_fails branch from 2878139 to fa7f481 Compare January 11, 2015 03:58

asfgit closed this in 74de94e Jan 19, 2015

nchammas reviewed Feb 11, 2015
View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-4504][Examples] fix run-example failure if multiple example jars present #3377

[SPARK-4504][Examples] fix run-example failure if multiple example jars present #3377

gvramana commented Nov 20, 2014

AmplabJenkins commented Nov 20, 2014

vanzin Nov 21, 2014

gvramana Nov 23, 2014

pwendell Nov 25, 2014

gvramana commented Nov 27, 2014

gvramana commented Dec 1, 2014

JoshRosen commented Dec 24, 2014

JoshRosen Dec 24, 2014

JoshRosen commented Dec 24, 2014

nchammas commented Dec 24, 2014

nchammas Dec 24, 2014

srowen commented Jan 5, 2015

JoshRosen commented Jan 5, 2015

srowen commented Jan 5, 2015

gvramana commented Jan 11, 2015

nchammas commented Jan 11, 2015

JoshRosen commented Jan 15, 2015

SparkQA commented Jan 15, 2015

SparkQA commented Jan 15, 2015

AmplabJenkins commented Jan 15, 2015

JoshRosen commented Jan 19, 2015

nchammas Feb 11, 2015

gvramana commented Feb 12, 2015

[SPARK-4504][Examples] fix run-example failure if multiple example jars present #3377

[SPARK-4504][Examples] fix run-example failure if multiple example jars present #3377

Conversation

gvramana commented Nov 20, 2014

AmplabJenkins commented Nov 20, 2014

vanzin Nov 21, 2014

Choose a reason for hiding this comment

gvramana Nov 23, 2014

Choose a reason for hiding this comment

pwendell Nov 25, 2014

Choose a reason for hiding this comment

gvramana commented Nov 27, 2014

gvramana commented Dec 1, 2014

JoshRosen commented Dec 24, 2014

JoshRosen Dec 24, 2014

Choose a reason for hiding this comment

JoshRosen commented Dec 24, 2014

nchammas commented Dec 24, 2014

nchammas Dec 24, 2014

Choose a reason for hiding this comment

srowen commented Jan 5, 2015

JoshRosen commented Jan 5, 2015

srowen commented Jan 5, 2015

gvramana commented Jan 11, 2015

nchammas commented Jan 11, 2015

JoshRosen commented Jan 15, 2015

SparkQA commented Jan 15, 2015

SparkQA commented Jan 15, 2015

AmplabJenkins commented Jan 15, 2015

JoshRosen commented Jan 19, 2015

nchammas Feb 11, 2015

Choose a reason for hiding this comment

gvramana commented Feb 12, 2015