vigneshio commented on code in PR #4793:
URL: https://github.com/apache/polaris/pull/4793#discussion_r3550923436


##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisAdminService.java:
##########
@@ -803,18 +803,10 @@ public void deleteCatalog(String name) {
         metaStoreManager.dropEntityIfExists(
             getCurrentPolarisContext(), null, entity, Map.of(), cleanup);
 
-    // at least some handling of error
-    if (!dropEntityResult.isSuccess()) {
-      if (dropEntityResult.failedBecauseNotEmpty()) {
-        throw new BadRequestException(
-            "Catalog '%s' cannot be dropped, it is not empty", 
entity.getName());
-      } else {
-        throw new BadRequestException(
-            "Catalog '%s' cannot be dropped, concurrent modification detected. 
Please try "
-                + "again",
-            entity.getName());
-      }
-    }
+    DropEntityExceptionMapper.throwIfFailed(
+        dropEntityResult,
+        new DropEntityExceptionMapper.DropFailureContext(
+            String.format("Catalog '%s'", entity.getName()), null));

Review Comment:
   Done - changed `entityLabe`l to `Supplier<String>` so the formatting is 
deferred until there's an actual failure. Added a quick test to make sure we 
skip evaluation on success paths. Thanks for the catch! 💯 



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

Reply via email to