Skip to content
Closed
Show file tree
Hide file tree
Changes from 26 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions core/src/main/scala/org/apache/spark/SparkContext.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1775,7 +1775,7 @@ class SparkContext(config: SparkConf) extends Logging {
* @note A path can be added only once. Subsequent additions of the same path are ignored.
*/
def addJar(path: String) {
def addJarFile(file: File): String = {
def addLocalJarFile(file: File): String = {
try {
if (!file.exists()) {
throw new FileNotFoundException(s"Jar ${file.getAbsolutePath} not found")
Expand All @@ -1792,12 +1792,36 @@ class SparkContext(config: SparkConf) extends Logging {
}
}

def addRemoteJarFile(path: String): String = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Better to change to checkRemoteJarFile, here in this method it only checks the jar file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

origin addFileJar will also check jar exists. all same to local jar file .

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

addJarFile also adds the jar file to fileserver, that's the key purpose there, not just checking.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reasonable, done .

val hadoopPath = new Path(path)
val scheme = new URI(path).getScheme
if (!Array("http", "https", "ftp").contains(scheme)) {
try {
val fs = hadoopPath.getFileSystem(hadoopConfiguration)
if (!fs.exists(hadoopPath)) {
throw new FileNotFoundException(s"Jar ${path} not found")
}
if (fs.isDirectory(hadoopPath)) {
throw new IllegalArgumentException(
s"Directory ${path} is not allowed for addJar")
}
path
} catch {
case NonFatal(e) =>
logError(s"Failed to add $path to Spark environment", e)
null
Comment thread
srowen marked this conversation as resolved.
}
} else {
path
}
}

if (path == null) {
logWarning("null specified as parameter to addJar")
} else {
val key = if (path.contains("\\")) {
// For local paths with backslashes on Windows, URI throws an exception
addJarFile(new File(path))
addLocalJarFile(new File(path))
} else {
val uri = new URI(path)
// SPARK-17650: Make sure this is a valid URL before adding it to the list of dependencies
Expand All @@ -1806,12 +1830,12 @@ class SparkContext(config: SparkConf) extends Logging {
// A JAR file which exists only on the driver node
case null =>
// SPARK-22585 path without schema is not url encoded
addJarFile(new File(uri.getRawPath))
addLocalJarFile(new File(uri.getRawPath))
// A JAR file which exists only on the driver node
case "file" => addJarFile(new File(uri.getPath))
case "file" => addLocalJarFile(new File(uri.getPath))
// A JAR file which exists locally on every worker node
case "local" => "file:" + uri.getPath
case _ => path
case _ => addRemoteJarFile(path)
}
}
if (key != null) {
Expand Down
11 changes: 11 additions & 0 deletions core/src/test/scala/org/apache/spark/SparkContextSuite.scala
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,17 @@ class SparkContextSuite extends SparkFunSuite with LocalSparkContext with Eventu
}
}

test("add hdfs jar files not exists") {
try {
val jarPath = "hdfs:///no/path/to/TestUDTF.jar"
sc = new SparkContext(new SparkConf().setAppName("test").setMaster("local"))
sc.addJar(jarPath)
assert(sc.listJars().forall(!_..contains("TestUDTF.jar")))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you have an extra period here

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

==
Before commit code, accidentally hit the keyboard, have change it .

} finally {
sc.stop()
}
}

test("SPARK-17650: malformed url's throw exceptions before bricking Executors") {
try {
sc = new SparkContext(new SparkConf().setAppName("test").setMaster("local"))
Expand Down