dwsmith1983 commented on code in PR #5872:
URL: https://github.com/apache/datafusion-comet/pull/5872#discussion_r4104795269
##########
native/core/src/parquet/objectstore/s3.rs:
##########
@@ -239,24 +240,33 @@ fn extract_s3_config_options(
s3_configs.insert(AmazonS3ConfigKey::Region, region.to_string());
}
- // Extract and handle path style access (virtual hosted style)
- let mut virtual_hosted_style_request = false;
- if let Some(path_style) = get_config_trimmed(configs, bucket,
"path.style.access") {
- virtual_hosted_style_request = path_style.to_lowercase() == "true";
- s3_configs.insert(
- AmazonS3ConfigKey::VirtualHostedStyleRequest,
- virtual_hosted_style_request.to_string(),
- );
- }
-
- // Extract endpoint configuration and modify if virtual hosted style is
enabled
- if let Some(endpoint) = get_config_trimmed(configs, bucket, "endpoint") {
- let normalized_endpoint =
- normalize_endpoint(endpoint, bucket, virtual_hosted_style_request);
- if let Some(endpoint) = normalized_endpoint {
- s3_configs.insert(AmazonS3ConfigKey::Endpoint, endpoint);
+ // Hadoop defaults fs.s3a.path.style.access to false, which means
virtual-hosted addressing,
+ // and treats non-boolean text as that default. object_store expects the
inverse flag.
+ let path_style_access = get_config_trimmed(configs, bucket,
"path.style.access")
+ .is_some_and(|value| value.eq_ignore_ascii_case("true"));
+ let mut virtual_hosted_style_request = !path_style_access;
Review Comment:
> Could we apply the SDK's DNS bucket-name eligibility rules independently
of the HTTPS dotted-name rule and add a final-URL regression test?
Yes. `is_virtual_hostable_bucket` now follows SDK 2.29.52's
`isVirtualHostableS3Bucket`: without dots the name must match
`[a-z\d][a-z\d\-]{1,61}[a-z\d]`, and over plain HTTP, where the rules pass
`allowSubdomains=true`, dots are allowed but an IPv4-shaped name and adjacent
`.`/`-` pairs are not. A name that fails goes path-style whatever
`fs.s3a.path.style.access` says. The HTTPS dotted-name rule is now the
`allow_dots=false` case of the same check, and it applies to both the default
AWS endpoint and a custom one.
`test_bucket_the_sdk_cannot_virtual_host_stays_path_style` reads the final
URL from an object_store presigned GET. `s3a://LegacyBucket/object` with
`fs.s3a.endpoint.region=us-east-1`, no endpoint and the flag unset now resolves
to `https://s3.us-east-1.amazonaws.com/LegacyBucket/object`, the same as base.
The test also covers an underscore, a leading or trailing hyphen, a
64-character name, and, over an HTTP endpoint, an IPv4-shaped name, a dot next
to a hyphen and a mixed-case name. A lowercase `legacy-bucket` still resolves
to `https://legacy-bucket.s3.us-east-1.amazonaws.com/object`. The user guide
now describes the name rule.
--
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]
