Re: [PR] Ensure that RestCatalog passes user config to FileIO [iceberg-rust]

["liurenjie1024 (via GitHub)" <gi...@apache.org>]

liurenjie1024 commented on code in PR #476:
URL: https://github.com/apache/iceberg-rust/pull/476#discussion_r1699321914


##########
crates/catalog/rest/src/catalog.rs:
##########
@@ -504,8 +504,15 @@ impl Catalog for RestCatalog {
             .query::<LoadTableResponse, ErrorResponse, OK>(request)
             .await?;
 
+        let config = resp
+            .config
+            .unwrap_or_default()
+            .into_iter()
+            .chain(self.user_config.props.clone().into_iter())

Review Comment:
   I think both two methods have pros and cons.
   
   If we prioritize user config, we get flexibility max flexibility, but also would be easy to introduce bugs. For example, what if user misconfiged some fileio property which should be managed by rest catalog?
   
   Also, [the rest catalog spec](https://github.com/apache/iceberg/blob/6319712b612b724fedbc5bed41942ac3426ffe48/open-api/rest-catalog-open-api.yaml#L625) 's statement make me feel that we should repsect rest catalog's response.



-- 
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-unsubscribe@iceberg.apache.org

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


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