comphead commented on code in PR #7221:
URL: https://github.com/apache/datafusion-comet/pull/7221#discussion_r4378973080
##########
spark/src/main/scala/org/apache/comet/objectstore/NativeConfig.scala:
##########
@@ -341,19 +341,8 @@ object NativeConfig {
val scheme =
Option(uri.getScheme).map(_.toLowerCase(Locale.ROOT)).getOrElse("file")
import scala.jdk.CollectionConverters._
- val options = scala.collection.mutable.Map[String, String]()
-
- // Scheme lists consumed on the native side ride along in the options map
(both empty by
- // default).
- val libhdfsSchemes = hadoopConf.get(COMET_LIBHDFS_SCHEMES_KEY)
- if (StringUtils.isNotBlank(libhdfsSchemes)) {
- options(COMET_LIBHDFS_SCHEMES_KEY) = libhdfsSchemes
- }
- val s3CompliantRaw = hadoopConf.get(COMET_S3_COMPLIANT_SCHEMES_KEY)
- if (StringUtils.isNotBlank(s3CompliantRaw)) {
- options(COMET_S3_COMPLIANT_SCHEMES_KEY) = s3CompliantRaw
- }
- val s3CompliantSchemes = parseSchemeSet(s3CompliantRaw)
+ val options = scala.collection.mutable.Map[String, String]() ++=
schemeListOptions(hadoopConf)
+ val s3CompliantSchemes =
parseSchemeSet(hadoopConf.get(COMET_S3_COMPLIANT_SCHEMES_KEY))
Review Comment:
Now that the raw value is no longer kept here, this is the body of
`resolveS3CompliantSchemes` at line 91, so it can call
`resolveS3CompliantSchemes(hadoopConf)`.
##########
spark/src/main/scala/org/apache/comet/objectstore/AbfsAuthResolver.scala:
##########
@@ -467,6 +469,42 @@ 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 safeAuthority =
+ if (at < 0) authority
+ else authority.substring(0, at).takeWhile(_ != ':') +
authority.substring(at)
+ val location = s"${uri.getScheme}://$safeAuthority"
+ 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)),
+ _) =>
Review Comment:
This arm mirrors the provider check in `resolved_settings` in `azure.rs`,
but that file still rejects some `Resolved` client credentials accounts.
`client_creds_settings` errors when `oauth_endpoint_parts` finds no tenant
segment, and `datasources.md` says an `http://` client endpoint fails, since
the store is built without `allow_http`. Hadoop posts to
`fs.azure.account.oauth2.client.endpoint` as given, so Spark can read such an
account while the native scan still plans and every task fails. That is the
failure this PR removes for the other cases.
Could `nativeFallbackReason` also decline a client credentials endpoint that
is not `https` or has no tenant segment? If those are left out on purpose, the
new `azure.rs` doc line could say that scans can still reach these errors. I
traced this from the code and have not run it.
--
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]
