singhpk234 commented on a change in pull request #4259:
URL: https://github.com/apache/iceberg/pull/4259#discussion_r820844515
##########
File path: aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java
##########
@@ -162,7 +173,28 @@ private String cleanWarehousePath(String path) {
@Override
protected TableOperations newTableOps(TableIdentifier tableIdentifier) {
- return new GlueTableOperations(glue, lockManager, catalogName,
awsProperties, fileIO, tableIdentifier);
+ if (isS3FileIOConfigured()) {
+ // Create different S3FileIO to use different credentials in future
Review comment:
[minor] can we elaborate more as to why we need to create new S3FileO
obj . (As you stated this would help in picking up most up to date creds)
##########
File path: aws/src/test/java/org/apache/iceberg/aws/s3/TestS3OutputStream.java
##########
@@ -85,6 +87,8 @@
private final Random random = new Random(1);
private final Path tmpDir = Files.createTempDirectory("s3fileio-test-");
private final String newTmpDirectory = "/tmp/newStagingDirectory";
+ private final List<Tag> tags = ImmutableList.of(
+ Tag.builder().key("abc").value("123").build());
Review comment:
[question] what would be the behaviour when
```
private final List<Tag> tags = ImmutableList.of(
Tag.builder().key("abc").value("123").build(),
Tag.builder().key("abc").value("xyz").build());
```
- does for tag abc we will have xyz as value or 123 or the request will be
rejected ?
- Do we require to maintain uniqueness at Tag Key level if yes then
List<Tag> might not be a good DS.
##########
File path: aws/src/main/java/org/apache/iceberg/aws/glue/GlueCatalog.java
##########
@@ -162,7 +173,28 @@ private String cleanWarehousePath(String path) {
@Override
protected TableOperations newTableOps(TableIdentifier tableIdentifier) {
- return new GlueTableOperations(glue, lockManager, catalogName,
awsProperties, fileIO, tableIdentifier);
+ if (isS3FileIOConfigured()) {
+ // Create different S3FileIO to use different credentials in future
+ FileIO s3FileIO = new S3FileIO(toTags(tableIdentifier));
+ s3FileIO.initialize(this.catalogProperties);
+ closeableGroup.addCloseable(s3FileIO);
+ return new GlueTableOperations(glue, lockManager, catalogName,
awsProperties, s3FileIO, tableIdentifier);
+ } else {
+ return new GlueTableOperations(glue, lockManager, catalogName,
awsProperties, fileIO, tableIdentifier);
+ }
+ }
+
+ private boolean isS3FileIOConfigured() {
+ String fileIOImpl =
this.catalogProperties.get(CatalogProperties.FILE_IO_IMPL);
+ return fileIOImpl == null || fileIOImpl.equals(S3_FILE_IO_IMPL);
Review comment:
[question] can we unify this logic with FileIO initialization ? my
rationale is what if we decide to update the behaviour when fileIOImpl is null
and we don't use S3FileIOImp instead.
--
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]
