Github user galv commented on a diff in the pull request:
https://github.com/apache/spark/pull/21511#discussion_r193945410
--- Diff:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStep.scala
---
@@ -150,6 +152,16 @@ private[spark] class BasicExecutorFeatureStep(
.endResources()
.build()
}.getOrElse(executorContainer)
+ val containerWithLimitGpus = executorLimitGpus.map { limitGpus =>
+ val executorGpuLimitQuantity = new QuantityBuilder(false)
+ .withAmount(limitGpus)
+ .build()
+ new ContainerBuilder(containerWithLimitCores)
+ .editResources()
+ .addToLimits(gpuProvider+"/gpu", executorGpuLimitQuantity)
--- End diff --
Style: whitespace around the "+".
More importantly, you're assuming that the name of the resource is always
going to be "gpu". I'm not sure this is a great idea.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
