Kurtiscwright commented on code in PR #2623:
URL: https://github.com/apache/iceberg-rust/pull/2623#discussion_r3400589453


##########
crates/catalog/s3tables/src/utils.rs:
##########
@@ -39,10 +39,6 @@ pub(crate) async fn create_sdk_config(
 ) -> SdkConfig {
     let mut config = aws_config::defaults(BehaviorVersion::latest());
 
-    if properties.is_empty() {
-        return config.load().await;
-    }
-

Review Comment:
   I considered that approach, but I chose the current implementation for two 
reasons:
   1. It read like a premature optimization to me to have this check. This 
validation is only ran at Catalog load time so once per session and the number 
of checks are only 4 at the moment.
   2. Keeping this check leaves this same class of bug for any future property 
flag implementations.
   
   For both of the above reasons. I felt it was better to remove the check 
altogether rather than shuffle it further down. I can add it back in if there 
is strong disagreement with my two reasons listed above.



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