sungwy commented on code in PR #1145:
URL: https://github.com/apache/iceberg-python/pull/1145#discussion_r1805681185


##########
pyiceberg/catalog/dynamodb.py:
##########
@@ -393,7 +393,7 @@ def drop_namespace(self, namespace: Union[str, Identifier]) 
-> None:
             raise NoSuchNamespaceError(f"Database does not exist: 
{database_name}") from e
 
     def list_tables(self, namespace: Union[str, Identifier]) -> 
List[Identifier]:
-        """List tables under the given namespace in the catalog (including 
non-Iceberg tables).
+        """List Iceberg tables under the given namespace in the catalog.

Review Comment:
   I noticed that we only updated the docstring here, without a change in the 
code.
   Does Dynamodb already only return Iceberg tables?



##########
pyiceberg/catalog/glue.py:
##########
@@ -779,3 +779,7 @@ def list_views(self, namespace: Union[str, Identifier]) -> 
List[Identifier]:
 
     def drop_view(self, identifier: Union[str, Identifier]) -> None:
         raise NotImplementedError
+
+    @staticmethod
+    def __is_iceberg_table(table: TableTypeDef) -> bool:
+        return table["Parameters"] is not None and 
table["Parameters"][TABLE_TYPE].lower() == ICEBERG.lower()

Review Comment:
   nit: `pyiceberg.catalog.ICEBERG` is already lower cased
   
   ```suggestion
           return table["Parameters"] is not None and 
table["Parameters"][TABLE_TYPE].lower() == ICEBERG
   ```



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