[GitHub] spark pull request #20451: [SPARK-23146][WIP] Support client mode for Kubern...

[tnachen <gi...@git.apache.org>]

Github user tnachen commented on a diff in the pull request:

    https://github.com/apache/spark/pull/20451#discussion_r199340748
  
    --- Diff: resource-managers/kubernetes/core/src/main/scala/org/apache/spark/deploy/k8s/SparkKubernetesClientFactory.scala ---
    @@ -88,6 +103,56 @@ private[spark] object SparkKubernetesClientFactory {
         new DefaultKubernetesClient(httpClientWithCustomDispatcher, config)
       }
     
    +  def createOutClusterKubernetesClient(
    +                             master: String,
    +                             namespace: Option[String],
    +                             kubernetesAuthConfPrefix: String,
    +                             sparkConf: SparkConf,
    +                             maybeServiceAccountToken: Option[File],
    +                             maybeServiceAccountCaCert: Option[File]): KubernetesClient = {
    +     val oauthTokenFileConf = s"$kubernetesAuthConfPrefix.$OAUTH_TOKEN_FILE_CONF_SUFFIX"
    +     val oauthTokenConf = s"$kubernetesAuthConfPrefix.$OAUTH_TOKEN_CONF_SUFFIX"
    +     val oauthTokenFile = sparkConf.getOption(oauthTokenFileConf)
    +       .map(new File(_))
    +       .orElse(maybeServiceAccountToken)
    +     val oauthTokenValue = sparkConf.getOption(oauthTokenConf)
    +     OptionRequirements.requireNandDefined(
    --- End diff --
    
    Since it's only used once I'm not sure it warrents a separate file/method for checking Options.
    Also the method signature isn't quite clear for me what it does. (especially requireN)
    How about just a simple match that @squito suggested here?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org