Github user foxish commented on a diff in the pull request: https://github.com/apache/spark/pull/20148#discussion_r159588871 --- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/MountSecretsBootstrap.scala --- @@ -28,20 +28,26 @@ private[spark] class MountSecretsBootstrap(secretNamesToMountPaths: Map[String, * * @param pod the pod into which the secret volumes are being added. * @param container the container into which the secret volumes are being mounted. + * @param addNewVolumes whether to add new secret volumes for the secrets. * @return the updated pod and container with the secrets mounted. */ - def mountSecrets(pod: Pod, container: Container): (Pod, Container) = { + def mountSecrets( --- End diff -- Can we separate this method into `addSecretVolumes` and `mountSecrets`? The former would just need the pod argument, and the latter would take the container as argument. That way, we could probably separate this out better and it would make it more readable than the branching. WDYT?
--- --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org