HonahX commented on code in PR #678:
URL: https://github.com/apache/iceberg-python/pull/678#discussion_r1585910772
##########
pyiceberg/catalog/__init__.py:
##########
@@ -394,6 +394,11 @@ def table_exists(self, identifier: Union[str, Identifier])
-> bool:
Returns:
bool: True if the table exists, False otherwise.
"""
+ try:
Review Comment:
I think it is better for RestCatalog to maintain a separate implementation
to make a head request to
[/v1/{prefix}/namespaces/{namespace}/tables/{table}](https://github.com/apache/iceberg/blob/main/open-api/rest-catalog-open-api.yaml#L814),
ref: https://github.com/apache/iceberg-python/pull/512#discussion_r1519661896,
https://github.com/apache/iceberg-python/issues/507#issue-2177108431
The try-catch implementation is for other non-rest catalogs and thus it is
put in the `MetastoreCatalog` instead of the `Catalog` interface.
A little bit more context regarding the `MetastoreCatalog`: while working on
https://github.com/apache/iceberg-python/pull/498, we found that many helper
functions as well as some implementations are for non-rest catalogs only. So we
decide to move those to another layer, `MetastoreCatalog` to make the
inheritance look better:
https://github.com/apache/iceberg-python/pull/498#discussion_r1537251443.
Do these sound reasonable to you?
--
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]
