andygrove commented on code in PR #7221:
URL: https://github.com/apache/datafusion-comet/pull/7221#discussion_r4379120692
##########
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:
Done. `objectStoreOptionsAndAliasOptions` now calls
`resolveS3CompliantSchemes(hadoopConf)`. Pushed in 8c7c35d26c.
##########
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:
I agree, and I traced the same two cases. `nativeFallbackReason` now
declines a `Resolved` `ClientCredsTokenProvider` whose endpoint is not `https`
or has no tenant segment, using the same tenant rule as `oauth_endpoint_parts`
(the segment before the last `oauth2`, else the first). The check is
conservative where `java.net.URI` and the native URL parser could disagree: a
percent escape, a backslash or a dot segment falls back rather than being
guessed at. The reason names the provider but not the endpoint, since the
endpoint can carry user info.
`NativeConfigSuite` now has the fallback cases for an `http` endpoint and
one with no tenant, a password-in-endpoint check, and a table of endpoints
taken from the `azure.rs` unit test. `datasources.md` and the `azure.rs` doc
line say these cases fall back. `NativeConfigSuite`, `AbfsAuthResolverSuite`
and `AbfsAuthParitySuite` pass on the default Spark 4.1 profile. Pushed in
8c7c35d26c.
One related thing I left alone: Workload Identity passes
`fs.azure.account.oauth2.msi.authority` to the store the same way, so an
`http://` authority there would fail natively too. I did not trace that case,
so it is not in this change.
--
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]
