Yikun commented on a change in pull request #35886:
URL: https://github.com/apache/spark/pull/35886#discussion_r828855941
##########
File path:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesUtils.scala
##########
@@ -381,4 +381,42 @@ object KubernetesUtils extends Logging {
}
}
}
+
+ /**
+ * This function builds the EnvVar objects for each key-value env.
+ */
+ @Since("3.3.1")
Review comment:
I think it should be `3.4.0`.
If you want to submit a PR for 3.3.1, you need first merge it in master
**after 3.3.0 released published**, then backport to 3.3 branch. I guess it's
not your intention.
##########
File path:
resource-managers/kubernetes/core/src/test/scala/org/apache/spark/deploy/k8s/KubernetesUtilsSuite.scala
##########
@@ -127,4 +127,34 @@ class KubernetesUtilsSuite extends SparkFunSuite with
PrivateMethodTester {
}
}
}
+
Review comment:
Looks like you have done test on utils.
It would be good if you can make sure we have exsiting UT to protected the
changes on featurestep.
##########
File path:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesUtils.scala
##########
@@ -381,4 +381,42 @@ object KubernetesUtils extends Logging {
}
}
}
+
+ /**
+ * This function builds the EnvVar objects for each key-value env.
+ */
+ @Since("3.3.1")
+ def buildEnvVarsWithKV(env: Seq[(String, String)]): Seq[EnvVar] = {
Review comment:
Random thoughts: now that we're refactoring, might we think `Map` be a
better choice than `Seq`?
##########
File path:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesUtils.scala
##########
@@ -381,4 +381,42 @@ object KubernetesUtils extends Logging {
}
}
}
+
+ /**
+ * This function builds the EnvVar objects for each key-value env.
+ */
+ @Since("3.3.1")
+ def buildEnvVarsWithKV(env: Seq[(String, String)]): Seq[EnvVar] = {
+ if (env.isEmpty) {
+ Seq.empty
+ } else {
+ env.filter( env => env._2 != null)
+ .map { env =>
+ new EnvVarBuilder()
+ .withName(env._1)
+ .withValue(env._2)
+ .build()
+ }
+ }
+ }
+
+ /**
+ * This function builds he EnvVar objects for each field ref env.
+ */
+ @Since("3.3.1")
Review comment:
likewise
##########
File path:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesUtils.scala
##########
@@ -381,4 +381,42 @@ object KubernetesUtils extends Logging {
}
}
}
+
+ /**
+ * This function builds the EnvVar objects for each key-value env.
+ */
+ @Since("3.3.1")
+ def buildEnvVarsWithKV(env: Seq[(String, String)]): Seq[EnvVar] = {
+ if (env.isEmpty) {
+ Seq.empty
+ } else {
+ env.filter( env => env._2 != null)
+ .map { env =>
+ new EnvVarBuilder()
+ .withName(env._1)
+ .withValue(env._2)
+ .build()
+ }
+ }
+ }
+
+ /**
+ * This function builds he EnvVar objects for each field ref env.
Review comment:
nits: s/he/the
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
