SanjayMarreddi commented on code in PR #12299:
URL: https://github.com/apache/iceberg/pull/12299#discussion_r1968300773


##########
aws/src/main/java/org/apache/iceberg/aws/s3/S3FileIOProperties.java:
##########
@@ -640,12 +663,21 @@ public S3FileIOProperties(Map<String, String> properties) 
{
             properties,
             S3_DIRECTORY_BUCKET_LIST_PREFIX_AS_DIRECTORY,
             S3_DIRECTORY_BUCKET_LIST_PREFIX_AS_DIRECTORY_DEFAULT);
+    this.isS3AnalyticsAcceleratorEnabled =
+        PropertyUtil.propertyAsBoolean(
+            properties, S3_ANALYTICS_ACCELERATOR_ENABLED, 
S3_ANALYTICS_ACCELERATOR_ENABLED_DEFAULT);
+    this.s3AnalyticsacceleratorProperties =
+        PropertyUtil.propertiesWithPrefix(properties, 
S3_ANALYTICS_ACCELERATOR_PREFIX);
 
     ValidationException.check(
         keyIdAccessKeyBothConfigured(),
         "S3 client access key ID and secret access key must be set at the same 
time");
   }
 
+  public Map<String, String> toMap() {
+    return Collections.unmodifiableMap(allProperties);

Review Comment:
   This is being used in `S3SeekableInputStreamFactorySupplier` for creating 
correct `s3AsyncClient` in both cases of `S3FileIO` creation (Constructor with 
`s3FileIOProperties` vs Constructor + `initialise` )
   
   I can see that the state of `allProperties` map is not up-to date when 
setters are called. We could fix this issue by:
   
   1. Making sure that the setters update this `allProperties` map.
   2. Avoid having this `toMap` overall  by making sure 
`S3FileIOAwsClientFactories` support `s3FileIOProperties` directly instead of 
`Map<String, String> properties`
   
   Would like to hear from the community on which approach we should follow ( 
including any new suggestions )
   
   



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to