jerryshao commented on code in PR #12465:
URL: https://github.com/apache/gravitino/pull/12465#discussion_r3841123585
##########
clients/client-python/gravitino/client/base_schema_catalog.py:
##########
@@ -19,6 +19,7 @@
from typing import Dict, List, Optional
from gravitino.api.catalog import Catalog
+from gravitino.api.authorization.supports_roles import SupportsRoles
Review Comment:
**[Minor]** This new import is inserted right after `from
gravitino.api.catalog import Catalog`, breaking alphabetical import order
(`authorization.supports_roles` sorts before `catalog`).
Every other file touched by this same PR (`generic_fileset.py`,
`generic_model.py`, `generic_schema.py`, `gravitino_metalake.py`,
`relational_table.py`) places the new `authorization.supports_roles` import
alphabetically correctly. Worth moving this one up to match, for consistency.
##########
clients/client-python/gravitino/api/catalog.py:
##########
@@ -224,6 +225,17 @@ def supports_tags(self) -> SupportsTags:
"""
raise UnsupportedOperationException("Catalog does not support tag
operations")
+ def supports_roles(self) -> SupportsRoles:
+ """Return role operations supported by this catalog.
+
+ Returns:
+ SupportsRoles: The role operations supported by this catalog.
+
+ Raises:
+ UnsupportedOperationException: If this catalog does not support
role operations.
+ """
+ raise UnsupportedOperationException("Catalog does not support role
operations")
Review Comment:
**[Confirmed bug]** This raises the locally-defined
`UnsupportedOperationException(Exception)` from the bottom of this file, not
`gravitino.exceptions.base.UnsupportedOperationException` used by every sibling
default (`Metalake`, `Schema`, `Table`, `Fileset`, `Model`).
Code written to catch the standard
`gravitino.exceptions.base.UnsupportedOperationException` around a generic
`metadata_object.supports_roles()` call (to detect "role ops unsupported"
uniformly across metalakes/catalogs/schemas/tables/filesets) will not catch it
for a `Catalog` whose `supports_roles()` falls through to this default, since
the two `UnsupportedOperationException` classes are unrelated hierarchies
(`Exception` vs `GravitinoRuntimeException`). This PR's own unit test
(`tests/unittests/test_supports_roles.py`) has to alias-import
`UnsupportedOperationException as CatalogUnsupportedOperationException`
specifically to work around this, confirming the divergence is real. Currently
latent since all concrete catalogs go through `BaseSchemaCatalog`'s override,
but any future/third-party `Catalog` subclass that doesn't extend
`BaseSchemaCatalog` will hit it.
--
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]
