New (preview) Web UI scaffolding #22695

koszti · 2024-07-16T22:59:22Z

Description

This scaffolding PR aims to modernize the web UI for Trino, incorporating up-to-date tools and a contemporary look and feel.

Partially addressing and proposing technical solutions for #22697

Additional context and related issues

Currently, the Trino web UI uses outdated components like React 16, jQuery, and Bootstrap 3. Updating the UI to use more recent tools and frameworks will provide a fresher appearance and improved functionality. For instance, the Trino gateway utilizes Vite with modern dependencies, suggesting that the Trino web UI could benefit from adopting a similar approach for consistency.

Original slack thread: https://trinodb.slack.com/archives/CKCEWGYT0/p1720904086284609

IMPORTANT: This PR does not disrupt the current web app. Instead, it introduces a new web app scaffolding accessible at a new endpoint (/ui/preview), which is visible only in development mode. This approach allows for further development and testing and can switch to the new web app once migrating of all functionalities is completed.

This PR Utilizes the Following Tools and Development Features:

Using vite.js as the primary frontend tool. Similar to trino-gateway.
Hot Module Replacement (HMR) for instant and precise updates without reloading the page or losing the application state
Proxying API requests from the development UI to the Trino development server
Eliminating the need to rebuild the Java project after packaging, unlike the current web app

What's not included:

A React component library. We can decide on one later, with options like Semi Design (used in Trino Gateway) or Ant Design, among others.

To Develop the preview webapp locally

Start the Full Development Server as documented here. This step is only necessary once, and you won't need to restart it while developing the Preview Web UI. It is required solely to proxy API requests to valid server endpoints.

Once the full dev server is running start the preview webapp in dev mode:

$ cd core/trino-main/src/main/resources/webapp-preview
$ npm install && npm run dev

  VITE v5.3.4  ready in 108 ms

  ➜  Local:   http://localhost:5173/ui/preview
  ➜  Network: use --host to expose
  ➜  press h + enter to show help

To Build and package the preview webapp

Build steps are fully automated using Maven, eliminating the need to commit generated JS files to the git repository as was done with the current web app.

The preview web app is disabled by default and enabled only in the full development server by setting the web-ui.preview.enabled=true property. This ensures that the preview web app is available only to developers and not to production end users. Once all functionalities are migrated from the old web app to the new one, we can swap them, making the new web app the default for end users.

If the preview webapp is enabled then it's running in parallel to the current one and accessible at /ui/preview:

Release notes

