virajjasani commented on code in PR #5230:
URL: https://github.com/apache/hadoop/pull/5230#discussion_r1051156833


##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/V2Migration.java:
##########
@@ -95,4 +99,13 @@ public static void v1GetObjectMetadataCalled() {
         + "will be changed as part of upgrading S3A to AWS SDK V2");
   }
 
+  /**
+   * Warns on use of getObjectMetadata.
+   */
+  public static void regionNotConfigured() {
+    WARN_ON_REGION_NOT_CONFIGURED.warn("A region has not been configured. 
Cross region support "
+            + "will be removed as part of upgrading S3A to AWS SDK V2. To 
avoid errors, set the "
+        + "bucket's region in {}.", AWS_REGION);
+  }

Review Comment:
   How about passing region name to the method and let the method decide 
whether to warn?
   
   ```
     public static void warnIfRegionNotConfigured(String regionName) {
       if (StringUtils.isBlank(regionName)) {
         WARN_ON_REGION_NOT_CONFIGURED.warn("A region has not been configured. 
Cross region support "
             + "will be removed as part of upgrading S3A to AWS SDK V2. To 
avoid errors, set the "
             + "bucket's region in {}.", AWS_REGION);
       }
     }
   
   ```
   



##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -234,6 +236,12 @@ protected AmazonS3 buildAmazonS3Client(
     AmazonS3ClientBuilder b = AmazonS3Client.builder();
     configureBasicParams(b, awsConf, parameters);
 
+    String region = getConf().getTrimmed(AWS_REGION);
+
+    if (StringUtils.isBlank(region)) {
+      V2Migration.regionNotConfigured();
+    }
+
     // endpoint set up is a PITA
     AwsClientBuilder.EndpointConfiguration epr
         = createEndpointConfiguration(parameters.getEndpoint(),

Review Comment:
   nit: `getConf().getTrimmed(AWS_REGION)` passed to 
`createEndpointConfiguration()` can be replaced with `region`



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