Skip to content

Commit e1d72f2

Browse files
ifilonenkomccheah
authored andcommitted
[SPARK-25264][K8S] Fix comma-delineated arguments passed into PythonRunner and RRunner
## What changes were proposed in this pull request? Fixes the issue brought up in kubeflow/spark-operator#273 where the arguments were being comma-delineated, which was incorrect wrt to the PythonRunner and RRunner. ## How was this patch tested? Modified unit test to test this change. Author: Ilan Filonenko <[email protected]> Closes apache#22257 from ifilonenko/SPARK-25264.
1 parent 32da87d commit e1d72f2

File tree

4 files changed

+8
-6
lines changed

4 files changed

+8
-6
lines changed

resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/bindings/PythonDriverFeatureStep.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,12 @@ private[spark] class PythonDriverFeatureStep(
3030
override def configurePod(pod: SparkPod): SparkPod = {
3131
val roleConf = kubernetesConf.roleSpecificConf
3232
require(roleConf.mainAppResource.isDefined, "PySpark Main Resource must be defined")
33+
// Delineation is done by " " because that is input into PythonRunner
3334
val maybePythonArgs = Option(roleConf.appArgs).filter(_.nonEmpty).map(
3435
pyArgs =>
3536
new EnvVarBuilder()
3637
.withName(ENV_PYSPARK_ARGS)
37-
.withValue(pyArgs.mkString(","))
38+
.withValue(pyArgs.mkString(" "))
3839
.build())
3940
val maybePythonFiles = kubernetesConf.pyFiles().map(
4041
// Dilineation by ":" is to append the PySpark Files to the PYTHONPATH

resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/bindings/RDriverFeatureStep.scala

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,12 @@ private[spark] class RDriverFeatureStep(
3030
override def configurePod(pod: SparkPod): SparkPod = {
3131
val roleConf = kubernetesConf.roleSpecificConf
3232
require(roleConf.mainAppResource.isDefined, "R Main Resource must be defined")
33+
// Delineation is done by " " because that is input into RRunner
3334
val maybeRArgs = Option(roleConf.appArgs).filter(_.nonEmpty).map(
3435
rArgs =>
3536
new EnvVarBuilder()
3637
.withName(ENV_R_ARGS)
37-
.withValue(rArgs.mkString(","))
38+
.withValue(rArgs.mkString(" "))
3839
.build())
3940
val envSeq =
4041
Seq(new EnvVarBuilder()

resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/bindings/PythonDriverFeatureStepSuite.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ class PythonDriverFeatureStepSuite extends SparkFunSuite {
4444
Some(PythonMainAppResource("local:///main.py")),
4545
"test-app",
4646
"python-runner",
47-
Seq("5 7")),
47+
Seq("5", "7", "9")),
4848
appResourceNamePrefix = "",
4949
appId = "",
5050
roleLabels = Map.empty,
@@ -66,7 +66,7 @@ class PythonDriverFeatureStepSuite extends SparkFunSuite {
6666
.toMap
6767
assert(envs(ENV_PYSPARK_PRIMARY) === expectedMainResource)
6868
assert(envs(ENV_PYSPARK_FILES) === expectedPySparkFiles)
69-
assert(envs(ENV_PYSPARK_ARGS) === "5 7")
69+
assert(envs(ENV_PYSPARK_ARGS) === "5 7 9")
7070
assert(envs(ENV_PYSPARK_MAJOR_PYTHON_VERSION) === "2")
7171
}
7272
test("Python Step testing empty pyfiles") {

resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/features/bindings/RDriverFeatureStepSuite.scala

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ class RDriverFeatureStepSuite extends SparkFunSuite {
3838
Some(RMainAppResource(mainResource)),
3939
"test-app",
4040
"r-runner",
41-
Seq("5 7")),
41+
Seq("5", "7", "9")),
4242
appResourceNamePrefix = "",
4343
appId = "",
4444
roleLabels = Map.empty,
@@ -58,6 +58,6 @@ class RDriverFeatureStepSuite extends SparkFunSuite {
5858
.map(env => (env.getName, env.getValue))
5959
.toMap
6060
assert(envs(ENV_R_PRIMARY) === expectedMainResource)
61-
assert(envs(ENV_R_ARGS) === "5 7")
61+
assert(envs(ENV_R_ARGS) === "5 7 9")
6262
}
6363
}

0 commit comments

Comments
 (0)