(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

cla-bot · 2024-07-16T22:59:25Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

cla-bot · 2024-07-16T23:27:32Z

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

colebow · 2024-07-18T17:18:06Z

core/trino-main/src/main/resources/webapp-preview/src/App.tsx

+        <button onClick={() => setCount((count) => count + 1)}>
+          count is {count}


colebow · 2024-07-18T17:37:31Z

.github/DEVELOPMENT.md

@@ -256,6 +256,32 @@ To iterate quickly, simply re-build the project in IntelliJ after packaging is
 complete. Project resources will be hot-reloaded and changes are reflected on
 browser refresh.

+## Building the Preview Web UI


I think this likely shouldn't go straight into development.md yet - that file is for core development, and we can maintain this information closer to where the work is being done for now, as most people running/building Trino won't be interacting with the preview Web UI, especially when it's this barebones.

make sense. I moved it to README.md under the webapp-preview directory

colebow · 2024-07-18T17:40:47Z

core/trino-main/src/main/resources/webapp-preview/src/App.css

@@ -0,0 +1,42 @@
+#root {
+  max-width: 1280px;


Does this play nicely with 4K monitors as well as mobile?

good question. It's generated by vite using the react-typescript template documented in the getting started section:

npm create vite@latest webapp-preview -- --template react-ts

I think we should review these defaults and customise separately in another PR if needed.

wendigo · 2024-07-19T15:54:37Z

I like the idea. I always wanted to get rid of the old UI but never had the time or experience to start working on that. Thanks, @koszti for bringing this up!

colebow · 2024-07-22T17:57:17Z

I think you'll need to squash some of these commits before merge - Trino practice is to have each commit perform an atomic unit of work, so a commit and revert commit in the same PR should be removed/squashed into something else, and the empty CI commit needs to go. Force pushing is encouraged!

Otherwise, though, this looks like a solid start to me, and I'm excited to collaborate and get more done.

koszti · 2024-07-22T20:21:38Z

squashed everything to a single commit.

1 CI check failed. Seems like it's because of some timing issue which is not related to this PR. Is making all CI checks to be green a requirement to get things merged?

colebow · 2024-07-22T21:19:30Z

Generally we can merge through unrelated CI failures, though I'm not 100% sure that this is unrelated - it's a test for web UI authentication, which could be adjacent to this. You may need to add the new config property to TestOAuth2WebUiAuthenticationFilterWithJwt.java?

koszti · 2024-07-22T22:00:21Z

Hmm, authentication should not be changed by this PR. Can we somehow re-run only the failed test?

I'm nearly sure it was passing earlier ~~but after the squashed commit I no longer can see old checks~~. I can't reproduce it locally but based on the error message it says something about expired tokens of 2 seconds which makes me suspicious it's a timing issue or slow running CI checks (?):

 ExpiredJwt JWT expired 2116 milliseconds ago at 2024-07-22T19:36:42.000Z

I might be wrong but re-triggering the failed tests would give some confidence.

UPDATE: Here is the latest GHA on the last commit before the squash: https://github.com/trinodb/trino/actions/runs/10012690646/job/27678754938 . The problematic test passed but another one failed (trino-exasol) which must be irrelevant. 🤞

wendigo · 2024-07-23T08:14:47Z

I've retriggered trino-main alone. This could be a flaky test. Let's see :)

core/trino-main/pom.xml

core/trino-main/src/main/resources/webapp-preview/index.html

wendigo · 2024-07-23T08:18:57Z

testing/trino-server-dev/etc/config.properties

@@ -25,6 +25,8 @@ scheduler.http-client.idle-timeout=1m
 query.client.timeout=5m
 query.min-expire-age=30m

+web-ui.preview.enabled=true


I'd prefer not to add this here. Instead you can create a QueryRunner that exposes new web UI.

can you please give a bit more details or instructions how to do that by any chance? I'm not familiar with QueryRunner

/* * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */ package io.trino.server.ui; import io.airlift.log.Logger; import io.trino.server.testing.TestingTrinoServer; import io.trino.testing.StandaloneQueryRunner; import static io.trino.testing.TestingHandles.TEST_CATALOG_NAME; import static io.trino.testing.TestingSession.testSessionBuilder; public class PreviewUiQueryRunner { private PreviewUiQueryRunner() {} public static void main(String[] args) { StandaloneQueryRunner queryRunner = new StandaloneQueryRunner(testSessionBuilder() .setCatalog(TEST_CATALOG_NAME) .setSchema("tiny") .build(), PreviewUiQueryRunner::configureTrinoServer); Logger log = Logger.get(PreviewUiQueryRunner.class); log.info("======== SERVER STARTED ========"); log.info("\n====\n%s\n====", queryRunner.getCoordinator().getBaseUrl()); } private static void configureTrinoServer(TestingTrinoServer.Builder builder) { builder.addProperty("web-ui.preview.enabled", "true"); builder.addProperty("http-server.http.port", "8080"); } }

Something like that :)

thanks added. Indeed, using this class is a a much faster option rather than running the full dev sever.

wendigo · 2024-07-23T08:48:27Z

Yeah, it was flake after all :)

wendigo · 2024-07-23T09:16:23Z

@koszti rebased on master to resolve conflict with pom.xml

wendigo · 2024-07-23T19:17:19Z

@mosabua @colebow what's the plan forward with this?

mosabua · 2024-07-23T19:50:12Z

