HyukjinKwon commented on a change in pull request #32894:
URL: https://github.com/apache/spark/pull/32894#discussion_r650602478
##########
File path:
core/src/main/scala/org/apache/spark/deploy/security/HBaseDelegationTokenProvider.scala
##########
@@ -97,7 +97,8 @@ private[security] class HBaseDelegationTokenProvider
creds.addToken(token.getService, token)
} catch {
case NonFatal(e) =>
- logWarning(s"Failed to get token from service $serviceName", e)
+ logWarning(s"Failed to get token from service $serviceName due to $e.
" +
+ s"If HBase is not used, set spark.security.credentials.hbase.enabled
to false")
Review comment:
When this is explicitly enabled, users or admins would likely know how
to disable this .. I am not very sure if this is worthwhile. Stacktrace might
be a bit noisy but ideally HBase other other services should not be down if
they are in use, and should be up as soon as possible. Given that this warning
is not thrown by default easily, I think being noisy here is a-okay. I don't
feel strongly on this.
Code-wise, we'll have to fix other instances too together:
```
core/src/main/scala/org/apache/spark/deploy/security/HBaseDelegationTokenProvider.scala:
logWarning(s"Failed to get token from service $serviceName due to " +
e +
core/src/main/scala/org/apache/spark/deploy/security/HBaseDelegationTokenProvider.scala:
logWarning(s"Failed to get token from service $serviceName", e)
core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala:
logWarning(s"Failed to get token from service $serviceName", e)
external/kafka-0-10-token-provider/src/main/scala/org/apache/spark/kafka010/KafkaDelegationTokenProvider.scala:
logWarning(s"Failed to get token from service: $serviceName " +
sql/hive/src/main/scala/org/apache/spark/sql/hive/security/HiveDelegationTokenProvider.scala:
logWarning(s"Failed to get token from service $serviceName", e)
```
and here we might have to use the configuration template (e.g., at
`HadoopDelegationTokenManager`) and `serviceName` instead of hard-coding
`spark.security.credentials.hbase.enabled`.
Lastly, we would have tho show the error message (`e.getMessage`) and
exception type together.
##########
File path:
core/src/main/scala/org/apache/spark/deploy/security/HBaseDelegationTokenProvider.scala
##########
@@ -97,7 +97,8 @@ private[security] class HBaseDelegationTokenProvider
creds.addToken(token.getService, token)
} catch {
case NonFatal(e) =>
- logWarning(s"Failed to get token from service $serviceName", e)
+ logWarning(s"Failed to get token from service $serviceName due to $e.
" +
+ s"If HBase is not used, set spark.security.credentials.hbase.enabled
to false")
Review comment:
When this is explicitly enabled, users or admins would likely know how
to disable this .. I am not very sure if this is worthwhile. Stacktrace might
be a bit noisy but ideally HBase other other services should not be down if
they are in use, and should be up as soon as possible. Given that this warning
is not thrown by default or easily, I think being noisy here is a-okay. I don't
feel strongly on this.
Code-wise, we'll have to fix other instances too together:
```
core/src/main/scala/org/apache/spark/deploy/security/HBaseDelegationTokenProvider.scala:
logWarning(s"Failed to get token from service $serviceName due to " +
e +
core/src/main/scala/org/apache/spark/deploy/security/HBaseDelegationTokenProvider.scala:
logWarning(s"Failed to get token from service $serviceName", e)
core/src/main/scala/org/apache/spark/deploy/security/HadoopFSDelegationTokenProvider.scala:
logWarning(s"Failed to get token from service $serviceName", e)
external/kafka-0-10-token-provider/src/main/scala/org/apache/spark/kafka010/KafkaDelegationTokenProvider.scala:
logWarning(s"Failed to get token from service: $serviceName " +
sql/hive/src/main/scala/org/apache/spark/sql/hive/security/HiveDelegationTokenProvider.scala:
logWarning(s"Failed to get token from service $serviceName", e)
```
and here we might have to use the configuration template (e.g., at
`HadoopDelegationTokenManager`) and `serviceName` instead of hard-coding
`spark.security.credentials.hbase.enabled`.
Lastly, we would have tho show the error message (`e.getMessage`) and
exception type together.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
