Github user mccheah commented on a diff in the pull request:
https://github.com/apache/spark/pull/21092#discussion_r182607048
--- Diff:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicDriverFeatureStep.scala
---
@@ -88,15 +94,22 @@ private[spark] class BasicDriverFeatureStep(
.addToRequests("memory", driverMemoryQuantity)
.addToLimits("memory", driverMemoryQuantity)
.endResources()
- .addToArgs("driver")
+ .addToArgs(driverDockerContainer)
.addToArgs("--properties-file", SPARK_CONF_PATH)
.addToArgs("--class", conf.roleSpecificConf.mainClass)
- // The user application jar is merged into the spark.jars list and
managed through that
- // property, so there is no need to reference it explicitly here.
- .addToArgs(SparkLauncher.NO_RESOURCE)
- .addToArgs(conf.roleSpecificConf.appArgs: _*)
- .build()
+ val driverContainer =
+ if (driverDockerContainer == "driver-py") {
--- End diff --
I think in general I'd prefer having two separate step types here. They can
share some logic in either a utils class or a shared superclass. But you only
apply one step type for Java apps vs one step type for Python apps.
Another way is to have the basic driver step only do work that would be
strictly agnostic of python vs java, and then have a separate step for either
Java or Python; the orchestrator picks which one to invoke based on the app
resource type. To do this I think the step's constructor needs to take more
than just the `KubernetesConf` as an argument - it needs to take the
appropriate specifically-typed `MainAppResource` as an argument in the
constructor as well. This breaks the convention that we've set so far but for
now that's probably ok, as long as we don't get parameter length blowup as we
go forward.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
