steveloughran commented on code in PR #6479:
URL: https://github.com/apache/hadoop/pull/6479#discussion_r1472852413


##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -290,35 +290,34 @@ private <BuilderT extends S3BaseClientBuilder<BuilderT, 
ClientT>, ClientT> void
     builder.fipsEnabled(fipsEnabled);
 
     if (endpoint != null) {
-      boolean overrideEndpoint = true;
       checkArgument(!fipsEnabled,
           "%s : %s", ERROR_ENDPOINT_WITH_FIPS, endpoint);
       boolean endpointEndsWithCentral =
           endpointStr.endsWith(CENTRAL_ENDPOINT);
-      // No region was configured or the endpoint is central,
+
+      // No region was configured,
       // determine the region from the endpoint.
-      if (region == null || endpointEndsWithCentral) {
+      if (region == null) {
         region = getS3RegionFromEndpoint(endpointStr,
             endpointEndsWithCentral);
         if (region != null) {
           origin = "endpoint";
-          if (endpointEndsWithCentral) {
-            // No need to override endpoint with "s3.amazonaws.com".
-            // Let the client take care of endpoint resolution. Overriding
-            // the endpoint with "s3.amazonaws.com" causes 400 Bad Request
-            // errors for non-existent buckets and objects.
-            // ref: https://github.com/aws/aws-sdk-java-v2/issues/4846
-            overrideEndpoint = false;
-            builder.crossRegionAccessEnabled(true);
-            origin = "origin with cross region access";
-            LOG.debug("Enabling cross region access for endpoint {}",
-                endpointStr);
-          }
         }
       }
-      if (overrideEndpoint) {
+
+      // No need to override endpoint with "s3.amazonaws.com".
+      // Let the client take care of endpoint resolution. Overriding
+      // the endpoint with "s3.amazonaws.com" causes 400 Bad Request
+      // errors for non-existent buckets and objects.
+      // ref: https://github.com/aws/aws-sdk-java-v2/issues/4846
+      if (!endpointEndsWithCentral) {
         builder.endpointOverride(endpoint);
         LOG.debug("Setting endpoint to {}", endpoint);
+      } else {
+        builder.crossRegionAccessEnabled(true);
+        origin = "origin with cross region access";

Review Comment:
   "central endpoint with cross region access"



##########
hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEndpointRegion.java:
##########
@@ -257,6 +283,83 @@ public void testWithVPCE() throws Throwable {
     expectInterceptorException(client);
   }
 
+  @Test
+  public void testCentralEndpointWithUSWest2Region() throws Throwable {
+    describe("Access bucket using central endpoint and us-west-2 region");
+    final Configuration conf = getConfiguration();
+    removeBaseAndBucketOverrides(conf, ENDPOINT, AWS_REGION);
+
+    final Configuration newConf = new Configuration(conf);
+
+    newConf.set(ENDPOINT, CENTRAL_ENDPOINT);
+    newConf.set(AWS_REGION, US_WEST_2);
+
+    newFS = new S3AFileSystem();
+    newFS.initialize(getFileSystem().getUri(), newConf);
+
+    assertOpsUsingNewFs();
+  }
+
+  @Test
+  public void testCentralEndpointWithEUWest2Region() throws Throwable {

Review Comment:
   let's go with a public bucket



##########
hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/aws_sdk_v2_changelog.md:
##########
@@ -93,6 +93,20 @@ the bucket. The behaviour has now been changed to:
   response. Otherwise it will throw an error with status code 301 permanently 
moved. This error
   contains the region of the bucket in its header, which we can then use to 
configure the client.
 

Review Comment:
   * put it in `connecting.md`, link in index.md as "how to connect"
   * Highlight that it is complex and improving, so may be considered unstable.
   * Link to `third_party_stores.html` to make clear if you are working with 
third party stores to look there.



-- 
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: common-issues-unsubscr...@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org
For additional commands, e-mail: common-issues-h...@hadoop.apache.org

Reply via email to