[GitHub] [iceberg] rdblue commented on a diff in pull request #6161: Python: Load the S3 Session token

[GitBox <gi...@apache.org>]

rdblue commented on code in PR #6161:
URL: https://github.com/apache/iceberg/pull/6161#discussion_r1018547445


##########
python/pyiceberg/table/__init__.py:
##########
@@ -90,3 +110,16 @@ def snapshot_by_name(self, name: str) -> Optional[Snapshot]:
     def history(self) -> List[SnapshotLogEntry]:
         """Get the snapshot history of this table."""
         return self.metadata.snapshot_log
+
+    @cached_property
+    def io(self) -> FileIO:
+        return load_file_io({**self.catalog_properties, **self.metadata.properties, **self.config})

Review Comment:
   This is the only part that I think we may need to change. It's a bit strange to me that we're passing catalog configuration into a table. And that catalog configuration may contain things that we don't want to expose. In the other catalog/table implementation, we basically make the catalog responsible for building a table's FileIO and then passing it in when it creates the table. That keeps the catalog properties from being used outside the catalog.
   
   What do you think about something similar in Python? What should be responsible for creating and configuring 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: 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