dwsmith1983 commented on code in PR #7127:
URL: https://github.com/apache/datafusion-comet/pull/7127#discussion_r4383850143
##########
native/core/src/parquet/parquet_support.rs:
##########
@@ -620,14 +622,13 @@ fn uses_azure_container_key(scheme: &str) -> bool {
|| scheme.starts_with("abfss+comet-")
}
-pub(crate) fn object_store_authority(url: &Url) -> &str {
- let start = if uses_azure_container_key(url.scheme()) {
+pub(crate) fn object_store_authority(url: &Url) -> Cow<'_, str> {
+ if uses_azure_container_key(url.scheme()) {
// ABFS URLs encode the container in the userinfo.
- url::Position::BeforeUsername
+ authority_without_password(url)
Review Comment:
Fixed in `CometFileKeyUnwrapper`. Registration and lookup now share one
cache key: S3 aliases fold to `s3a` as before, and for `abfs`/`abfss` the
scheme is lowercased and the password is dropped from the authority by the same
rule as native `authority_without_password` (the user ends at the first `:`,
the host follows the last `@`, an empty user leaves only the host). The
decryption factory still gets the original Hadoop `Path`, and the
missing-retriever error prints the key rather than the caller's path.
`TestCometFileKeyUnwrapper.abfsKeyStoredWithPasswordIsFoundWithout` stores a
retriever under `abfss://data:[email protected]/t/r.parquet`
and reads the key back through the password-free path native sends, and through
the stored path. `abfsKeyDropsThePasswordAsNativeDoes` covers the key itself:
password, a password containing `:`, an empty password, port, empty container,
no user info, an already clean authority, an upper-case scheme, an authority
ended by `?` or `#`, `@` and `:` in the path, a URL with no authority, and
non-ABFS paths left alone. `missingAbfsKeyErrorOmitsThePassword` checks the
error text.
--
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]
