[BUILD] `build/dist` supports `--web-ui` #4397

pan3793 · 2023-02-22T08:27:03Z

Why are the changes needed?

Usage:
+----------------------------------------------------------------------------------------------+
| ./build/dist [--name <custom_name>] [--tgz] [--web-ui] [--flink-provided] [--hive-provided]  |
|              [--spark-provided] [--mvn <maven_executable>] <maven build options>             |
+----------------------------------------------------------------------------------------------+
name:           -  custom binary name, using project version if undefined
tgz:            -  whether to make a whole bundled package
web-ui:         -  whether to include web ui
flink-provided: -  whether to make a package without Flink binary
hive-provided:  -  whether to make a package without Hive binary
spark-provided: -  whether to make a package without Spark binary
mvn:            -  external maven executable location

How was this patch tested?

Add some test cases that check the changes thoroughly including negative and positive cases if possible
Add screenshots for manual tests if appropriate

Create binary artifacts using build/dist --tgz --web-ui and run, then open http://0.0.0.0:10099/ui

Run test locally before make a pull request

pan3793 · 2023-02-22T08:27:31Z

cc @zhaohehuhu

codecov-commenter · 2023-02-22T11:15:30Z

Codecov Report

Merging #4397 (d308def) into master (e4d0962) will decrease coverage by 0.08%.
The diff coverage is 100.00%.

❗ Current head d308def differs from pull request most recent head 97901d6. Consider uploading reports for the commit 97901d6 to get more accurate results

@@             Coverage Diff              @@
##             master    #4397      +/-   ##
============================================
- Coverage     53.78%   53.71%   -0.08%     
  Complexity       13       13              
============================================
  Files           564      564              
  Lines         30909    30914       +5     
  Branches       4162     4162              
============================================
- Hits          16623    16604      -19     
- Misses        12733    12755      +22     
- Partials       1553     1555       +2

Impacted Files	Coverage Δ
...ver/http/authentication/AuthenticationFilter.scala	`91.66% <ø> (-0.12%)`	⬇️
...ache/kyuubi/server/KyuubiRestFrontendService.scala	`84.69% <100.00%> (+0.99%)`	⬆️
...e/kyuubi/jdbc/hive/ClosedOrCancelledException.java	`0.00% <0.00%> (-100.00%)`	⬇️
...kyuubi/server/trino/api/v1/StatementResource.scala	`50.66% <0.00%> (-8.00%)`	⬇️
.../apache/kyuubi/server/api/v1/BatchesResource.scala	`68.83% <0.00%> (-2.80%)`	⬇️
...in/spark/authz/ranger/SparkRangerAdminPlugin.scala	`64.47% <0.00%> (-2.64%)`	⬇️
...g/apache/kyuubi/operation/BatchJobSubmission.scala	`75.27% <0.00%> (-2.20%)`	⬇️
...rg/apache/kyuubi/ctl/cmd/log/LogBatchCommand.scala	`59.09% <0.00%> (-1.52%)`	⬇️
.../engine/spark/session/SparkSQLSessionManager.scala	`77.50% <0.00%> (-1.25%)`	⬇️
...apache/kyuubi/service/TBinaryFrontendService.scala	`48.38% <0.00%> (-1.08%)`	⬇️
... and 1 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

pan3793 · 2023-02-22T15:26:47Z

...erver/src/main/scala/org/apache/kyuubi/server/http/authentication/AuthenticationFilter.scala

