[K8S] KUBERNETES_EXECUTOR_REQUEST_CORES

classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

[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").

Reply | Threaded
Open this post in threaded view
|

Re: [K8S] KUBERNETES_EXECUTOR_REQUEST_CORES

attilapiros
This post was updated on .
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: dev-unsubscribe@spark.apache.org