Copilot commented on code in PR #10863:
URL: https://github.com/apache/gravitino/pull/10863#discussion_r3136411962
##########
.github/workflows/gvfs-fuse-build-test.yml:
##########
@@ -78,7 +78,7 @@ jobs:
- name: Integration test
run: |
- ./gradlew build -PskipWeb=true -x :clients:client-python:build -x
test -x web
+ ./gradlew build -PskipWeb=true -x :clients:client-python:build -x
test -x web -PskipTrinoConnector=true
./gradlew compileDistribution -PskipWeb=true -x
:clients:client-python:build -x test -x web
Review Comment:
`-PskipTrinoConnector=true` is only applied to the first `./gradlew build`
invocation. The subsequent `./gradlew compileDistribution ...` call will still
include/configure the `trino-connector` subprojects (and may trigger JDK 24
toolchain resolution/downloads), which defeats the purpose of introducing the
skip flag for this workflow. Apply the same `-PskipTrinoConnector=true`
property to the `compileDistribution` invocation as well (and any other Gradle
invocations in this job that should not include Trino).
```suggestion
./gradlew compileDistribution -PskipWeb=true -x
:clients:client-python:build -x test -x web -PskipTrinoConnector=true
```
##########
.github/actions/setup-java-toolchains/action.yml:
##########
@@ -0,0 +1,31 @@
+name: 'Setup Java Toolchains'
+description: 'Install a primary JDK and JDK 24 (for Trino), register JDK 24
with Gradle toolchain'
+
+inputs:
+ java-version:
+ description: 'Primary Java version'
+ default: '17'
+ cache:
+ description: 'Gradle cache setting'
+ default: 'gradle'
+
+runs:
+ using: composite
+ steps:
+ - uses: actions/setup-java@v4
+ id: setup-jdk24
+ with:
+ java-version: 24
+ distribution: temurin
+
+ - uses: actions/setup-java@v4
+ with:
+ java-version: ${{ inputs.java-version }}
+ distribution: temurin
+ cache: ${{ inputs.cache }}
+
+ - name: Register JDK 24 with Gradle toolchain
+ shell: bash
+ run: |
+ mkdir -p ~/.gradle
+ echo "org.gradle.java.installations.paths=${{
steps.setup-jdk24.outputs.path }}" >> ~/.gradle/gradle.properties
Review Comment:
The composite action appends `org.gradle.java.installations.paths=...` to
`~/.gradle/gradle.properties` using `>>`. If the action is ever invoked more
than once in a job, this will create duplicate keys, which is harder to reason
about and can lead to surprising precedence issues. Prefer making this step
idempotent (e.g., overwrite the property, or remove any existing
`org.gradle.java.installations.paths` entries before writing).
```suggestion
GRADLE_PROPERTIES=~/.gradle/gradle.properties
TMP_GRADLE_PROPERTIES="$(mktemp)"
if [ -f "${GRADLE_PROPERTIES}" ]; then
awk '!/^org\.gradle\.java\.installations\.paths=/'
"${GRADLE_PROPERTIES}" > "${TMP_GRADLE_PROPERTIES}"
fi
echo "org.gradle.java.installations.paths=${{
steps.setup-jdk24.outputs.path }}" >> "${TMP_GRADLE_PROPERTIES}"
mv "${TMP_GRADLE_PROPERTIES}" "${GRADLE_PROPERTIES}"
```
--
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]
