vanzin commented on a change in pull request #27191: [SPARK-30495][SS] Consider 
spark.security.credentials.kafka.enabled and cluster configuration when 
checking latest delegation token
URL: https://github.com/apache/spark/pull/27191#discussion_r366503933
 
 

 ##########
 File path: 
external/kafka-0-10-token-provider/src/main/scala/org/apache/spark/kafka010/KafkaTokenUtil.scala
 ##########
 @@ -291,13 +291,14 @@ private[spark] object KafkaTokenUtil extends Logging {
   }
 
   def isConnectorUsingCurrentToken(
+      sparkConf: SparkConf,
       params: ju.Map[String, Object],
       clusterConfig: Option[KafkaTokenClusterConf]): Boolean = {
-    if (params.containsKey(SaslConfigs.SASL_JAAS_CONFIG)) {
+    if (sparkConf.getBoolean("spark.security.credentials.kafka.enabled", true) 
&&
 
 Review comment:
   I'm kinda wondering if this extra check is needed, instead of just `if 
(clusterConfig.isDefined && params.containsKey(SaslConfigs.SASL_JAAS_CONFIG)`.
   
   If it's really needed it's ok, although I'm not a big fan of hardcoding that 
config name here (instead of trying to reuse 
`HadoopDelegationTokenManager.isServiceEnabled`).
   
   On a side note this method would be better if called `needTokenUpdate`; it's 
kinda weird to return `true` (as in "yes it's using the current token") when 
there is no token at all.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to