dennishuo commented on code in PR #2223:
URL: https://github.com/apache/polaris/pull/2223#discussion_r2373743877
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java:
##########
@@ -1686,14 +1691,33 @@ public boolean grantPrivilegeOnNamespaceToRole(
PolarisAuthorizableOperation.ADD_NAMESPACE_GRANT_TO_CATALOG_ROLE;
authorizeGrantOnNamespaceOperationOrThrow(op, catalogName, namespace,
catalogRoleName);
+ CatalogEntity catalogEntity =
+ findCatalogByName(catalogName)
+ .orElseThrow(() -> new NotFoundException("Parent catalog %s not
found", catalogName));
PolarisEntity catalogRoleEntity =
findCatalogRoleByName(catalogName, catalogRoleName)
.orElseThrow(() -> new NotFoundException("CatalogRole %s not
found", catalogRoleName));
PolarisResolvedPathWrapper resolvedPathWrapper =
resolutionManifest.getResolvedPath(namespace);
if (resolvedPathWrapper == null
|| !resolvedPathWrapper.isFullyResolvedNamespace(catalogName,
namespace)) {
- throw new NotFoundException("Namespace %s not found", namespace);
+ boolean rbacForFederatedCatalogsEnabled =
+ getCurrentPolarisContext()
+ .getRealmConfig()
+
.getConfig(FeatureConfiguration.ENABLE_SUB_CATALOG_RBAC_FOR_FEDERATED_CATALOGS);
+ if (resolutionManifest.getIsPassthroughFacade() &&
rbacForFederatedCatalogsEnabled) {
+ resolvedPathWrapper =
+ createSyntheticNamespaceEntities(catalogEntity, namespace,
resolvedPathWrapper);
+ if (resolvedPathWrapper == null
+ || !resolvedPathWrapper.isFullyResolvedNamespace(catalogName,
namespace)) {
+ throw new RuntimeException(
+ String.format(
+ "Failed to create synthetic namespace entities for namespace
%s in catalog %s",
Review Comment:
Ah you're right, maybe not UnprocessableEntityException, I misremembered the
meaning.
409 is also potentially accurate, just that it happens to be a server-side
conflict where the caller doesn't necessarily need to do anything different in
the request to retry it successfully. So we could return
`CommitFailedException`.
But it's also true that as long as we don't yet *drop* facade entities, it's
an unexpected illegal state. Then the "raw" RuntimeException correctly gets
mapped to `INTERNAL_SERVER_ERROR`.
Maybe for now we could just add a TODO here to possibly update the exception
thrown as we refine the possible retry scenarios?
--
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]