@@ -79,7 +79,6 @@ class AuthenticationFilter(conf: KyuubiConf) extends Filter with Logging {

  override def init(filterConfig: FilterConfig): Unit = {
    initAuthHandlers()
-    super.init(filterConfig)


To suppress the follow exception.

java.lang.AbstractMethodError: javax.servlet.Filter.init(Ljavax/servlet/FilterConfig;)V at org.apache.kyuubi.server.http.authentication.AuthenticationFilter.init(AuthenticationFilter.scala:82) ~[kyuubi-server_2.12-1.8.0-SNAPSHOT.jar:1.8.0-SNAPSHOT] at org.eclipse.jetty.servlet.FilterHolder.initialize(FilterHolder.java:140) ~[jetty-servlet-9.4.50.v20221201.jar:9.4.50.v20221201] at org.eclipse.jetty.servlet.ServletHandler.lambda$initialize$0(ServletHandler.java:750) ~[jetty-servlet-9.4.50.v20221201.jar:9.4.50.v20221201] at java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:948) ~[?:1.8.0_332] at java.util.stream.Streams$ConcatSpliterator.forEachRemaining(Streams.java:742) ~[?:1.8.0_332] at java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:647) ~[?:1.8.0_332] at org.eclipse.jetty.servlet.ServletHandler.initialize(ServletHandler.java:774) ~[jetty-servlet-9.4.50.v20221201.jar:9.4.50.v20221201] at org.eclipse.jetty.servlet.ServletContextHandler.startContext(ServletContextHandler.java:379) ~[jetty-servlet-9.4.50.v20221201.jar:9.4.50.v20221201] at org.eclipse.jetty.server.handler.ContextHandler.doStart(ContextHandler.java:916) ~[jetty-server-9.4.50.v20221201.jar:9.4.50.v20221201] at org.eclipse.jetty.servlet.ServletContextHandler.doStart(ServletContextHandler.java:288) ~[jetty-servlet-9.4.50.v20221201.jar:9.4.50.v20221201] at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:73) ~[jetty-util-9.4.50.v20221201.jar:9.4.50.v20221201] at org.apache.kyuubi.server.http.authentication.KyuubiHttpAuthenticationFactory$HttpHandlerWrapperFactory$$anon$1.doStart(KyuubiHttpAuthenticationFactory.scala:89) ~[kyuubi-server_2.12-1.8.0-SNAPSHOT.jar:1.8.0-SNAPSHOT] at org.eclipse.jetty.util.component.AbstractLifeCycle.start(AbstractLifeCycle.java:73) ~[jetty-util-9.4.50.v20221201.jar:9.4.50.v20221201] at org.apache.kyuubi.server.ui.JettyServer.addHandler(JettyServer.scala:56) ~[kyuubi-server_2.12-1.8.0-SNAPSHOT.jar:1.8.0-SNAPSHOT] at org.apache.kyuubi.server.KyuubiRestFrontendService.startInternal(KyuubiRestFrontendService.scala:88) ~[kyuubi-server_2.12-1.8.0-SNAPSHOT.jar:1.8.0-SNAPSHOT] at org.apache.kyuubi.server.KyuubiRestFrontendService.start(KyuubiRestFrontendService.scala:170) ~[kyuubi-server_2.12-1.8.0-SNAPSHOT.jar:1.8.0-SNAPSHOT] at org.apache.kyuubi.service.CompositeService.$anonfun$start$1(CompositeService.scala:47) ~[kyuubi-common_2.12-1.8.0-SNAPSHOT.jar:1.8.0-SNAPSHOT] at org.apache.kyuubi.service.CompositeService.$anonfun$start$1$adapted(CompositeService.scala:45) ~[kyuubi-common_2.12-1.8.0-SNAPSHOT.jar:1.8.0-SNAPSHOT] at scala.collection.mutable.ResizableArray.foreach(ResizableArray.scala:62) ~[scala-library-2.12.17.jar:?] at scala.collection.mutable.ResizableArray.foreach$(ResizableArray.scala:55) ~[scala-library-2.12.17.jar:?] at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:49) ~[scala-library-2.12.17.jar:?] at org.apache.kyuubi.service.CompositeService.start(CompositeService.scala:45) ~[kyuubi-common_2.12-1.8.0-SNAPSHOT.jar:1.8.0-SNAPSHOT] at org.apache.kyuubi.service.Serverable.start(Serverable.scala:51) ~[kyuubi-common_2.12-1.8.0-SNAPSHOT.jar:1.8.0-SNAPSHOT] at org.apache.kyuubi.server.KyuubiServer.start(KyuubiServer.scala:182) ~[kyuubi-server_2.12-1.8.0-SNAPSHOT.jar:1.8.0-SNAPSHOT] at org.apache.kyuubi.server.KyuubiServer$.startServer(KyuubiServer.scala:67) ~[kyuubi-server_2.12-1.8.0-SNAPSHOT.jar:1.8.0-SNAPSHOT] at org.apache.kyuubi.server.KyuubiServer$.main(KyuubiServer.scala:99) ~[kyuubi-server_2.12-1.8.0-SNAPSHOT.jar:1.8.0-SNAPSHOT] at org.apache.kyuubi.server.KyuubiServer.main(KyuubiServer.scala) ~[kyuubi-server_2.12-1.8.0-SNAPSHOT.jar:1.8.0-SNAPSHOT]

pan3793 · 2023-02-23T02:16:54Z

pom.xml

