wombatu-kun commented on code in PR #18837:
URL: https://github.com/apache/hudi/pull/18837#discussion_r3628851132
##########
pom.xml:
##########
@@ -2306,6 +2304,13 @@
<module>packaging/hudi-metaserver-server-bundle</module>
</modules>
</profile>
+ <profile>
+ <!-- RFC-105: non-shaded Trino connector. JDK 25 + Trino SPI; off by
default. -->
+ <id>hudi-trino</id>
+ <modules>
+ <module>hudi-trino</module>
Review Comment:
No release path builds or stages `org.apache.hudi:hudi-trino`: both
`deploy_staging_jars.sh` and `deploy_staging_jars_java17.sh` abort unless
JAVA_HOME is 11 or 17 respectively and neither passes `-Phudi-trino`, while the
module enforces JDK 25, and the published `hudi-trino-bundle` is dropped here.
How is the artifact the Trino-side shim depends on meant to be staged for a
release?
##########
.github/workflows/hudi_trino_compat.yml:
##########
@@ -0,0 +1,101 @@
+name: Hudi Trino SPI Compatibility
+
+on:
+ schedule:
+ - cron: '17 4 * * *'
+ workflow_dispatch:
+
+# The failure handler files/updates a drift report issue.
+permissions:
+ contents: read
+ issues: write
+
+env:
+ MVN_ARGS: -e -ntp -B -V -Dgpg.skip -Djacoco.skip
+
+jobs:
+ compile-against-trino-master:
+ name: Compile hudi-trino against trinodb/trino master
+ runs-on: ubuntu-latest
+ steps:
+ - name: Checkout Hudi
+ uses: actions/checkout@v5
+ with:
+ path: hudi
+ - name: Checkout trinodb/trino master
+ uses: actions/checkout@v5
+ with:
+ repository: trinodb/trino
+ ref: master
+ path: trino
+ # Hudi targets Java 11 and uses Lombok 1.18.36, which does not run on
JDK 25.
+ # Install the upstream Hudi modules under JDK 17 first, then compile the
connector
+ # under JDK 25.
+ - name: Set up JDK 17
+ uses: actions/setup-java@v5
+ with:
+ java-version: '17'
+ distribution: 'temurin'
+ cache: maven
+ - name: Install upstream Hudi modules (JDK 17)
+ working-directory: hudi
+ run: mvn $MVN_ARGS install -pl
:hudi-common,:hudi-hive-sync,:hudi-io,:hudi-sync-common -am
-Dmaven.test.skip=true -Drat.skip -Dcheckstyle.skip
+ - name: Set up JDK 25
+ uses: actions/setup-java@v5
+ with:
+ java-version: '25'
+ distribution: 'temurin'
+ cache: maven
+ - name: Read Trino version
+ id: trino-version
+ working-directory: trino
+ run: |
+ set -euo pipefail
+ # Ask Maven for the project version rather than grepping the pom:
the first <version> in
+ # trinodb/trino's root pom belongs to the <parent>
(io.airlift:airbase), not to Trino.
+ # Keep the -SNAPSHOT suffix -- master's version is unreleased, so it
only resolves against
+ # the artifacts installed from source in the next step.
+ VERSION=$(mvn -q -N help:evaluate -Dexpression=project.version
-DforceStdout)
+ echo "trino_version=$VERSION" >> "$GITHUB_OUTPUT"
+ echo "Detected Trino version: $VERSION"
+ - name: Install Trino modules from master (JDK 25)
+ working-directory: trino
+ # hudi-trino compiles against these plus their transitive modules
(spi, cache, metastore,
+ # hive-formats, memory-context). They must come from the master
checkout -- resolving from
+ # Maven Central would defeat the point of the drift check.
+ run: mvn $MVN_ARGS install -pl
:trino-hive,:trino-filesystem-manager,:trino-parquet,:trino-plugin-toolkit -am
-DskipTests -Dair.check.skip-all=true
+ - name: Compile hudi-trino against current Trino SPI (JDK 25)
+ working-directory: hudi
+ run: |
+ mvn $MVN_ARGS -Phudi-trino \
+ -Dtrino.version=${{ steps.trino-version.outputs.trino_version }} \
+ -pl hudi-trino compile
+ - name: Open issue on failure
+ if: failure()
Review Comment:
`if: failure()` is job-scoped, so a failed checkout or a broken
trinodb/trino master build also files "hudi-trino SPI drift detected", and if
the failure is at or before `Read Trino version` the title and body render with
an empty version. Give the compile step an `id` and guard on `failure() &&
steps.compile.outcome == 'failure'`.
##########
hudi-trino/README.md:
##########
@@ -0,0 +1,40 @@
+# hudi-trino
+
+Hudi connector for Trino (RFC-105). Published as `org.apache.hudi:hudi-trino`
-- a regular non-shaded JAR. The Trino-side `trino-hudi` plugin module depends
on this artifact and Trino's URLClassLoader isolates the plugin's transitive
deps from the rest of the server, so no shading is required.
+
+## Build
+
+Excluded from default builds. Activate the `hudi-trino` Maven profile:
+
+```
+# tests need Trino test-jars not on Maven Central (see Running tests); skip
them in the default build
+mvn -Phudi-trino -pl hudi-trino install -Dmaven.test.skip=true
+```
+
+Requires JDK 25 (enforced via `maven-enforcer-plugin`).
+
+## Running tests
+
+Tests depend on Trino test-jars (`trino-spi`, `trino-filesystem`,
`trino-hive`, `trino-main` at the `tests` classifier). Trino does not publish
three of those to Maven Central, so the test deps live behind the
`hudi-trino-tests` profile, off by default.
+
+To run the tests:
+
+1. Build the matching Trino version locally so its `*-tests.jar` artifacts
land in your `~/.m2` (see `trino.version` in the root pom for the version to
build).
+2. Activate both profiles:
+
+```
+mvn -Phudi-trino,hudi-trino-tests -pl hudi-trino test
+```
+
+CI keeps `hudi-trino-tests` off so the build resolves cleanly against Maven
Central.
Review Comment:
"CI keeps `hudi-trino-tests` off so the build resolves cleanly against Maven
Central" contradicts the workflow, whose last step runs `mvn
-Phudi-trino,hudi-trino-tests -pl hudi-trino test` after installing the Trino
test-jars from a source checkout of the 481 tag. Update the line to say CI
builds the test-jars from source and does enable the profile.
##########
.github/workflows/hudi_trino_ci.yml:
##########
@@ -0,0 +1,117 @@
+name: Hudi Trino Connector CI
+
+on:
+ push:
+ branches:
+ - master
+ - 'release-*'
+ paths:
+ - 'hudi-trino/**'
+ - '.github/workflows/hudi_trino_ci.yml'
+ # No `paths:` filter here on purpose. test-hudi-trino-plugin is a required
status check
+ # in .asf.yaml, and a path-filtered workflow is never instantiated on PRs
that miss the
+ # filter, leaving the required context permanently pending. Run on every PR
instead and
+ # skip the expensive steps via the detect-trino-changes job below.
+ pull_request:
+ branches:
+ - master
+ - 'release-*'
+ workflow_dispatch:
+
+concurrency:
+ group: hudi-trino-ci-${{ github.ref }}
+ cancel-in-progress: ${{ !contains(github.ref, 'master') &&
!contains(github.ref, 'release-') }}
+
+env:
+ MVN_ARGS: -e -ntp -B -V -Dgpg.skip -Djacoco.skip -Pwarn-log
+
+jobs:
+ changes:
+ name: detect-trino-changes
+ runs-on: ubuntu-latest
+ outputs:
+ trino: ${{ steps.filter.outputs.trino }}
+ steps:
+ - name: Detect hudi-trino changes
+ id: filter
+ env:
+ GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+ REPO: ${{ github.repository }}
+ EVENT: ${{ github.event_name }}
+ PR_NUMBER: ${{ github.event.pull_request.number }}
+ BEFORE_SHA: ${{ github.event.before }}
+ AFTER_SHA: ${{ github.sha }}
+ run: |
+ set -euo pipefail
+ TRINO=false
+ if [ "$EVENT" = "pull_request" ]; then
+ FILES=$(gh api --paginate "repos/$REPO/pulls/$PR_NUMBER/files"
--jq '.[].filename')
+ elif [ "$EVENT" = "push" ]; then
+ FILES=$(gh api "repos/$REPO/compare/$BEFORE_SHA...$AFTER_SHA" --jq
'.files[].filename')
+ else
+ # workflow_dispatch and anything else: always run the full build.
+ FILES=""
+ TRINO=true
+ fi
+ echo "Changed files:"
+ printf '%s\n' "$FILES"
+ while IFS= read -r f; do
+ [ -z "$f" ] && continue
+ case "$f" in
+ hudi-trino/*) TRINO=true ;;
Review Comment:
The filter matches only `hudi-trino/*`, but the module depends on
`hudi-common`, `hudi-io`, `hudi-hive-sync` and `hudi-sync-common`, and the root
`pom.xml` owns `trino.version` plus the dependency pins this module
counteracts, so a change to any of those reports the required check green
without compiling the connector. Should those paths trigger it too, or is
building trinodb/trino from source on every `hudi-common` PR too expensive to
be worth it?
##########
pom.xml:
##########
@@ -87,7 +86,7 @@
<maven-surefire-plugin.version>3.5.4</maven-surefire-plugin.version>
<maven-failsafe-plugin.version>3.5.4</maven-failsafe-plugin.version>
<!-- bump to 3.5.3 to fix MSHADE-461 -->
- <maven-shade-plugin.version>3.5.3</maven-shade-plugin.version>
+ <maven-shade-plugin.version>3.6.2</maven-shade-plugin.version>
Review Comment:
The comment above still says `bump to 3.5.3 to fix MSHADE-461` while the
value is now 3.6.2. hudi-trino is non-shaded and the root declaration has no
executions, so this only changes how the bundle modules shade - was the bump
needed for this migration, or can it go in its own PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
