wombatu-kun commented on code in PR #19410:
URL: https://github.com/apache/hudi/pull/19410#discussion_r3679368478
##########
.github/workflows/bot.yml:
##########
@@ -87,8 +87,11 @@ jobs:
#
# Stage the copy outside the workspace. `mvn apache-rat:check` in the
RAT check step scans
# the filesystem from the repository root, so a copy of the tree left
inside the workspace
- # gets scanned too, and every root-anchored exclude in the root pom
(such as
- # `hudi-trino-plugin/**`) silently stops applying to it.
+ # gets scanned too, and every root-anchored exclude in the root pom
(such as `.github/**`)
+ # silently stops applying to it.
+ #
+ # hudi-trino needs no RAT exclude: its files carry AL headers that RAT
accepts, and its
+ # Trino-style license checks are handled by airlift.
Review Comment:
The module declares no airlift or airbase license plugin, so nothing in this
repo performs the Trino-style check this claims. Drop the clause or point it at
whatever actually runs it.
##########
scripts/release/deploy_staging_jars_java25.sh:
##########
@@ -0,0 +1,96 @@
+#!/bin/bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You 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.
+#
+
+##
+## Variables with defaults (if not overwritten by environment)
+##
+MVN=${MVN:-mvn}
+# fail immediately
+set -o errexit
+set -o nounset
+
+CURR_DIR=$(pwd)
+if [ ! -d "$CURR_DIR/packaging" ] ; then
+ echo "You have to call the script from the repository root dir that contains
'packaging/'"
+ exit 1
+fi
+
+# Validate Java version (hudi-trino requires Java 25)
+EXPECTED_JAVA_VERSION=25
+JAVA_VERSION_OUTPUT=$("${JAVA_HOME:+$JAVA_HOME/bin/}java" -version 2>&1)
+JAVA_MAJOR_VERSION=$(echo "$JAVA_VERSION_OUTPUT" | awk -F[\".] '/version/
{print ($2 == "1" ? $3 : $2); exit}')
+if [ "$JAVA_MAJOR_VERSION" != "$EXPECTED_JAVA_VERSION" ]; then
+ echo "Error: Java $EXPECTED_JAVA_VERSION is required for this script, but
found:"
+ echo "$JAVA_VERSION_OUTPUT"
+ echo "Set JAVA_HOME to a Java $EXPECTED_JAVA_VERSION installation and retry."
+ exit 1
+fi
+
+if [ "$#" -gt "1" ]; then
+ echo "Only accept 0 or 1 argument. Use -h to see examples."
+ exit 1
+fi
+
+declare -a ALL_VERSION_OPTS=(
+# org.apache.hudi:hudi-trino (RFC-105), a plain library jar compiled with JDK
25.
+# No -am: Lombok cannot run on JDK 25, so upstream Hudi modules (hudi-common,
+# hudi-hive-sync, hudi-io:shaded, hudi-sync-common) must already be in the
local m2
+# from a prior deploy_staging_jars.sh (JDK 11) run.
+"-Phudi-trino -pl hudi-trino"
+)
+printf -v joined "'%s'\n" "${ALL_VERSION_OPTS[@]}"
+
+if [ "${1:-}" == "-h" ]; then
+ echo "
+Usage: $(basename "$0") [OPTIONS]
+
+Options:
+<version option> One of the version options below
+${joined}
+-h, --help
+"
+ exit 0
+fi
+
+VERSION_OPT=${1:-}
+valid_version_opt=false
+for v in "${ALL_VERSION_OPTS[@]}"; do
+ [[ $VERSION_OPT == "$v" ]] && valid_version_opt=true
+done
+
+if [ "$valid_version_opt" = true ]; then
+ # run deploy for only specified version option
+ ALL_VERSION_OPTS=("$VERSION_OPT")
+elif [ "$#" == "1" ]; then
+ echo "Version option $VERSION_OPT is invalid. Use -h to see examples."
+ exit 1
+fi
+
+COMMON_OPTIONS="-DdeployArtifacts=true -DskipTests
-DretryFailedDeploymentCount=10"
Review Comment:
`-DskipTests` skips surefire but not `testCompile`, and hudi-trino's test
sources need the `hudi-trino-tests` profile deps (`hudi-java-client` plus Trino
test-jars that are not on Maven Central), so this pass fails at test-compile.
Use `-Dmaven.test.skip=true` instead, the way `hudi_trino_ci.yml` builds the
connector.
##########
hudi-trino/pom.xml:
##########
@@ -595,6 +595,14 @@
<annotationProcessorPaths combine.self="override"/>
</configuration>
</plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <configuration>
+ <!-- The parent release profile pins javadoc source to 11;
records need 25. -->
+ <source>${hudi.trino.java.version}</source>
Review Comment:
The parent's `failOnError=false` is still inherited here, so the next
javadoc break in this module is as silent as the source level was. Worth
setting `failOnError` to true for hudi-trino, or is the javadoc not clean
enough for that yet?
##########
hudi-trino/pom.xml:
##########
@@ -595,6 +595,14 @@
<annotationProcessorPaths combine.self="override"/>
</configuration>
</plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <configuration>
+ <!-- The parent release profile pins javadoc source to 11;
records need 25. -->
Review Comment:
Records are supported from source 16, not 25; the reason to use 25 here is
matching the module's compile release. Reword to say the javadoc source follows
`hudi.trino.java.version`.
##########
scripts/release/deploy_staging_jars_java25.sh:
##########
@@ -0,0 +1,96 @@
+#!/bin/bash
+
+#
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You 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.
+#
+
+##
+## Variables with defaults (if not overwritten by environment)
+##
+MVN=${MVN:-mvn}
+# fail immediately
+set -o errexit
+set -o nounset
+
+CURR_DIR=$(pwd)
+if [ ! -d "$CURR_DIR/packaging" ] ; then
+ echo "You have to call the script from the repository root dir that contains
'packaging/'"
+ exit 1
+fi
+
+# Validate Java version (hudi-trino requires Java 25)
+EXPECTED_JAVA_VERSION=25
+JAVA_VERSION_OUTPUT=$("${JAVA_HOME:+$JAVA_HOME/bin/}java" -version 2>&1)
+JAVA_MAJOR_VERSION=$(echo "$JAVA_VERSION_OUTPUT" | awk -F[\".] '/version/
{print ($2 == "1" ? $3 : $2); exit}')
+if [ "$JAVA_MAJOR_VERSION" != "$EXPECTED_JAVA_VERSION" ]; then
+ echo "Error: Java $EXPECTED_JAVA_VERSION is required for this script, but
found:"
+ echo "$JAVA_VERSION_OUTPUT"
+ echo "Set JAVA_HOME to a Java $EXPECTED_JAVA_VERSION installation and retry."
+ exit 1
+fi
+
+if [ "$#" -gt "1" ]; then
+ echo "Only accept 0 or 1 argument. Use -h to see examples."
+ exit 1
+fi
+
+declare -a ALL_VERSION_OPTS=(
+# org.apache.hudi:hudi-trino (RFC-105), a plain library jar compiled with JDK
25.
+# No -am: Lombok cannot run on JDK 25, so upstream Hudi modules (hudi-common,
+# hudi-hive-sync, hudi-io:shaded, hudi-sync-common) must already be in the
local m2
+# from a prior deploy_staging_jars.sh (JDK 11) run.
+"-Phudi-trino -pl hudi-trino"
+)
+printf -v joined "'%s'\n" "${ALL_VERSION_OPTS[@]}"
+
+if [ "${1:-}" == "-h" ]; then
+ echo "
+Usage: $(basename "$0") [OPTIONS]
+
+Options:
+<version option> One of the version options below
+${joined}
+-h, --help
+"
+ exit 0
+fi
+
+VERSION_OPT=${1:-}
+valid_version_opt=false
+for v in "${ALL_VERSION_OPTS[@]}"; do
+ [[ $VERSION_OPT == "$v" ]] && valid_version_opt=true
+done
+
+if [ "$valid_version_opt" = true ]; then
+ # run deploy for only specified version option
+ ALL_VERSION_OPTS=("$VERSION_OPT")
+elif [ "$#" == "1" ]; then
+ echo "Version option $VERSION_OPT is invalid. Use -h to see examples."
+ exit 1
+fi
+
+COMMON_OPTIONS="-DdeployArtifacts=true -DskipTests
-DretryFailedDeploymentCount=10"
+for v in "${ALL_VERSION_OPTS[@]}"
+do
+ echo "Cleaning everything before any deployment"
+ $MVN clean $COMMON_OPTIONS ${v}
+ echo "Building with options ${v}"
+ $MVN install $COMMON_OPTIONS ${v}
Review Comment:
With no `-am` in the version option, the separate `install` pass rebuilds
exactly what `deploy` builds, and `${v%-am}` strips a suffix that is never
present. Dropping both leaves `mvn clean deploy`, which does the same work once
- nit.
##########
hudi-trino/pom.xml:
##########
@@ -595,6 +595,14 @@
<annotationProcessorPaths combine.self="override"/>
</configuration>
</plugin>
+ <plugin>
+ <groupId>org.apache.maven.plugins</groupId>
+ <artifactId>maven-javadoc-plugin</artifactId>
+ <configuration>
+ <!-- The parent release profile pins javadoc source to 11;
records need 25. -->
+ <source>${hudi.trino.java.version}</source>
Review Comment:
Child plugin configuration wins over the parent's profile-injected
configuration in the Maven model merge, so `source` resolves to 25 here while
the parent's `doclint` and `failOnError` still merge in. The residual risk is
`failOnError=false`, not the source level.
--
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]
