Move UI to separate module

[ZacBlanco]

Description

Moves the UI to its own module which can be built in parallel or excluded entirely from the build in presto-main.

Fixes #22741

Motivation and Context

Discussed in the TSC meeting on 2024-05-14. The UI doesn't actually depend on anything in presto-main. This also prevents having the yarn build phases generate files into the source directories.

Impact

No public facing changes.

• The UI module can be skipped in the build by passing -pl '!presto-ui' to maven.

Test Plan

• built ui module separately and load SPA resources successfully
• built with maven via ./mvnw clean install -pl presto-main -am -DskipTests and then ran each of the IcebergQueryRunner and HiveQueryRunner to test the UI. Ran a few queries and clicked on all pages to ensure that the UI loaded each page successfully

Contributor checklist

• Please make sure your submission complies with our development, formatting, commit message, and attribution guidelines.
• PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
• Documented new properties (with its default value), SQL syntax, functions, or other functionality.
• If release notes are required, they follow the release notes guidelines.
• Adequate tests were added if applicable.
• CI passed.

Release Notes

== NO RELEASE NOTE ==

[ZacBlanco]

FYI: @yhwang

[ZacBlanco]

Also FYI @tdcmeehan if this merges, you will probably need to update the path in the CODEOWNERS file for @yhwang

[yhwang]

FYI, in the documentation, it's called Presto Console. should we use presto-console instead of presto-ui ?

[steveburnett]

FYI, in the documentation, it's called Presto Console. should we use presto-console instead of presto-ui ?

A good idea! - but maybe not necessary now. I suggest keeping this simple and leaving this PR as is. After this PR is merged and the UI is moved to its own module, then renaming the module could be a separate and also simple task for a followup PR.

[ZacBlanco]

I agree with @steveburnett 's opinion. I also feel that "UI" is slightly less ambiguous in naming than "console" where "console" could refer to a terminal or a user interface. Either way, I don't have super strong feelings but if we feel that -ui is not sufficient for the name we can rename it in a later PR.

[steveburnett]

LGTM!

[yhwang]

@ZacBlanco the SPA page doesn't work. JS files are not embedded into the HTML page.
The rest of the reorg looks good to me!

[ZacBlanco]

Thanks for pointing that out @yhwang . The issue should be solved now

[yhwang]

the last comment is the size of the SPA, The original size was about 821K, and now it jumps to 5.7M. Since it includes all of the JS in the webpack.conf.js. I hit this before and separated the SPA to another webpack file. I don't have solution to have them in the same webpack. If you can solve that, it would be great.

Edit
I believe I tried the scriptMatchPattern option of the html-inline-script-webpack-plugin plugin. But I can't get it work properly.

[ZacBlanco]

Thanks for pointing out the size issue too. I fixed it by including the chunks argument on the HtmlWebpackPlugin config.

[yhwang]

LGTM!

Also good to know the chunk option of the html-webpack-plugin.

[tdcmeehan]

Thanks @ZacBlanco!

[rschlussel]

@ZacBlanco why does presto-main have a dependency on presto-ui? When I run a build excluding the presto-ui module I get an error like

[ERROR] Failed to execute goal on project presto-main: Could not resolve dependencies for project com.facebook.presto:presto-main:jar:0.288-SNAPSHOT: com.facebook.presto:presto-ui:jar:0.288-SNAPSHOT was not found in https://maven.thefacebook.com/nexus/content/groups/public during a previous attempt. This failure was cached in the local repository and resolution is not reattempted until the update interval of nexus has elapsed or updates are forced -> [Help 1]

[tdcmeehan]

@rschlussel it's probably because the the Presto server has a dependency on the UI assets (it's packaged as part of the server). Although I can see this goes against the goal of optionally allowing one to separate out the UI. Perhaps a further refinement would be to version them separately (i.e. move it out of the monorepo entirely)?

[ZacBlanco]

The dependency is used in order to force building presto-ui so that we don't accidentally forget to include it on the CP/release build. Without specifying the dependency, you can build presto-main and the UI won't be available unless the dependency is specifically included on the JVM classpath which is a situation I aimed to avoid. I also wanted to make sure that it wasn't accidentally left out in a release build.

If you build the presto-ui module once and get it installed to your local maven cache, you should be able to avoid building it later on. e.g. ./mvnw install -pl presto-ui -DskipTests, then you should be able to run ./mvnw install -pl 'presto-main,!presto-ui' -am -DskipTests.

[rschlussel]

yes, but the problem is that I can't build it locally because I get errors downloading node.js (presumably some kind of firewall issue we need to figure out). That's why i want to exclude it in the first place.
2 questions

1. in general should this be a dependency of presto-server rather than presto-main?
2. for either case, maybe we can have a build flag to skip the ui entirely.

[ZacBlanco]

I sometimes use the UI locally launching HiveQueryRunner or IcebergQueryRunner, so I would prefer to not have to have it packaged in presto-server (some people were thinking about deprecating/removing that module anyways?). One option that might work is to create a profile target which turns the dependency into a runtime or provided dependency so maven doesn't complain about not finding the jar. Then you could just add something like -P no-ui to the maven command and it should work. I'll try to make a PR for that today.

[elharo]

There are other ways to make sure we build the UI. I agree with @rschlussel that presto-main should not depend on presto-ui.