+                    <artifactId>frontend-maven-plugin</artifactId>
+                    <version>${maven.plugin.frontend.version}</version>
+                    <configuration>
+                        <nodeDownloadRoot>https://npmmirror.com/mirrors/node/</nodeDownloadRoot>


This mirror contains all released versions, not like Apache mirror which only contains the "current version".

Should we set the official download address by default and give a profile to change download source.

OK, moved to mirror-cdn, you can verify by w/ and w/o -Pmirror-cdn

I'm not sure, only set to properties can be noticed by frontend-maven-plugin's configuration ?

You can clear the local cache and do package, the log will show the download link

Yeah, i have tested locally. W/ -Pmirror-cdn, see log output as expected.

Downloading https://npmmirror.com/mirrors/node/v16.19.1/node-v16.19.1-darwin-x64.tar.gz to Downloading https://registry.npmmirror.com/pnpm/-/pnpm-7.27.1.tgz to

zwangsheng

LGTM

zwangsheng · 2023-02-23T02:42:36Z

pom.xml

+                    <artifactId>frontend-maven-plugin</artifactId>
+                    <version>${maven.plugin.frontend.version}</version>
+                    <configuration>
+                        <nodeDownloadRoot>https://npmmirror.com/mirrors/node/</nodeDownloadRoot>


Should we set the official download address by default and give a profile to change download source.

cfmcgrady · 2023-02-23T03:26:50Z

Tested locally, it works as expected.

pan3793 · 2023-02-23T03:45:44Z

Thanks, merged to master

github-actions bot added kind:build kind:deploy module:server labels Feb 22, 2023

pan3793 changed the title ~~[BUILD] build/dist support --web-ui~~ [BUILD] build/dist supports --web-ui Feb 22, 2023

pan3793 commented Feb 22, 2023

View reviewed changes

[BUILD] build/dist support --web-ui

f0ac16b

pan3793 force-pushed the webui-build branch from b486574 to f0ac16b Compare February 22, 2023 19:37

github-actions bot added the kind:infra license, community building, project builds, asf infra related, etc. label Feb 22, 2023

pan3793 added 3 commits February 23, 2023 03:56

regex

7091d5b

nit

c1d184a

nit

9abca47

pan3793 requested a review from cfmcgrady February 22, 2023 20:32

nit

d308def

pan3793 self-assigned this Feb 23, 2023

pan3793 added this to the v1.8.0 milestone Feb 23, 2023

remove unused dep

c5751dd

pan3793 commented Feb 23, 2023

View reviewed changes

zwangsheng approved these changes Feb 23, 2023

View reviewed changes

pan3793 added 2 commits February 23, 2023 11:05

mirror-cdn

37d5e2a

doc

97901d6

github-actions bot added the kind:documentation Documentation is a feature! label Feb 23, 2023

cfmcgrady approved these changes Feb 23, 2023

View reviewed changes

pan3793 closed this in fafd017 Feb 23, 2023

pan3793 deleted the webui-build branch February 23, 2023 03:54

zwangsheng mentioned this pull request Mar 6, 2023

[Subtask][KPIP-5] Build Binary Package with Web UI Module #3314

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUILD] `build/dist` supports `--web-ui` #4397

[BUILD] `build/dist` supports `--web-ui` #4397

pan3793 commented Feb 22, 2023 •

edited

pan3793 commented Feb 22, 2023

codecov-commenter commented Feb 22, 2023 •

edited

pan3793 Feb 22, 2023

pan3793 Feb 23, 2023

zwangsheng Feb 23, 2023

pan3793 Feb 23, 2023

zwangsheng Feb 23, 2023

pan3793 Feb 23, 2023

zwangsheng Feb 23, 2023

zwangsheng left a comment

zwangsheng Feb 23, 2023

cfmcgrady commented Feb 23, 2023

pan3793 commented Feb 23, 2023

[BUILD] build/dist supports --web-ui #4397

[BUILD] build/dist supports --web-ui #4397

Conversation

pan3793 commented Feb 22, 2023 • edited

Why are the changes needed?

How was this patch tested?

pan3793 commented Feb 22, 2023

codecov-commenter commented Feb 22, 2023 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zwangsheng left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cfmcgrady commented Feb 23, 2023

pan3793 commented Feb 23, 2023

[BUILD] `build/dist` supports `--web-ui` #4397

[BUILD] `build/dist` supports `--web-ui` #4397

pan3793 commented Feb 22, 2023 •

edited

codecov-commenter commented Feb 22, 2023 •

edited