Copilot commented on code in PR #13446:
URL: https://github.com/apache/gravitino/pull/13446#discussion_r4073236092
##########
core/src/main/java/org/apache/gravitino/utils/MetadataObjectUtil.java:
##########
@@ -347,6 +348,15 @@ public static void checkMetadataObject(String metalake,
MetadataObject object) {
}
break;
+ case SEMANTIC_MODEL:
+ NameIdentifierUtil.checkSemanticModel(identifier);
+ try {
+ env.semanticModelDispatcher().loadSemanticModel(identifier);
+ } catch (NoSuchSemanticModelException e) {
+ throw exceptionToThrowSupplier.get();
+ }
+ break;
Review Comment:
`checkMetadataObject` appears to use internal dispatchers for existence
checks (likely to avoid auth/side-effects). Using the public
`semanticModelDispatcher()` here can change runtime behavior (e.g.,
authorization failures or different resolution semantics) compared to other
object types. Consider adding and using an `internalSemanticModelDispatcher()`
on `GravitinoEnv` for consistency; if that’s not feasible, add an explicit
comment explaining why the public dispatcher is safe/required here and what
behavior differences are expected.
##########
core/src/test/java/org/apache/gravitino/utils/TestMetadataObjectUtil.java:
##########
@@ -325,6 +328,9 @@ public void
testCheckMetadataObjectUsesInternalDispatchers() {
"metalake", MetadataObjects.of(null, "job",
MetadataObject.Type.JOB));
MetadataObjectUtil.checkMetadataObject(
"metalake", MetadataObjects.of(null, "template",
MetadataObject.Type.JOB_TEMPLATE));
+ MetadataObjectUtil.checkMetadataObject(
+ "metalake",
+ MetadataObjects.of("catalog.schema", "sm",
MetadataObject.Type.SEMANTIC_MODEL));
Review Comment:
This test is in `testCheckMetadataObjectUsesInternalDispatchers`, but the
new assertion covers a code path that uses `env.semanticModelDispatcher()`
(non-internal). To avoid misleading intent, consider renaming/splitting the
test to reflect that it validates dispatcher usage in general (or explicitly
documents that semantic models are an exception because no internal dispatcher
exists).
--
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]
