Github user mccheah commented on a diff in the pull request:
https://github.com/apache/spark/pull/21669#discussion_r215446604
--- Diff:
resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/submit/KubernetesDriverBuilder.scala
---
@@ -51,7 +51,11 @@ private[spark] class KubernetesDriverBuilder(
provideJavaStep: (
KubernetesConf[KubernetesDriverSpecificConf]
=> JavaDriverFeatureStep) =
- new JavaDriverFeatureStep(_)) {
+ new JavaDriverFeatureStep(_),
--- End diff --
Argument type is a function with the default value being assigned to the
default constructor. It might be cleaner to write this as follows:
- Constructor in the class declaration has _no_ default arguments
- Provide an empty arg constructor (`def this()`) which passes along all
default implementations to the full-args constructor.
In this case default arguments are hurting more than they are helping, and
in the unit tests, we always supply mocks for _every_ arg, meaning we always
use all defaults (real implementation) or no defaults (test implementation).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
