rdblue commented on a change in pull request #1216:
URL: https://github.com/apache/iceberg/pull/1216#discussion_r476619221



##########
File path: python/iceberg/core/filesystem/filesystem_tables.py
##########
@@ -37,7 +37,19 @@ def load(self: "FilesystemTables", table_identifier: str) -> 
Table:
 
     def create(self: "FilesystemTables", schema: Schema, table_identifier: 
str, spec: PartitionSpec = None,
                properties: dict = None, location: str = None) -> Table:
+        """
+        Create a new table on the filesystem.
+
+        Note: it is expected that the filesystem has atomic operations to 
ensure consistency for metadata updates.
+        Filesystems that don't have this guarantee could lead to data loss.
+
+        Only table_identifier or location should be not None. Will throw an 
error if both are not None.

Review comment:
       This doesn't agree with what is enforced. I like that `location` must 
always be `None` in the code so it would be good to state that here. It would 
also be easier to read since it avoids "should be not None".




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

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