danielcweeks commented on code in PR #5178:
URL: https://github.com/apache/iceberg/pull/5178#discussion_r912274568


##########
aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIO.java:
##########
@@ -79,6 +79,7 @@ public class S3FileIO implements FileIO, 
SupportsBulkOperations, SupportsPrefixO
   private String credential = null;
   private SerializableSupplier<S3Client> s3;
   private AwsProperties awsProperties;
+  private Map<String, String> properties = null;

Review Comment:
   Ok, I think I understand why you're doing this now, which is basically that 
we need to re-initialize an S3/GCS FileIO (as opposed to the java/kryo 
serialization, which captures the aws/gcp properties for deserialization).  
However, this pulls in far more than we would want to include.
   
   We could have the `properties()` return the aws properties configuration:
   
   ```java
     @Override
     public Map<String, String> properties() {
       return awsProperties.toMap();
     }
   ```
   
   This is a little painful for S3FileIO due to the number of properties, but I 
feel like we don't want to send all of the config properties (including 
sensitive properties) everywhere with the FileIO.



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