amogh-jahagirdar commented on code in PR #4909:
URL: https://github.com/apache/iceberg/pull/4909#discussion_r886243443
##########
aws/src/integration/java/org/apache/iceberg/aws/TestAssumeRoleAwsClientFactory.java:
##########
@@ -117,7 +65,7 @@ public void testAssumeRoleGlueCatalog() throws Exception {
Assert.assertEquals(AccessDeniedException.class, e.getClass());
}
- Namespace namespace = Namespace.of("allowed_" +
UUID.randomUUID().toString().replace("-", ""));
+ Namespace namespace = Namespace.of("iceberg_aws_ci_" +
UUID.randomUUID().toString().replace("-", ""));
Review Comment:
Nit: iceberg_aws_ci_ maybe could be a static prefix? Not blocking though
considering it was already like that
##########
aws/src/integration/java/org/apache/iceberg/aws/TestAssumeRoleAwsClientFactory.java:
##########
@@ -129,41 +77,20 @@ public void testAssumeRoleGlueCatalog() throws Exception {
}
@Test
- public void testAssumeRoleS3FileIO() throws Exception {
- String bucketArn = "arn:aws:s3:::" + AwsIntegTestUtil.testBucketName();
- iam.putRolePolicy(PutRolePolicyRequest.builder()
- .roleName(roleName)
- .policyName(policyName)
- .policyDocument("{" +
- "\"Version\":\"2012-10-17\"," +
- "\"Statement\":[{" +
- "\"Sid\":\"policy1\"," +
- "\"Effect\":\"Allow\"," +
- "\"Action\":\"s3:ListBucket\"," +
- "\"Resource\":[\"" + bucketArn + "\"]," +
- "\"Condition\":{\"StringLike\":{\"s3:prefix\":[\"allowed/*\"]}}}
,{" +
- "\"Sid\":\"policy2\"," +
- "\"Effect\":\"Allow\"," +
- "\"Action\":\"s3:GetObject\"," +
- "\"Resource\":[\"" + bucketArn + "/allowed/*\"]}]}")
- .build());
- waitForIamConsistency();
-
- S3FileIO s3FileIO = new S3FileIO();
- s3FileIO.initialize(assumeRoleProperties);
- InputFile inputFile = s3FileIO.newInputFile("s3://" +
AwsIntegTestUtil.testBucketName() + "/denied/file");
+ public void testAssumeRoleS3FileIO() {
+ S3Client s3Client = AwsClientFactories.from(assumeRoleProperties).s3();
try {
- inputFile.exists();
+
s3Client.getBucketAccelerateConfiguration(GetBucketAccelerateConfigurationRequest.builder()
Review Comment:
I guess here we can use any method, so it does not matter but would
getBucket just be less verbose to read?
--
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]
