[K8S] KUBERNETES_EXECUTOR_REQUEST_CORES

[Jacek Laskowski]

Hi,

I'm curious why this line [1] uses sparkConf to find KUBERNETES_EXECUTOR_REQUEST_CORES while the next [2] goes to kubernetesConf?

(I'm not going to mention it's getOrElse case here and wait till I got OK to change this due to the above "misuse").

[1] https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStep.scala#L71

[2] https://github.com/apache/spark/blob/master/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/features/BasicExecutorFeatureStep.scala#L72

[attilapiros]

Hi,

This is just an extra unnecessary direct usage of the "sparkConf" member val (those two lines are added by two different PRs).

Actually both uses the same "sparkConf" to give back the config value, as
"KubernetesExecutorConf" extends the "KubernetesConf" [1] which uses the
passed "sparkConf" to get back the value in the get method [2].

So technically this does not cause any problem but it is better to harmonize
it and call the contains method directly on "kubernetesConf" (see the
method [3]) to avoid confusion for the next readers.

[1] https://github.com/apache/spark/blob/9b875ceada60732899053fbd90728b4944d1c03d/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesConf.scala#L132-L138

[2] https://github.com/apache/spark/blob/9b875ceada60732899053fbd90728b4944d1c03d/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesConf.scala#L67

[3] https://github.com/apache/spark/blob/9b875ceada60732899053fbd90728b4944d1c03d/resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/KubernetesConf.scala#L65

Best Regards,
Attila




--
Sent from: http://apache-spark-developers-list.1001551.n3.nabble.com/

---------------------------------------------------------------------
To unsubscribe e-mail: [e-mail]