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


##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -361,6 +366,13 @@ private static URI getS3Endpoint(String endpoint, final 
Configuration conf) {
    */
   private static Region getS3RegionFromEndpoint(String endpoint) {
 
+    // S3 VPC endpoint parsing
+    Matcher matcher = VPC_ENDPOINT_PATTERN.matcher(endpoint);
+    if(matcher.find()) {
+      LOG.debug("Endpoint {} is vpc endpoint; parsing", endpoint);
+      return Region.of(matcher.group(1));

Review Comment:
   add a debug log saying "mapping to vpce"



##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -361,6 +366,13 @@ private static URI getS3Endpoint(String endpoint, final 
Configuration conf) {
    */
   private static Region getS3RegionFromEndpoint(String endpoint) {
 
+    // S3 VPC endpoint parsing
+    Matcher matcher = VPC_ENDPOINT_PATTERN.matcher(endpoint);
+    if(matcher.find()) {

Review Comment:
   nit, add a space after `if`



##########
hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/DefaultS3ClientFactory.java:
##########
@@ -361,6 +366,13 @@ private static URI getS3Endpoint(String endpoint, final 
Configuration conf) {
    */
   private static Region getS3RegionFromEndpoint(String endpoint) {
 
+    // S3 VPC endpoint parsing
+    Matcher matcher = VPC_ENDPOINT_PATTERN.matcher(endpoint);
+    if(matcher.find()) {
+      LOG.debug("Endpoint {} is vpc endpoint; parsing", endpoint);

Review Comment:
   so this is going to match on anything with .vpce. isn't it? I think it 
should include amazonaws.{com,com.cn} at the end so if someone ever sets up an 
internal host called vpce there's no confusion.



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