LuciferYang commented on code in PR #13524:
URL: https://github.com/apache/gravitino/pull/13524#discussion_r4115746444
##########
core/src/main/java/org/apache/gravitino/authorization/UserGroupManager.java:
##########
@@ -65,6 +65,7 @@ protected UserGroupManager(EntityStore store, IdGenerator
idGenerator) {
}
User addUser(String metalake, String name) throws UserAlreadyExistsException
{
+ MetalakeManager.checkMetalake(NameIdentifier.of(metalake), store);
Review Comment:
Done in a5d7383a: added `testAddRemoveUserGroupRejectsDisabledMetalake`,
which disables a fixture metalake and asserts all four operations throw
`MetalakeNotInUseException`, so both outcomes of the new validation are covered.
On the bulk-operation note: `checkMetalake` reads the metalake through the
entity store, and metalake entities are cacheable, so the per-item checks in a
bulk add/remove hit the cache after the first and cost a light lookup on a cold
admin path. I kept the check per operation for a clear fail-fast per item
rather than hoisting it, but I can hoist it into the bulk methods if you prefer.
--
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]
