singhpk234 commented on code in PR #4855:
URL: https://github.com/apache/iceberg/pull/4855#discussion_r885211402
##########
aws/src/integration/java/org/apache/iceberg/aws/dynamodb/TestDynamoDbCatalog.java:
##########
@@ -296,6 +296,6 @@ public void testDropNamespace() {
}
private static String genRandomName() {
- return UUID.randomUUID().toString().replace("-", "");
+ return "IcebergAwsCI_" + UUID.randomUUID().toString().replace("-", "");
Review Comment:
Nit : for consistency across
```suggestion
return "iceberg_aws_ci_" + UUID.randomUUID().toString().replace("-", "");
```
##########
aws/src/integration/java/org/apache/iceberg/aws/s3/TestS3FileIOIntegration.java:
##########
@@ -80,26 +82,32 @@ public class TestS3FileIOIntegration {
private static byte[] contentBytes;
private static String content;
private static String kmsKeyArn;
+ private static String kmsKeyAlias;
private static int deletionBatchSize;
private String objectKey;
private String objectUri;
@BeforeClass
public static void beforeClass() {
- clientFactory = AwsClientFactories.defaultFactory();
+ clientFactory = AwsClientFactories.from(
+ ImmutableMap.of(AwsProperties.HTTP_CLIENT_TYPE,
AwsProperties.HTTP_CLIENT_TYPE_APACHE));
s3 = clientFactory.s3();
kms = clientFactory.kms();
s3Control =
AwsIntegTestUtil.createS3ControlClient(AwsIntegTestUtil.testRegion());
crossRegionS3Control =
AwsIntegTestUtil.createS3ControlClient(AwsIntegTestUtil.testCrossRegion());
bucketName = AwsIntegTestUtil.testBucketName();
crossRegionBucketName = AwsIntegTestUtil.testCrossRegionBucketName();
- accessPointName = UUID.randomUUID().toString();
- crossRegionAccessPointName = UUID.randomUUID().toString();
+ accessPointName = "icebergawsci-" + UUID.randomUUID();
+ crossRegionAccessPointName = "icebergawsci-" + UUID.randomUUID();
prefix = UUID.randomUUID().toString();
contentBytes = new byte[1024 * 1024 * 10];
deletionBatchSize = 3;
content = new String(contentBytes, StandardCharsets.UTF_8);
kmsKeyArn = kms.createKey().keyMetadata().arn();
+ kms.createAlias(CreateAliasRequest.builder()
+ .targetKeyId(kmsKeyArn)
+ .aliasName("alias/iceberg-aws-ci-" + UUID.randomUUID())
+ .build());
Review Comment:
is it required ? I see kmsKeyAlias declared above as well but it neither
assigned this value nor used
##########
aws/src/integration/java/org/apache/iceberg/aws/s3/TestS3FileIOIntegration.java:
##########
@@ -80,26 +82,32 @@ public class TestS3FileIOIntegration {
private static byte[] contentBytes;
private static String content;
private static String kmsKeyArn;
+ private static String kmsKeyAlias;
private static int deletionBatchSize;
private String objectKey;
private String objectUri;
@BeforeClass
public static void beforeClass() {
- clientFactory = AwsClientFactories.defaultFactory();
+ clientFactory = AwsClientFactories.from(
+ ImmutableMap.of(AwsProperties.HTTP_CLIENT_TYPE,
AwsProperties.HTTP_CLIENT_TYPE_APACHE));
s3 = clientFactory.s3();
kms = clientFactory.kms();
s3Control =
AwsIntegTestUtil.createS3ControlClient(AwsIntegTestUtil.testRegion());
crossRegionS3Control =
AwsIntegTestUtil.createS3ControlClient(AwsIntegTestUtil.testCrossRegion());
bucketName = AwsIntegTestUtil.testBucketName();
crossRegionBucketName = AwsIntegTestUtil.testCrossRegionBucketName();
- accessPointName = UUID.randomUUID().toString();
- crossRegionAccessPointName = UUID.randomUUID().toString();
+ accessPointName = "icebergawsci-" + UUID.randomUUID();
+ crossRegionAccessPointName = "icebergawsci-" + UUID.randomUUID();
Review Comment:
Nit : better to have it in a const, and used across
```suggestion
accessPointName = "iceberg-aws-ci-" + UUID.randomUUID();
crossRegionAccessPointName = "iceberg-aws-ci-" + UUID.randomUUID();
```
##########
aws/src/integration/java/org/apache/iceberg/aws/s3/TestS3FileIOIntegration.java:
##########
@@ -171,6 +179,7 @@ public void testNewOutputStream() throws Exception {
@Test
public void testNewOutputStreamWithAccessPoint() throws Exception {
+ waitForIamConsistency();
Review Comment:
[question] why do we require to sleep 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]
