[#10705] improvement(build): add switch to enable web-ui in configuration and gradlew build parameter#10706
[#10705] improvement(build): add switch to enable web-ui in configuration and gradlew build parameter#10706
Conversation
Add a skipWebWar Gradle property to bypass web/web-v2 war build and packaging for non-frontend scenarios. Introduce gravitino.server.webui.enable to allow server startup without web war in deploy mode, and wire CI workflows to use -PskipWebWar=true except frontend jobs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR adds a Gradle property to optionally skip building/packaging the Web UI WARs and introduces a server configuration flag to allow starting Gravitino without the Web UI when those WARs are omitted (useful for faster CI/integration-test scenarios).
Changes:
- Add
-PskipWebWar=truesupport to avoid building/copyingwebandweb-v2WARs during distribution packaging and some integration-test setups. - Introduce
gravitino.server.webui.enable(defaulttrue) and wire it into server startup so Jetty can run without WARs when UI is disabled. - Update multiple non-frontend GitHub workflows to pass
-PskipWebWar=true.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
build.gradle.kts |
Makes compileDistribution optionally skip Web WAR build/copy; attempts to flip the packaged config when skipping. |
core/src/main/java/org/apache/gravitino/Configs.java |
Adds Configs.SERVER_UI_ENABLED config entry. |
conf/gravitino.conf.template |
Documents and defaults gravitino.server.webui.enable=true. |
server/src/main/java/org/apache/gravitino/server/GravitinoServer.java |
Passes UI-enable flag into Jetty initialization; removes Web UI filter registration. |
server/src/test/java/org/apache/gravitino/server/TestGravitinoServer.java |
Adds a startup test intended to cover “UI disabled” mode. |
web/integration-test/build.gradle.kts |
Makes web build dependency optional under skipWebWar. |
web-v2/integration-test/build.gradle.kts |
Makes web-v2 build dependency optional under skipWebWar. |
.github/workflows/*.yml |
Propagates -PskipWebWar=true to non-frontend workflows to reduce CI cost. |
| @Test | ||
| public void testStartAndStopWithWebUiDisabled() throws Exception { | ||
| ServerConfig config = new ServerConfig(); | ||
| config.loadFromMap( | ||
| ImmutableMap.of( | ||
| GravitinoServer.WEBSERVER_CONF_PREFIX + JettyServerConfig.WEBSERVER_HTTP_PORT.getKey(), | ||
| String.valueOf(RESTUtils.findAvailablePort(5000, 6000)), | ||
| Configs.SERVER_UI_ENABLED.getKey(), | ||
| "false", | ||
| AuxiliaryServiceManager.GRAVITINO_AUX_SERVICE_PREFIX | ||
| + AuxiliaryServiceManager.AUX_SERVICE_NAMES, | ||
| ""), | ||
| t -> true); | ||
| GravitinoServer localServer = new GravitinoServer(config, GravitinoEnv.getInstance()); | ||
| localServer.initialize(); | ||
| localServer.start(); | ||
| localServer.stop(); | ||
| } |
There was a problem hiding this comment.
This new test likely doesn’t validate the intended behavior because unit tests run with GRAVITINO_TEST set (see server/build.gradle.kts), and JettyServer already tolerates missing WARs in that mode even when UI is enabled. As a result, the test would still pass if Configs.SERVER_UI_ENABLED were ignored. Consider strengthening it by asserting that UI is actually disabled (e.g., verify JettyServer.initialize is called with shouldEnableUI=false, or assert the server uses the basic ServletContextHandler instead of WebAppContext, or run a forked test without GRAVITINO_TEST).
Re-add WebUIFilter registrations guarded by gravitino.server.webui.enable. This keeps historical UI redirect behavior for enabled UI while preserving no-web startup when disabled. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Code Coverage Report
Files
|
|
|
||
| // Frontend tests depend on the web page, so we need to build the web module first. | ||
| dependsOn(":web-v2:web:build") | ||
| if (!skipWebWar) { |
There was a problem hiding this comment.
Why not skip the entire web module? The comment above says the frontend tests depend on the web build, but you only removed the build step.
There was a problem hiding this comment.
Why not introduce a skipWeb option to skip everything related to the web module?
| dependsOn(":catalogs:catalog-kafka:jar", ":catalogs:catalog-kafka:runtimeJars") | ||
|
|
||
| // Frontend tests depend on the web page, so we need to build the web module first. | ||
| dependsOn(":web:web:build") |
build.gradle.kts
Outdated
| from(projectDir.dir("bin")) { into("package/bin") } | ||
| from(projectDir.dir("web/web/build/libs/${rootProject.name}-web-$version.war")) { into("package/web") } | ||
| from(projectDir.dir("web-v2/web/build/libs/${rootProject.name}-web-$version.war")) { into("package/web-v2") } | ||
| if (!skipWebWar) { |
There was a problem hiding this comment.
Can we check whether this condition is really necessary? If the web module has no build artifacts, copying it shouldn’t have any effect anyway, right?
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Addressed the review suggestions for #10705 in commit 557dae8:
Validation done locally:
Also, the earlier |
|
Please update the pr description |
|
My initial feeling is that this makes the thing too complex. I need to take a deep look. |
|
|
||
| val skipWeb: Boolean = (project.findProperty("skipWeb") as? String)?.toBoolean() ?: false | ||
| val skipWebWar: Boolean = (project.findProperty("skipWebWar") as? String)?.toBoolean() ?: false | ||
| val skipFrontend: Boolean = skipWeb || skipWebWar |
There was a problem hiding this comment.
Add three different properties makes things so complicated, do we have a simple solution?
|
|
||
| tasks.test { | ||
| val skipWeb = (rootProject.findProperty("skipWeb") as? String)?.toBoolean() ?: false | ||
| val skipWebWar = (rootProject.findProperty("skipWebWar") as? String)?.toBoolean() ?: false |
There was a problem hiding this comment.
Does the skipWebWar need to be removed?
if you remove it, the skipFrontend will not be useful
What changes were proposed in this pull request?
-PskipWebWar=truesupport to skip web/web-v2 war build and packaging.gravitino.server.webui.enable(defaulttrue) and makeGravitinoServerrespect it, so deploy mode can start without war when web is skipped.skipWebWar.-PskipWebWar=true.Why are the changes needed?
In many non-frontend scenarios (especially integration tests), building web war is unnecessary and slows CI. This change avoids that cost while keeping server startup valid.
Fix: #10705
Does this PR introduce any user-facing change?
gravitino.server.webui.enable(defaulttrue).-PskipWebWar=true.How was this patch tested?
./gradlew --no-daemon :server:test -PskipITs./gradlew --no-daemon compileDistribution -PskipWebWar=true -x test./gradlew help -PskipWebWar=true -q