Github user srowen commented on a diff in the pull request: https://github.com/apache/spark/pull/15420#discussion_r82609547 --- Diff: core/src/main/scala/org/apache/spark/deploy/worker/DriverRunner.scala --- @@ -147,7 +147,8 @@ private[deploy] class DriverRunner( * Will throw an exception if there are errors downloading the jar. */ private def downloadUserJar(driverDir: File): String = { - val jarPath = new Path(driverDesc.jarUrl) + // Remove query string if jarUrl is http based + val jarPath = new Path(driverDesc.jarUrl.takeWhile(_ != '?')) --- End diff -- I think there are a few small funny things about this method that we could clean up. First, jarUrl is really a URI, so parsing it as a Path is a little odd. It's just used to get the file name, but `java.net.URI` is probably the better tool for the job, though it takes an extra step to pick out just the file name. - `destPath` is actually the same as `localJarFile` but defined differently. - File existence checked twice for no reason - Exception is thrown not IOException - Log message doesn't actually correctly describe what it's downloading That is, I wonder if it's worth a little cleanup to get to something like ``` /** * Download the user jar into the supplied directory and return its local path. * Will throw an exception if there are errors downloading the jar. */ private def downloadUserJar(driverDir: File): String = { val jarFileName = new URI(driverDesc.jarUrl).getPath.split("/").last val localJarFile = new File(driverDir, jarFileName) if (!localJarFile.exists()) { // May already exist if running multiple workers on one node logInfo(s"Copying user jar ${driverDesc.jarUrl} to $localJarFile") Utils.fetchFile( driverDesc.jarUrl, driverDir, conf, securityManager, SparkHadoopUtil.get.newConfiguration(conf), System.currentTimeMillis(), useCache = false) if (!localJarFile.exists()) { // Verify copy succeeded throw new IOException(s"Did not see expected jar $jarFileName in $driverDir") } } localJarFile.getAbsolutePath } ``` ? Have not tested it directly.
--- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. --- --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org