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]