Copilot commented on code in PR #10971:
URL: https://github.com/apache/gravitino/pull/10971#discussion_r3213159177
##########
server/build.gradle.kts:
##########
@@ -63,6 +63,7 @@ dependencies {
testImplementation(libs.mockito.inline)
testImplementation(libs.ognl)
+ testRuntimeOnly(project(":plugins:idp-basic"))
testRuntimeOnly(libs.junit.jupiter.engine)
Review Comment:
The server now relies on `ServiceLoader` to find an `IdpManager` provider
during `GravitinoEnv` initialization, but `plugins:idp-basic` is only added as
`testRuntimeOnly`. This means `:server` can pass tests yet still fail to start
in non-test runtime classpaths (e.g., running from IDE/`gradlew :server:run`)
because no provider is visible. If the IdP plugin is required at runtime, add
it as a runtime dependency (or adjust `IdpManagerFactory`/bootstrapping to load
it from the distribution plugin directory).
##########
core/src/main/java/org/apache/gravitino/GravitinoEnv.java:
##########
@@ -685,5 +701,7 @@ private void initGravitinoServerComponents() {
BuiltInJobTemplateEventListener builtInJobTemplateListener =
new BuiltInJobTemplateEventListener(jobManager, entityStore,
idGenerator);
eventListenerManager.addEventListener("builtin-job-template",
builtInJobTemplateListener);
+
+ this.idpManager = IdpManagerFactory.createIdpManager();
}
Review Comment:
`initGravitinoServerComponents()` unconditionally initializes `idpManager`
via `IdpManagerFactory.createIdpManager()`, which throws if no ServiceLoader
provider is present. This introduces a hard startup dependency on the IdP
plugin being on the runtime classpath and can break server initialization (and
potentially leak partially-initialized resources) in deployments/dev runs that
don't include `plugins:idp-basic`. Consider making IdP manager loading optional
(e.g., a `createOrDefault()`/no-op implementation or conditional init based on
config) and returning a consistent 501/405 from `/idp/*` when unavailable
instead of failing startup.
--
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]
