mukund-thakur commented on code in PR #6479:
URL: https://github.com/apache/hadoop/pull/6479#discussion_r1462409406
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -354,20 +360,21 @@ private static URI getS3Endpoint(String endpoint, final
Configuration conf) {
/**
* Parses the endpoint to get the region.
- * If endpoint is the central one, use US_EAST_1.
+ * If endpoint is the central one, use US_EAST_2.
*
* @param endpoint the configure endpoint.
+ * @param endpointEndsWithCentral true if the endpoint is configured as
central.
* @return the S3 region, null if unable to resolve from endpoint.
*/
- private static Region getS3RegionFromEndpoint(String endpoint) {
+ private static Region getS3RegionFromEndpoint(String endpoint, boolean
endpointEndsWithCentral) {
- if(!endpoint.endsWith(CENTRAL_ENDPOINT)) {
+ if (!endpointEndsWithCentral) {
LOG.debug("Endpoint {} is not the default; parsing", endpoint);
return AwsHostNameUtils.parseSigningRegion(endpoint,
S3_SERVICE_NAME).orElse(null);
}
- // endpoint is for US_EAST_1;
- return Region.US_EAST_1;
+ // endpoint is for US_EAST_2;
+ return Region.US_EAST_2;
Review Comment:
changing this causes confusion. Maybe its better to the use the variable
present in Constants.
##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -294,9 +295,14 @@ private <BuilderT extends S3BaseClientBuilder<BuilderT,
ClientT>, ClientT> void
builder.endpointOverride(endpoint);
// No region was configured, try to determine it from the endpoint.
if (region == null) {
- region = getS3RegionFromEndpoint(parameters.getEndpoint());
+ boolean endpointEndsWithCentral =
endpointStr.endsWith(CENTRAL_ENDPOINT);
+ region = getS3RegionFromEndpoint(endpointStr, endpointEndsWithCentral);
if (region != null) {
origin = "endpoint";
+ if (endpointEndsWithCentral) {
+ builder.crossRegionAccessEnabled(true);
+ LOG.debug("Enabling cross region access for endpoint {}",
endpointStr);
Review Comment:
we don't set the default region here?
--
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]
