mbutrovich commented on code in PR #7221:
URL: https://github.com/apache/datafusion-comet/pull/7221#discussion_r4383763946
##########
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:
> 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.
Should the fallback reason say so too? An endpoint like
`https://h/a/%2e%2e/t/oauth2/token` is `https` and has a tenant segment, but it
fails the check at [line
535](https://github.com/apache/datafusion-comet/blob/5ee970e6c528544e2a528b01ea7f4126abd2e1ac/spark/src/main/scala/org/apache/comet/objectstore/AbfsAuthResolver.scala#L535).
The
[reason](https://github.com/apache/datafusion-comet/blob/5ee970e6c528544e2a528b01ea7f4126abd2e1ac/spark/src/main/scala/org/apache/comet/objectstore/AbfsAuthResolver.scala#L504-L506)
then tells the user it is "not an https URL with a tenant path segment", which
sends them looking for the wrong problem. The native store builds for the
percent escape and backslash cases, per the [`oauth_endpoint_parts`
test](https://github.com/apache/datafusion-comet/blob/5ee970e6c528544e2a528b01ea7f4126abd2e1ac/native/core/src/parquet/objectstore/azure.rs#L982-L984).
Something like "an endpoint that is not an https URL with a tenant path
segment and no percent escape
, backslash or dot segment" would match the check.
[`datasources.md`](https://github.com/apache/datafusion-comet/blob/5ee970e6c528544e2a528b01ea7f4126abd2e1ac/docs/source/user-guide/latest/datasources.md?plain=1#L484)
could list these cases next to the `http://` and no-tenant ones.
--
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]
