dwsmith1983 commented on code in PR #7221:
URL: https://github.com/apache/datafusion-comet/pull/7221#discussion_r4371408783
##########
spark/src/main/scala/org/apache/comet/objectstore/AbfsAuthResolver.scala:
##########
@@ -467,6 +469,38 @@ private[comet] object AbfsAuthResolver extends Logging {
Markers.ERROR_MESSAGE -> message)
}
+ /**
+ * Why the native store cannot read `uri` under `outcome`, or None when it
can: a mechanism
+ * Hadoop named that runs Java code per request, a built-in OAuth provider
with no
+ * `object_store` counterpart, or a resolution that failed. All are known on
the driver, so the
+ * scan falls back to Spark's own reader, which authenticates through
hadoop-azure, instead of
+ * planning natively and failing every task.
+ */
+ def nativeFallbackReason(uri: URI, outcome: Outcome): Option[String] = {
+ // The container and host only: a password in the user info stays out of
the reason.
+ val authority = Option(uri.getRawAuthority).getOrElse("")
+ val at = authority.lastIndexOf('@')
+ val location = s"${uri.getScheme}://" +
+ (if (at < 0) authority
+ else authority.substring(0, at).takeWhile(_ != ':') +
authority.substring(at))
+ def unsupported(mechanism: String): String =
+ s"Native scan does not support ABFS authentication $mechanism for
$location"
+ outcome match {
+ case Declined(authType, providerClass, assertionProviderClass) =>
+ val classes = providerClass.map(c => s" with provider
$c").getOrElse("") +
+ assertionProviderClass.map(c => s" and client assertion provider
$c").getOrElse("")
+ Some(unsupported(s"auth type $authType$classes"))
+ case Resolved(
+ AuthTypes.OAUTH,
+ Some(c @ (Providers.REFRESH_TOKEN | Providers.USER_PASSWORD)),
+ _) =>
+ Some(unsupported(s"auth type ${AuthTypes.OAUTH} with provider $c"))
+ case Failed(_, message) =>
+ Some(s"Hadoop ABFS authentication failed on the driver for $location:
$message")
+ case _: Resolved | Unconfigured => None
Review Comment:
This line decides between native and Spark for every OAuth account, but the
"stay native" test at line 836 covers only `Unconfigured` and SharedKey. If the
RefreshToken/UserPassword arm above were widened to `Resolved(AuthTypes.OAUTH,
Some(_), _)`, every ClientCreds, MSI and Workload Identity scan would fall
back. Spark reads them fine, so nothing would fail. Could the test add one
`Resolved` case per supported provider, plus SAS with a fixed token, each
asserting `Right` with `comet.azure.resolution=resolved`?
`AbfsAuthResolverSuite` already has the key sets for each.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
