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
