Pass the actual iterable from the option to get files #139
Conversation
| @@ -77,8 +77,12 @@ private[spark] class Client( | |||
|
|
|||
| def run(): Unit = { | |||
| logInfo(s"Starting application $kubernetesAppId in Kubernetes...") | |||
| val submitterLocalFiles = KubernetesFileUtils.getOnlySubmitterLocalFiles(sparkFiles) | |||
| val submitterLocalJars = KubernetesFileUtils.getOnlySubmitterLocalFiles(sparkJars) | |||
| val submitterLocalFiles = KubernetesFileUtils.getOnlySubmitterLocalFiles(sparkFiles | |||
There was a problem hiding this comment.
We'll probably try to catch this at the unit test level, not the integration test level.
There was a problem hiding this comment.
I'm not sure how you'll catch this at the unit test level -- won't you have to call the run() method to hit this, and won't that pretty much only be done in an integration test?
There was a problem hiding this comment.
I think we do want to call run() in the unit test however. Might take a decent amount of effort to mock everything, but it's probably the way to go.
| val submitterLocalFiles = KubernetesFileUtils.getOnlySubmitterLocalFiles(sparkFiles | ||
| .map(_.split(",")) | ||
| .getOrElse(Array.empty[String])) | ||
| val submitterLocalJars = KubernetesFileUtils.getOnlySubmitterLocalFiles(sparkJars |
There was a problem hiding this comment.
the sparkJars and sparkFiles instance variables here are still in the comma-separated format which is a little unexpected. They also don't have type annotations which maybe could've helped in review.
Maybe we should do the comma-splitting in the first access of spark.files and spark.jars and then annotate the types as Iterable[String] instead of the current implicit Option[String] ?
private val sparkFiles = sparkConf.getOption("spark.files")
private val sparkJars = sparkConf.getOption("spark.jars")
|
Merging now, but I'm uncomfortable that we don't have a test that covers this so filed an issue at #141 |
* Pass the actual iterable from the option to get files * Split the original instance variables * Explicitly set the type of the array
* Pass the actual iterable from the option to get files * Split the original instance variables * Explicitly set the type of the array
…n-k8s#139) * Pass the actual iterable from the option to get files * Split the original instance variables * Explicitly set the type of the array (cherry picked from commit 913a60e)
…n-k8s#139) * Pass the actual iterable from the option to get files * Split the original instance variables * Explicitly set the type of the array
Closes #137