JingsongLi commented on code in PR #8179:
URL: https://github.com/apache/paimon/pull/8179#discussion_r3484986509
##########
paimon-core/src/main/java/org/apache/paimon/rest/RESTCatalog.java:
##########
@@ -910,6 +914,139 @@ public PagedList<Function> listFunctionDetailsPaged(
}
}
+ @Override
+ public List<String> listResources(String databaseName) throws
DatabaseNotExistException {
+ try {
+ return api.listResources(databaseName);
+ } catch (NoSuchResourceException e) {
+ throw new DatabaseNotExistException(databaseName);
+ } catch (ForbiddenException e) {
+ throw new DatabaseNoPermissionException(databaseName, e);
+ }
+ }
+
+ @Override
+ public Resource getResource(Identifier identifier) throws
ResourceNotExistException {
+ try {
+ GetResourceResponse response = api.getResource(identifier);
+ return toResource(identifier, response);
+ } catch (NoSuchResourceException e) {
+ throw new ResourceNotExistException(identifier, e);
+ } catch (ForbiddenException e) {
+ throw new TableNoPermissionException(identifier, e);
+ }
+ }
+
+ @Override
+ public void createResource(Identifier identifier, Resource resource,
boolean ignoreIfExists)
+ throws ResourceAlreadyExistException, DatabaseNotExistException {
+ RESTFunctionValidator.checkFunctionName(identifier.getObjectName());
+ try {
+ api.createResource(
+ identifier,
+ resource.comment().orElse(null),
+ resource.uri(),
+ resource.resourceType());
+ } catch (NoSuchResourceException e) {
+ throw new DatabaseNotExistException(identifier.getDatabaseName(),
e);
+ } catch (AlreadyExistsException e) {
+ if (ignoreIfExists) {
+ return;
+ }
+ throw new ResourceAlreadyExistException(identifier, e);
+ }
+ }
+
+ @Override
+ public void dropResource(Identifier identifier, boolean ignoreIfNotExists)
+ throws ResourceNotExistException {
+ RESTFunctionValidator.checkFunctionName(identifier.getObjectName());
+ try {
+ api.dropResource(identifier);
+ } catch (NoSuchResourceException e) {
+ if (ignoreIfNotExists) {
+ return;
+ }
+ throw new ResourceNotExistException(identifier, e);
+ }
+ }
+
+ @Override
+ public void alterResource(
+ Identifier identifier, List<ResourceChange> changes, boolean
ignoreIfNotExists)
+ throws ResourceNotExistException {
+ try {
+ api.alterResource(identifier, changes);
+ } catch (NoSuchResourceException e) {
+ if (!ignoreIfNotExists) {
+ throw new ResourceNotExistException(identifier, e);
+ }
+ } catch (ForbiddenException e) {
+ throw new TableNoPermissionException(identifier, e);
+ } catch (BadRequestException e) {
+ throw new IllegalArgumentException(e.getMessage());
+ }
+ }
+
+ @Override
+ public PagedList<String> listResourcesPaged(
+ String databaseName,
+ @Nullable Integer maxResults,
+ @Nullable String pageToken,
+ @Nullable String resourceNamePattern)
+ throws DatabaseNotExistException {
+ try {
+ return api.listResourcesPaged(databaseName, maxResults, pageToken,
resourceNamePattern);
+ } catch (NoSuchResourceException e) {
Review Comment:
These paged resource APIs should translate 403 the same way as listResources
and the view paging APIs do. The REST server checks database permissions before
reaching the resource handler, so a forbidden database will make
api.listResourcesPaged/listResourceDetailsPaged throw ForbiddenException; with
the current catches that REST-layer exception leaks to Catalog callers instead
of DatabaseNoPermissionException. Please catch ForbiddenException here and in
listResourceDetailsPaged and wrap it as
DatabaseNoPermissionException(databaseName, e).
##########
paimon-python/pypaimon/catalog/rest/rest_catalog.py:
##########
@@ -493,6 +494,129 @@ def list_function_details_paged(
except NoSuchResourceException as e:
raise DatabaseNotExistException(database_name) from e
+ # Resource CRUD: mirrors Java RESTCatalog resource handlers.
+ def list_resources(self, database_name: str) -> List[str]:
+ try:
+ return self.rest_api.list_resources(database_name)
+ except NoSuchResourceException as e:
+ raise DatabaseNotExistException(database_name) from e
+ except ForbiddenException as e:
+ raise DatabaseNoPermissionException(database_name) from e
+
+ def get_resource(self, identifier: Union[str, Identifier]) -> 'Resource':
+ if not isinstance(identifier, Identifier):
+ identifier = Identifier.from_string(identifier)
+ try:
+ response = self.rest_api.get_resource(identifier)
+ return self._to_resource(identifier, response)
+ except NoSuchResourceException as e:
+ raise ResourceNotExistException(identifier) from e
+ except ForbiddenException as e:
+ raise TableNoPermissionException(identifier) from e
+
+ def create_resource(self, identifier: Union[str, Identifier],
+ resource: 'Resource', ignore_if_exists: bool = False)
-> None:
+ if not isinstance(identifier, Identifier):
+ identifier = Identifier.from_string(identifier)
+ RESTApi.check_function_name(identifier.get_object_name())
+ try:
+ self.rest_api.create_resource(
+ identifier,
+ resource.comment(),
+ resource.uri(),
+ resource.resource_type(),
+ )
+ except NoSuchResourceException as e:
+ raise DatabaseNotExistException(identifier.get_database_name())
from e
+ except AlreadyExistsException as e:
+ if ignore_if_exists:
+ return
+ raise ResourceAlreadyExistException(identifier) from e
+
+ def drop_resource(self, identifier: Union[str, Identifier],
+ ignore_if_not_exists: bool = False) -> None:
+ if not isinstance(identifier, Identifier):
+ identifier = Identifier.from_string(identifier)
+ RESTApi.check_function_name(identifier.get_object_name())
+ try:
+ self.rest_api.drop_resource(identifier)
+ except NoSuchResourceException as e:
+ if ignore_if_not_exists:
+ return
+ raise ResourceNotExistException(identifier) from e
+
+ def alter_resource(self, identifier: Union[str, Identifier],
+ changes: List['ResourceChange'],
+ ignore_if_not_exists: bool = False) -> None:
+ if not isinstance(identifier, Identifier):
+ identifier = Identifier.from_string(identifier)
+ try:
+ self.rest_api.alter_resource(identifier, changes)
+ except NoSuchResourceException as e:
+ if not ignore_if_not_exists:
+ raise ResourceNotExistException(identifier) from e
+ except ForbiddenException as e:
+ raise TableNoPermissionException(identifier) from e
+ except BadRequestException as e:
+ raise IllegalArgumentError(str(e)) from e
+
+ def list_resources_paged(
+ self,
+ database_name: str,
+ max_results: Optional[int] = None,
+ page_token: Optional[str] = None,
+ resource_name_pattern: Optional[str] = None,
+ ) -> PagedList[str]:
+ try:
+ return self.rest_api.list_resources_paged(
+ database_name, max_results, page_token, resource_name_pattern)
+ except NoSuchResourceException as e:
Review Comment:
This has the same permission-mapping gap as the Java side. A forbidden
database makes rest_api.list_resources_paged and list_resource_details_paged
raise ForbiddenException, but the Catalog wrapper only converts
NoSuchResourceException, so callers see the REST exception instead of
DatabaseNoPermissionException. I reproduced it with the Python mock server:
list_resources raises DatabaseNoPermissionException, while these paged resource
APIs raise ForbiddenException. Please catch ForbiddenException here and in
list_resource_details_paged and wrap it as
DatabaseNoPermissionException(database_name).
--
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]
