dwsmith1983 opened a new pull request, #5872:
URL: https://github.com/apache/datafusion-comet/pull/5872

   ## Which issue does this PR close?
   
   Closes #4245, closes #2802.
   
   ## Rationale for this change
   
   Two gaps in how the native S3 store is configured from `fs.s3a.*` settings.
   
   The profile credentials provider ignored `fs.s3a.auth.profile.name` and 
`fs.s3a.auth.profile.file`, so a job that selects a named profile or a 
non-default credentials file on the Hadoop side got the SDK defaults on the 
native side.
   
   `fs.s3a.path.style.access` was applied inverted: `true` set object_store's 
`virtual_hosted_style_request` to true and then appended `/bucket` to the 
endpoint, which object_store, treating a virtual-hosted endpoint as already 
containing the bucket, sent as a path-style URL anyway. The net effect was that 
every custom endpoint was addressed path-style whatever the flag said, and 
virtual-hosted addressing (`bucket.host`) was never produced.
   
   ## What changes are included in this PR?
   
   - `CredentialProviderMetadata::Profile` carries `name` and `file`, read 
through the existing per-bucket config lookup with blanks treated as unset, and 
passed to the SDK builder. The file is loaded in credentials-file format, which 
is what Hadoop's `ProfileAWSCredentialsProvider` does; with no file the SDK 
default applies as before. `aws-runtime` becomes a direct dependency because 
the file-kind types re-exported by `aws_config` are deprecated aliases; it was 
already in the tree.
   - `path.style.access` is parsed the way Hadoop's `Configuration.getBoolean` 
parses it (default false, non-boolean text falls back to the default), 
`virtual_hosted_style_request` is its negation and is always passed, and 
`normalize_endpoint` returns the endpoint together with the effective mode so 
the two cannot disagree: virtual-hosted rebuilds 
`scheme://bucket.host[:port][/path]`, path-style leaves the endpoint alone for 
object_store to append the bucket, and an IP-literal host forces path-style the 
way the AWS SDK's endpoint rules do, so `http://127.0.0.1:9000` keeps working 
without the flag. `localhost` is not special-cased, matching Hadoop. The 
`s3.amazonaws.com` skip is unchanged.
   - The data sources page lists the two profile keys and explains the 
addressing rule.
   
   Behavior change: a custom `fs.s3a.endpoint` with `fs.s3a.path.style.access` 
unset is now addressed virtual-hosted, as Hadoop S3A addresses it. Deployments 
on MinIO, Ceph RGW or similar services behind a hostname that relied on the 
previous always-path-style behavior need `fs.s3a.path.style.access=true`, which 
Hadoop already requires for those services; IP-address endpoints keep working 
either way. Vendor alias schemes are unaffected because the JVM side already 
synthesizes the flag for them.
   
   ## How are these changes tested?
   
   52 unit tests in the S3 module, 11 of them written first and failing on the 
previous code (the profile metadata carried no name or file; 
`path.style.access` unset produced no flag; a hostname endpoint was never 
rewritten; an IP endpoint was rewritten to `bucket.127.0.0.1`). Coverage: the 
flag unset, `true`, `false`, mixed case with whitespace, and an invalid value; 
per-bucket override of the flag and of the endpoint, each against a global 
value set the other way; the `s3.amazonaws.com` skip in both modes; 
scheme-less, `http://`, port, trailing slash, path suffix, an AWS regional host 
and a dotted bucket name; IPv4 and IPv6 hosts with and without a port; 
`create_store` called with a custom endpoint in each mode and with an IP 
endpoint; profile name only, file only, both, neither, blank, trimmed and 
per-bucket, plus the provider chain building with the profile provider among 
others. Two existing tests that had encoded the inverted flag were replaced; 
one that asserted an empty confi
 g now asserts the endpoint key is absent, since the flag is always present.
   
   The `create_store` calls show object_store accepts each flag and endpoint 
pair but do not issue a request, since object_store parses the endpoint on 
first use. The four Scala tests that set an endpoint either set path-style 
access or are pure config-translation tests, so none needed changing. Full core 
crate suite passes, clippy and fmt clean.
   


-- 
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]

Reply via email to