address review comments

brkyvz · brkyvz · commit c73aabe78972 · 2015-02-25T22:47:52.000-08:00
diff --git a/core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala b/core/src/main/scala/org/apache/spark/deploy/SparkSubmit.scala
@@ -251,17 +251,19 @@ object SparkSubmit {
     }
 
     val isYarnCluster = clusterManager == YARN && deployMode == CLUSTER
-    if (args.packagesResolved != null) {
-      args.jars = mergeFileLists(args.jars, args.packagesResolved)
-      if (args.isPython) args.pyFiles = mergeFileLists(args.pyFiles, args.packagesResolved)
-    } else {
-      // This part needs to be added in case SparkSubmitDriverBootstrapper is not called
-      val resolvedMavenCoordinates =
-        SparkSubmitUtils.resolveMavenCoordinates(
-          args.packages, Option(args.repositories), Option(args.ivyRepoPath)).mkString(",")
-      if (resolvedMavenCoordinates.trim.length > 0) {
-        args.jars = mergeFileLists(args.jars, resolvedMavenCoordinates)
-        if (args.isPython) args.pyFiles = mergeFileLists(args.pyFiles, resolvedMavenCoordinates)
+    val packagesResolved =
+      if (args.packagesResolved != null) {
+        // SparkSubmitDriverBootstrapper already downloaded the jars for us
+        args.packagesResolved
+      } else {
+        SparkSubmitUtils.resolveMavenCoordinates(args.packages, Option(args.repositories), 
+          Option(args.ivyRepoPath)).mkString(",")
+      }
+
+    if (packagesResolved.nonEmpty) {
+      args.jars = mergeFileLists(args.jars, packagesResolved)
+      if (args.isPython) {
+        args.pyFiles = mergeFileLists(args.pyFiles, packagesResolved)
       }
     }
 
@@ -713,7 +715,7 @@ private[spark] object SparkSubmitUtils {
    * after a '!' by Ivy. It also sometimes contains '(bundle)' after '.jar'. Remove that as well.
    * @param artifacts Sequence of dependencies that were resolved and retrieved
    * @param cacheDirectory directory where jars are cached
-   * @return a comma-delimited list of paths for the dependencies
+   * @return A sequence of paths for the dependencies
    */
   private[spark] def resolveDependencyPaths(
       artifacts: Array[AnyRef],
@@ -778,7 +780,7 @@ private[spark] object SparkSubmitUtils {
    * @param coordinates Comma-delimited string of maven coordinates
    * @param remoteRepos Comma-delimited string of remote repositories other than maven central
    * @param ivyPath The path to the local ivy repository
-   * @return The comma-delimited path to the jars of the given maven artifacts including their
+   * @return A sequence of paths to the jars of the given maven artifacts including their
    *         transitive dependencies
    */
   private[spark] def resolveMavenCoordinates(
@@ -787,7 +789,7 @@ private[spark] object SparkSubmitUtils {
       ivyPath: Option[String],
       isTest: Boolean = false): Seq[String] = {
     if (coordinates == null || coordinates.trim.isEmpty) {
-      Seq("")
+      Seq.empty
     } else {
       val sysOut = System.out
       // To prevent ivy from logging to system out
diff --git a/core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala b/core/src/main/scala/org/apache/spark/deploy/SparkSubmitArguments.scala
@@ -396,7 +396,8 @@ private[spark] class SparkSubmitArguments(args: Seq[String], env: Map[String, St
         packages = value
         parse(tail)
 
-      // Internal flag to receive the resolved maven jars and add to --jars
+      // Spark-6031 Internal flag to receive the resolved maven jars and add to --jars. 
+      // This is only passed through the Bootstrapper
       case ("--packages-resolved") :: value :: tail =>
         packagesResolved = Utils.resolveURIs(value)
         parse(tail)
diff --git a/core/src/main/scala/org/apache/spark/deploy/SparkSubmitDriverBootstrapper.scala b/core/src/main/scala/org/apache/spark/deploy/SparkSubmitDriverBootstrapper.scala
@@ -97,7 +97,7 @@ private[spark] object SparkSubmitDriverBootstrapper {
     val resolvedMavenCoordinates =
       SparkSubmitUtils.resolveMavenCoordinates(
         submitPackages, submitRepositories, confIvyRepo)
-    if (resolvedMavenCoordinates.head.length > 0) {
+    if (resolvedMavenCoordinates.nonEmpty) {
       newClasspath += sys.props("path.separator") + 
         resolvedMavenCoordinates.mkString(sys.props("path.separator"))
       submitArgs = 
