You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/02/19 16:54:17 UTC
[GitHub] [spark] xkrogen commented on a change in pull request #31591: [SPARK-34472][CORE] Ship ivySettings file to driver in cluster mode
xkrogen commented on a change in pull request #31591:
URL: https://github.com/apache/spark/pull/31591#discussion_r579322484
##########
File path: resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/YarnClusterSuite.scala
##########
@@ -583,6 +597,47 @@ private object YarnClasspathTest extends Logging {
}
+private object YarnAddJarTest extends Logging {
+ def main(args: Array[String]): Unit = {
+ if (args.length != 1) {
+ // scalastyle:off println
+ System.err.println(
+ s"""
+ |Invalid command line: ${args.mkString(" ")}
+ |
+ |Usage: YarnAddJarTest [result file]
+ """.stripMargin)
+ // scalastyle:on println
+ System.exit(1)
+ }
+
+ val resultPath = args(0)
+ val sc = new SparkContext(new SparkConf())
+
+ var result = "failure"
+ try {
+ val settingsFile = sc.getConf.get("spark.jars.ivySettings")
+ // Delete the original ivySettings file, so we ensure that the YARN localized file
+ // is used by the addJar call
+ // In a real cluster mode, the original settings file at the absolute path won't be present
+ // on the driver
+ new File(settingsFile).delete()
+
+ val caught = intercept[Exception] {
Review comment:
Is there any more specific exception type we can catch here?
##########
File path: resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/YarnClusterSuite.scala
##########
@@ -583,6 +597,47 @@ private object YarnClasspathTest extends Logging {
}
+private object YarnAddJarTest extends Logging {
+ def main(args: Array[String]): Unit = {
+ if (args.length != 1) {
+ // scalastyle:off println
+ System.err.println(
+ s"""
+ |Invalid command line: ${args.mkString(" ")}
+ |
+ |Usage: YarnAddJarTest [result file]
+ """.stripMargin)
+ // scalastyle:on println
+ System.exit(1)
+ }
+
+ val resultPath = args(0)
+ val sc = new SparkContext(new SparkConf())
+
+ var result = "failure"
+ try {
+ val settingsFile = sc.getConf.get("spark.jars.ivySettings")
+ // Delete the original ivySettings file, so we ensure that the YARN localized file
+ // is used by the addJar call
+ // In a real cluster mode, the original settings file at the absolute path won't be present
+ // on the driver
+ new File(settingsFile).delete()
+
+ val caught = intercept[Exception] {
+ sc.addJar("ivy://org.fake-project.test:test:1.0.0")
+ }
+ if (caught.getMessage.contains("unresolved dependency: org.fake-project.test#test")) {
+ // "unresolved dependency" is expected as the dependency does not exist
+ // but exception like "Ivy settings file <file> does not exist should result in failure
Review comment:
Minor nit: missing closing quote on this line
##########
File path: resource-managers/yarn/src/test/scala/org/apache/spark/deploy/yarn/YarnClusterSuite.scala
##########
@@ -583,6 +597,47 @@ private object YarnClasspathTest extends Logging {
}
+private object YarnAddJarTest extends Logging {
Review comment:
There's a lot of duplicated code between all of these test apps for argument parsing / validation and writing out a `result` file. I think we should consider doing a little refactoring to allow them to inherit from a trait which handles some of this shared logic, or to leverage common utility methods.
But I have no complaints if you want to maintain the status quo in this patch instead of trying to tackle that as well. Maybe a good follow-on PR.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org