Good question .. probably should chat at maintainer meet and contributor call this week. Personally I think we want to move this forward.

koszti · 2024-07-23T21:03:49Z

Good question .. probably should chat at maintainer meet and contributor call this week. Personally I think we want to move this forward.

If it helps, I can add a few items I have in mind into #22697 as a starting point for a proper roadmap. But this will likely change, as I'm sure many of you have other ideas, and I can't create a complete list on my own. Let me know your thoughts.

mosabua · 2024-07-23T21:09:33Z

@koszti fleshing out the roadmap would be great .. just through your ideas on the issue and we can discuss and sort out more .. probably need to arrange sync calls over time

wendigo · 2024-07-24T06:48:33Z

@mosabua the only concern that I have at the moment with landing this is whether it will break reproducible builds.

wendigo · 2024-07-24T06:53:10Z

core/trino-main/src/main/java/io/trino/server/ui/PreviewUiQueryRunner.java

@@ -0,0 +1,44 @@
+/*


I think that this file should land in trino-main test sources

fixed, thanks

wendigo · 2024-07-24T06:58:57Z

@mosabua nevermind, tested, vite actually produces reproducible output

wendigo · 2024-07-24T07:00:28Z

core/trino-main/src/main/java/io/trino/server/ui/WebUiPreviewStaticResource.java

+            path = "index.html";
+        }
+
+        String fullPath = "/webapp-preview/dist/" + path;


can we drop dist from the final build dir? just /webapp-preview/ ?

The path needs to be the same in dev and in the prod build. I don't know how we can make it without having the dist subdir right now TBH.

By moving everything to webapp-preview we'll include all source files in the final build which isn't what we want to do.

wendigo · 2024-07-24T07:02:28Z

core/trino-main/src/main/resources/webapp-preview/.eslintrc.cjs

@@ -0,0 +1,18 @@
+module.exports = {


this could live in the directory: core/trino-main/src/javascript/ rather than in the resources as this will basically bundle sources with the target jar and we don't want that. we just want a build distribution being part of the jar.

are you sure? src/main/resources is a filtered directory and only the built assets moving into the target.

koszti · 2024-07-24T08:42:37Z

blaa the rebase hell kills my brain :< I messed up the PR. I'll try to fix in later today.

koszti · 2024-07-24T09:26:49Z

my bad. I accidentally discarded and closed the original PR. As a workaround I created another PR with all the changes discussed in this PR at #22783. Let's continue there please 🙇

koszti changed the title ~~Preview Web UI scaffolding~~ New (preview) Web UI scaffolding Jul 16, 2024

mosabua requested a review from colebow July 16, 2024 23:17

colebow reviewed Jul 18, 2024

View reviewed changes

cla-bot bot added the cla-signed label Jul 19, 2024

colebow approved these changes Jul 22, 2024

View reviewed changes

koszti force-pushed the preview-ui-scaffold branch from 4f5deb1 to 8c3fa19 Compare July 22, 2024 19:25

wendigo reviewed Jul 23, 2024

View reviewed changes

core/trino-main/pom.xml Outdated Show resolved Hide resolved

wendigo reviewed Jul 23, 2024

View reviewed changes

core/trino-main/src/main/resources/webapp-preview/index.html Outdated Show resolved Hide resolved

wendigo reviewed Jul 23, 2024

View reviewed changes

wendigo force-pushed the preview-ui-scaffold branch from 8c3fa19 to 845c102 Compare July 23, 2024 09:15

koszti force-pushed the preview-ui-scaffold branch 4 times, most recently from 5dfb733 to 2a672e4 Compare July 23, 2024 12:34

koszti force-pushed the preview-ui-scaffold branch from 2a672e4 to 0ca04ea Compare July 23, 2024 20:54

wendigo reviewed Jul 24, 2024

View reviewed changes

koszti force-pushed the preview-ui-scaffold branch from 0ca04ea to 6483023 Compare July 24, 2024 08:35

koszti closed this Jul 24, 2024

koszti force-pushed the preview-ui-scaffold branch from 6483023 to 0002afa Compare July 24, 2024 08:40

koszti mentioned this pull request Jul 24, 2024

New (preview) Web UI scaffolding #22783

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New (preview) Web UI scaffolding #22695

New (preview) Web UI scaffolding #22695

koszti commented Jul 16, 2024 •

edited

Loading

cla-bot bot commented Jul 16, 2024

cla-bot bot commented Jul 16, 2024

colebow Jul 18, 2024

colebow Jul 18, 2024 •

edited

Loading

koszti Jul 19, 2024 •

edited

Loading

colebow Jul 18, 2024

koszti Jul 19, 2024 •

edited

Loading

wendigo commented Jul 19, 2024

colebow commented Jul 22, 2024 •

edited

Loading

koszti commented Jul 22, 2024 •

edited

Loading

colebow commented Jul 22, 2024

koszti commented Jul 22, 2024 •

edited

Loading

wendigo commented Jul 23, 2024

wendigo Jul 23, 2024

koszti Jul 23, 2024

wendigo Jul 23, 2024

koszti Jul 23, 2024

wendigo commented Jul 23, 2024

wendigo commented Jul 23, 2024

wendigo commented Jul 23, 2024

mosabua commented Jul 23, 2024

koszti commented Jul 23, 2024

mosabua commented Jul 23, 2024 •

edited

Loading

wendigo commented Jul 24, 2024

wendigo Jul 24, 2024

koszti Jul 24, 2024

wendigo commented Jul 24, 2024

wendigo Jul 24, 2024

koszti Jul 24, 2024 •

edited

Loading

wendigo Jul 24, 2024

koszti Jul 24, 2024 •

edited

Loading

koszti commented Jul 24, 2024 •

edited

Loading

koszti commented Jul 24, 2024

		<button onClick={() => setCount((count) => count + 1)}>
		count is {count}

New (preview) Web UI scaffolding #22695

New (preview) Web UI scaffolding #22695

Conversation

koszti commented Jul 16, 2024 • edited Loading

Description

Additional context and related issues

To Develop the preview webapp locally

To Build and package the preview webapp

Release notes

cla-bot bot commented Jul 16, 2024

cla-bot bot commented Jul 16, 2024

Choose a reason for hiding this comment

colebow Jul 18, 2024 • edited Loading

Choose a reason for hiding this comment

koszti Jul 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koszti Jul 19, 2024 • edited Loading

Choose a reason for hiding this comment

wendigo commented Jul 19, 2024

colebow commented Jul 22, 2024 • edited Loading

koszti commented Jul 22, 2024 • edited Loading

colebow commented Jul 22, 2024

koszti commented Jul 22, 2024 • edited Loading

wendigo commented Jul 23, 2024

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

wendigo commented Jul 23, 2024

wendigo commented Jul 23, 2024

wendigo commented Jul 23, 2024

mosabua commented Jul 23, 2024

koszti commented Jul 23, 2024

mosabua commented Jul 23, 2024 • edited Loading

wendigo commented Jul 24, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wendigo commented Jul 24, 2024

Choose a reason for hiding this comment

koszti Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

koszti Jul 24, 2024 • edited Loading

Choose a reason for hiding this comment

koszti commented Jul 24, 2024 • edited Loading

koszti commented Jul 24, 2024

koszti commented Jul 16, 2024 •

edited

Loading

colebow Jul 18, 2024 •

edited

Loading

koszti Jul 19, 2024 •

edited

Loading

koszti Jul 19, 2024 •

edited

Loading

colebow commented Jul 22, 2024 •

edited

Loading

koszti commented Jul 22, 2024 •

edited

Loading

koszti commented Jul 22, 2024 •

edited

Loading

mosabua commented Jul 23, 2024 •

edited

Loading

koszti Jul 24, 2024 •

edited

Loading

koszti Jul 24, 2024 •

edited

Loading

koszti commented Jul 24, 2024 •

edited

Loading