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.

Sign up for GitHub

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

Jump to bottom

Upgrade presto-maven-plugin to 0.5 #23096

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Upgrade presto-maven-plugin to 0.5 #23096

wants to merge 1 commit into from

Conversation

dnskr
Copy link
Contributor

@dnskr dnskr commented Jun 27, 2024

Description

Upgrade presto-maven-plugin to the latest 0.5 version.

Motivation and Context

Release presto-maven-plugin:0.5 has fix prestodb/presto-maven-plugin#11 that resolves prestodb/presto-maven-plugin#1 issue.
The fix allows to build (at least) presto-tpch, presto-password-authenticators, presto-session-property-managers, presto-node-ttl-fetchers and presto-cluster-ttl-providers modules on Windows.

Impact

Developers can build more Presto modules on Windows.

Test Plan

Tested manually with the following commands:

mvn -f presto-tpch/pom.xml clean install -DskipTests=true
mvn -f presto-password-authenticators/pom.xml clean install -DskipTests=true
mvn -f presto-session-property-managers/pom.xml clean install -DskipTests=true
mvn -f presto-node-ttl-fetchers/pom.xml clean install -DskipTests=true
mvn -f presto-cluster-ttl-providers/pom.xml clean install -DskipTests=true

For instance, build result for presto-tpch before the upgrade:

$ mvn -f presto-tpch/pom.xml clean install -DskipTests=true
[INFO] Scanning for projects...
[INFO] 
[INFO] Using the MultiThreadedBuilder implementation with a thread count of 12
[INFO] 
[INFO] ------------------< com.facebook.presto:presto-tpch >-------------------
[INFO] Building presto-tpch 0.289-SNAPSHOT
...
[INFO] --- presto:0.4:generate-service-descriptor (default-generate-service-descriptor) @ presto-tpch ---
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  6.519 s (Wall Clock)
[INFO] Finished at: 2024-06-26T23:42:49+02:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal com.facebook.presto:presto-maven-plugin:0.4:generate-service-descriptor (default-generate-service-descriptor) on project presto-tpch: Execution default-generate-service-descriptor of goal com.facebook.presto:prest
o-maven-plugin:0.4:generate-service-descriptor failed: A required class was missing while executing com.facebook.presto:presto-maven-plugin:0.4:generate-service-descriptor: com/facebook/presto/tpch/ColumnNaming (wrong name: com\facebook\presto\tpch\ColumnNaming)

Build result for presto-tpch after the upgrade:

$ mvn -f presto-tpch/pom.xml clean install -DskipTests=true
[INFO] Scanning for projects...
[INFO] 
[INFO] Using the MultiThreadedBuilder implementation with a thread count of 12
[INFO] 
[INFO] ------------------< com.facebook.presto:presto-tpch >-------------------
[INFO] Building presto-tpch 0.289-SNAPSHOT
...
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  10.322 s (Wall Clock)
[INFO] Finished at: 2024-06-26T23:42:19+02:00
[INFO] ------------------------------------------------------------------------

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 ==

@dnskr dnskr requested a review from a team as a code owner June 27, 2024 11:44
@dnskr dnskr requested a review from presto-oss June 27, 2024 11:44
@dnskr dnskr marked this pull request as draft June 27, 2024 11:51
@dnskr dnskr marked this pull request as ready for review July 26, 2024 18:18
@dnskr dnskr force-pushed the upgrade-presto-maven-plugin-to-0.5 branch from 9a33cd5 to bc559e8 Compare July 26, 2024 18:31
tdcmeehan
tdcmeehan approved these changes Jul 26, 2024
View reviewed changes
Copy link
Contributor

@tdcmeehan tdcmeehan left a comment

Choose a reason for hiding this comment

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

Thank you @dnskr!

@tdcmeehan
Copy link
Contributor

It looks like the release isn't in central. Do you know if it was published?

@dnskr
Copy link
Contributor Author

dnskr commented Jul 26, 2024

It looks like the release isn't in central. Do you know if it was published?

It was not published due to credential issue and looks like the issue is not resolved yet.

@dnskr dnskr marked this pull request as draft November 24, 2024 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tdcmeehan tdcmeehan tdcmeehan approved these changes

@presto-oss presto-oss Awaiting requested review from presto-oss presto-oss is a code owner automatically assigned from prestodb/committers

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Windows Support
2 participants
@dnskr @tdcmeehan