andygrove commented on code in PR #7221:
URL: https://github.com/apache/datafusion-comet/pull/7221#discussion_r4388157268
##########
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 went with the probe. `NativeBase.abfsStoreError(url, keys, values)` runs
`azure::create_store`, the same call the scan makes, on the options `toOptions`
builds for a `Resolved` outcome. It returns the error, or null if the store
builds. `nativeFallbackReason` now asks it for every `Resolved` outcome and
puts native's message in the reason. That removes `clientCredsEndpointUsable`,
`httpsHost`, the RefreshToken/UserPassword arm and the copied endpoint table,
so the rule lives only in `azure.rs`. Both of your points follow from that:
- The reason is native's own error, such as ``...
`fs.azure.account.oauth2.client.endpoint` is not an `https` URL ...`` or ``...
has no `scheme://host` origin or no tenant segment before `oauth2` ``. It names
the key, never the value.
- The percent escape, backslash and dot segment endpoints are no longer
declined. Native builds a store for them, as the `oauth_endpoint_parts` test
pins, so they stay native. `NativeConfigSuite` now checks that, so
`datasources.md` has nothing extra to list.
Native now rejects an `http` client credentials endpoint or Workload
Identity authority when it builds the store, instead of on every token request,
which gives native writes the clear error up front. The probe gets only the
container and host, so a password in the file URL never reaches it. If the
library is not loaded, the probe assumes the store builds, as
`isObjectStoreSchemeSupported` does.
New `azure.rs` tests cover `store_error` and the `http` errors.
`NativeConfigSuite`, `AbfsAuthResolverSuite`, `AbfsAuthParitySuite` and
`AbfsScanFallbackSuite` pass on the default Spark 4.1 profile, and clippy is
clean. Pushed in beda9508f.
--
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]
