singhpk234 commented on a change in pull request #4334:
URL: https://github.com/apache/iceberg/pull/4334#discussion_r829680614
##########
File path:
aws/src/integration/java/org/apache/iceberg/aws/s3/TestS3FileIOIntegration.java
##########
@@ -268,4 +306,11 @@ private void validateRead(S3FileIO s3FileIO) throws
Exception {
stream.close();
Assert.assertEquals(content, result);
}
+
+ private String testAccessPointARN() {
Review comment:
- by configured you mean there is a s3bucketToAccessPoint is not empty
or introducing a flag to enable this feature.
- From the looks of this, it looks like it would be required trying out a
quick test case for the same.
##########
File path:
aws/src/integration/java/org/apache/iceberg/aws/s3/TestS3FileIOIntegration.java
##########
@@ -268,4 +306,11 @@ private void validateRead(S3FileIO s3FileIO) throws
Exception {
stream.close();
Assert.assertEquals(content, result);
}
+
+ private String testAccessPointARN() {
Review comment:
Thanks @jackye1995 , was not aware of this conf
- by configured you mean there is a s3bucketToAccessPoint is not empty or
introducing a flag to enable this feature.
- From the looks of this, it looks like it would be required trying out a
quick test case for the same.
##########
File path:
aws/src/integration/java/org/apache/iceberg/aws/s3/TestS3FileIOIntegration.java
##########
@@ -268,4 +306,11 @@ private void validateRead(S3FileIO s3FileIO) throws
Exception {
stream.close();
Assert.assertEquals(content, result);
}
+
+ private String testAccessPointARN() {
Review comment:
I tried this and it failed with "java.lang.IllegalArgumentException: The
region field of the ARN being passed as a bucket parameter to an S3 operation
does not match the region the client was configured with. To enable this
behavior and prevent this exception set 'useArnRegionEnabled' to true in the
configuration when building the S3 client. Provided region: 'us-east-2'; client
region: 'us-east-1'."
Looks like we need to set this
##########
File path:
aws/src/integration/java/org/apache/iceberg/aws/s3/TestS3FileIOIntegration.java
##########
@@ -268,4 +306,11 @@ private void validateRead(S3FileIO s3FileIO) throws
Exception {
stream.close();
Assert.assertEquals(content, result);
}
+
+ private String testAccessPointARN() {
Review comment:
I tried this and it failed with _`"java.lang.IllegalArgumentException:
The region field of the ARN being passed as a bucket parameter to an S3
operation does not match the region the client was configured with. To enable
this behavior and prevent this exception set 'useArnRegionEnabled' to true in
the configuration when building the S3 client. Provided region: 'us-east-2';
client region: 'us-east-1'."`_
Looks like we need to set this
##########
File path:
aws/src/integration/java/org/apache/iceberg/aws/s3/TestS3FileIOIntegration.java
##########
@@ -268,4 +306,11 @@ private void validateRead(S3FileIO s3FileIO) throws
Exception {
stream.close();
Assert.assertEquals(content, result);
}
+
+ private String testAccessPointARN() {
Review comment:
+1, same-single-region / multi-region doesn't need this (verified this
locally)
should we make this flag explicit to access
points`s3-acess-point.cross-single-region.enabled` or it can be as general as
`s3-cross-single-region.enabled` based on that we can set this in our s3 client
the required conf.
##########
File path:
aws/src/integration/java/org/apache/iceberg/aws/s3/TestS3FileIOIntegration.java
##########
@@ -268,4 +306,11 @@ private void validateRead(S3FileIO s3FileIO) throws
Exception {
stream.close();
Assert.assertEquals(content, result);
}
+
+ private String testAccessPointARN() {
Review comment:
This makes sense, having such a name and linking it to the direct AWS
SDK v2 repo as a reference adds more value. Let me make the changes.
##########
File path: aws/src/integration/java/org/apache/iceberg/aws/AwsIntegTestUtil.java
##########
@@ -81,4 +114,38 @@ public static void cleanGlueCatalog(GlueClient glue,
List<String> namespaces) {
}
}
}
+
+ public static S3ControlClient createCrossRegionS3ControlClient() {
+ return S3ControlClient.builder()
+ .httpClientBuilder(UrlConnectionHttpClient.builder())
+ .region(Region.of(testCrossRegion()))
+ .build();
+ }
Review comment:
[rational] This is required to create cross-region access point as
S3ControlClient has a validation within that it will allow creating AccessPoint
in the region where the S3ControlClient is configured.
Didn't add this in the factory rather added here as didn't wanted to change
public interface to expose setting 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]
