dwsmith1983 commented on code in PR #5872:
URL: https://github.com/apache/datafusion-comet/pull/5872#discussion_r4267506241
##########
native/core/src/parquet/objectstore/s3.rs:
##########
@@ -352,24 +354,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;
+
+ // Extract endpoint configuration and shape it for the selected addressing
style. The flag is
+ // taken from the normalized result so the endpoint and the flag never
disagree. A custom
+ // endpoint decides the bucket-name rule by its own scheme inside
normalize_endpoint; the
+ // default AWS endpoint is HTTPS, so the rule applies to it here without
dots.
+ let custom_endpoint = get_config_trimmed(configs, bucket, "endpoint")
+ .and_then(|endpoint| normalize_endpoint(endpoint, bucket,
virtual_hosted_style_request));
+ match custom_endpoint {
+ Some(normalized) => {
+ virtual_hosted_style_request =
normalized.virtual_hosted_style_request;
+ s3_configs.insert(AmazonS3ConfigKey::Endpoint,
normalized.endpoint);
+ }
+ None => {
Review Comment:
The probe now follows the same rule. `region_probe_url` checks
`is_virtual_hostable_bucket(bucket, false)` and sends a bucket that fails it to
`https://s3.amazonaws.com/{bucket}`. That path-style HEAD still returns
`x-amz-bucket-region` on the 301 or 403, and a 404 for a missing bucket, so the
header handling is unchanged. `test_region_probe_url` covers plain, dotted,
mixed-case, underscored, hyphen-edged, too-short and too-long names, and the
dotted and legacy rows fail against the old URL. Dotted and mixed-case buckets
no longer need `fs.s3a.endpoint.region`, so the data sources paragraph stays as
it is.
--
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]
