Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure inclusion of UI in JAR and container artifacts #337

Merged
merged 1 commit into from
May 8, 2024

Conversation

andythsu
Copy link
Member

@andythsu andythsu commented May 8, 2024

Description

  • Change the webapp outputDirectory to ${project.build.outputDirectory}/static
  • Move copy static step to prepare-package phase

Additional context and related issues

Release notes

( ) 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:

* 

@cla-bot cla-bot bot added the cla-signed label May 8, 2024
@andythsu andythsu changed the title switch the maven plugin order to build frontend webapp before copying… switch the maven plugin order to build frontend webapp first May 8, 2024
Copy link
Member

@ebyhr ebyhr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please follow the commit message guideline (capitalize and shorten): https://github.com/trinodb/trino/blob/master/.github/DEVELOPMENT.md#format-git-commit-messages

gateway-ha/pom.xml Outdated Show resolved Hide resolved
@andythsu andythsu changed the title switch the maven plugin order to build frontend webapp first Switch the maven plugin order to build frontend webapp first May 8, 2024
Copy link
Member

@mosabua mosabua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of relying on the order in the pom .. how about we move them into separate phases instead.

Full list is

pre-clean, clean, post-clean, validate, initialize, generate-sources, process-sources, generate-resources, process-resources, compile, process-classes, generate-test-sources, process-test-sources, generate-test-resources, process-test-resources, test-compile, process-test-classes, test, prepare-package, package, pre-integration-test, integration-test, post-integration-test, verify, install, deploy, pre-site, site, post-site, site-deploy

gateway-ha/pom.xml Outdated Show resolved Hide resolved
gateway-ha/pom.xml Outdated Show resolved Hide resolved
@mosabua
Copy link
Member

mosabua commented May 8, 2024

Worked with @andythsu - we think we got a cleaner solution. He is testing and will update PR.

@andythsu andythsu changed the title Switch the maven plugin order to build frontend webapp first Change the outputDirectory to ${project.build.outputDirectory}/static May 8, 2024
@andythsu andythsu changed the title Change the outputDirectory to ${project.build.outputDirectory}/static Change the webapp outputDirectory to ${project.build.outputDirectory}/static May 8, 2024
@andythsu
Copy link
Member Author

andythsu commented May 8, 2024

This is working fine for me. Would like to get someone else to test it

@mosabua
Copy link
Member

mosabua commented May 8, 2024

Building locally from scratch I get

jar tvf gateway-ha-9-SNAPSHOT-jar-with-dependencies.jar  | grep static
     0 Wed May 08 13:15:04 PDT 2024 static/
   460 Wed May 08 13:15:04 PDT 2024 static/index.html
     0 Wed May 08 13:15:04 PDT 2024 static/assets/
2464900 Wed May 08 13:15:04 PDT 2024 static/assets/index-jdJ8WnTU.js
428462 Wed May 08 13:15:04 PDT 2024 static/assets/index-WjKhXYTT.css
  9457 Wed May 08 13:15:04 PDT 2024 static/logo.svg

I tried firing up the jar directly with the quickstart setup (without the db but anyway) - all good - I do get the UI.

I built the docker container and fired it up with docker compose and that worked as well and showed the UI.

@mosabua
Copy link
Member

mosabua commented May 8, 2024

Lets change commit message to

"Fix build to ensure inclusion of UI in artifacts"

or just

"Ensure inclusion of UI in artifacts"

or

"Ensure inclusion of UI in JAR and container artifacts"

thoughts @colebow ?

Copy link
Member

@oneonestar oneonestar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before (statics missing in the first run):

$ git clone https://github.com/trinodb/trino-gateway.git
Cloning into 'trino-gateway'...
remote: Enumerating objects: 4616, done.
remote: Counting objects: 100% (254/254), done.
remote: Compressing objects: 100% (172/172), done.
remote: Total 4616 (delta 70), reused 197 (delta 47), pack-reused 4362
Receiving objects: 100% (4616/4616), 5.54 MiB | 12.15 MiB/s, done.
Resolving deltas: 100% (2197/2197), done.

$ cd trino-gateway

$ ./mvnw package -DskipTests=true -Dmaven.test.skip -q

$ jar tvf gateway-ha/target/gateway-ha-9-SNAPSHOT-jar-with-dependencies.jar | grep static
FAIL: 1

$ ./mvnw package -DskipTests=true -Dmaven.test.skip -q

$ jar tvf gateway-ha/target/gateway-ha-9-SNAPSHOT-jar-with-dependencies.jar | grep static
     0 Thu May 09 07:58:10 JST 2024 static/
   460 Thu May 09 07:58:10 JST 2024 static/index.html
     0 Thu May 09 07:58:10 JST 2024 static/assets/
428436 Thu May 09 07:58:10 JST 2024 static/assets/index-fsYBw5m6.css
2462945 Thu May 09 07:58:10 JST 2024 static/assets/index-q6-9m5Vn.js
  9457 Thu May 09 07:58:10 JST 2024 static/logo.svg

After:

$ rm -rf trino-gateway

$ git clone https://github.com/trinodb/trino-gateway.git
Cloning into 'trino-gateway'...
remote: Enumerating objects: 4616, done.
remote: Counting objects: 100% (253/253), done.
remote: Compressing objects: 100% (171/171), done.
remote: Total 4616 (delta 70), reused 197 (delta 47), pack-reused 4363
Receiving objects: 100% (4616/4616), 5.54 MiB | 15.47 MiB/s, done.
Resolving deltas: 100% (2196/2196), done.

$ cd trino-gateway

$ gh pr checkout 337
remote: Enumerating objects: 4, done.
remote: Counting objects: 100% (4/4), done.
remote: Total 4 (delta 3), reused 4 (delta 3), pack-reused 0
Unpacking objects: 100% (4/4), 452 bytes | 64.00 KiB/s, done.
From https://github.com/trinodb/trino-gateway
 * [new ref]         refs/pull/337/head -> jenkins
Switched to branch 'jenkins'

$ ./mvnw package -DskipTests=true -Dmaven.test.skip -q

$ jar tvf gateway-ha/target/gateway-ha-9-SNAPSHOT-jar-with-dependencies.jar | grep static
     0 Thu May 09 07:59:14 JST 2024 static/
   460 Thu May 09 07:59:14 JST 2024 static/index.html
     0 Thu May 09 07:59:14 JST 2024 static/assets/
428436 Thu May 09 07:59:14 JST 2024 static/assets/index-fsYBw5m6.css
2462945 Thu May 09 07:59:14 JST 2024 static/assets/index-q6-9m5Vn.js
  9457 Thu May 09 07:59:14 JST 2024 static/logo.svg

@mosabua mosabua changed the title Change the webapp outputDirectory to ${project.build.outputDirectory}/static Ensure inclusion of UI in JAR and container artifacts May 8, 2024
@mosabua mosabua merged commit 6363a88 into trinodb:main May 8, 2024
2 checks passed
@github-actions github-actions bot added this to the 9 milestone May 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants