You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "ajantha-bhat (via GitHub)" <gi...@apache.org> on 2026/06/24 08:33:59 UTC
[PR] CI: Select Spark PR matrix by changed version [iceberg]
ajantha-bhat opened a new pull request, #16946:
URL: https://github.com/apache/iceberg/pull/16946
depends on https://github.com/apache/iceberg/pull/16945
### Summary
This updates Spark CI to build an incremental PR matrix based on the changed Spark version path.
For ordinary PRs, Spark CI now uses the JVM plan from the earlier Java CI PR and then narrows the Spark matrix when changes are scoped to a known Spark version path:
- `spark/v3.5/**` runs Spark 3.5 only
- `spark/v4.0/**` runs Spark 4.0 only
- `spark/v4.1/**` runs Spark 4.1 only
- unknown Spark version paths, unknown Spark paths, missing changed-file detection, `full-ci`, and push/main/release/tag runs fall back to the full Spark matrix
Known Spark versions are read from `knownSparkVersions` in `gradle.properties`, so adding/removing Spark versions does not require hard-coding them in the planner.
This keeps the PR path conservative: stale or unknown planner inputs run more CI, not less.
### Testing
- `actionlint .github/workflows/spark-ci.yml`
- `bash -n .github/scripts/ci-plan-common.sh .github/scripts/plan-spark-ci.sh`
- YAML parse for `jvm-ci-plan.yml` and `spark-ci.yml`
- `git diff --check`
- Local planner checks:
- `spark/v4.1/**` -> Spark 4.1 only
- `spark/v3.5/**` -> Spark 3.5 only
- unknown Spark version/path -> all Spark versions
- `full-ci` / push -> all Spark versions with the full JVM matrix
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3584346520
##########
.github/workflows/spark-ci.yml:
##########
@@ -83,21 +99,12 @@ jobs:
matrix:
jvm: [17, 21]
event_name: ['${{ github.event_name }}']
- spark: ['3.5', '4.0', '4.1']
- scala: ['2.12', '2.13']
+ spark: ${{ fromJson(needs.plan.outputs.spark_versions) }}
+ scala: ${{ fromJson(needs.plan.outputs.scala_versions) }}
# Split iceberg-spark (core) from iceberg-spark-extensions/-runtime
# so they run as concurrent jobs rather than serially in one job.
tests: [core, extensions]
- exclude:
- # Spark 3.5 is the first version not failing on Java 21 (https://issues.apache.org/jira/browse/SPARK-42369)
- # Full Java 21 support is coming in Spark 4 (https://issues.apache.org/jira/browse/SPARK-43831)
- - spark: '4.0'
- scala: '2.12'
- - spark: '4.1'
- scala: '2.12'
- # Pull requests run the baseline JDK only.
- - event_name: pull_request
- jvm: 21
+ exclude: ${{ fromJson(needs.plan.outputs.matrix_excludes) }}
Review Comment:
Do we still need this exclude since we generate spark/scala versions at runtime?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-5481070218
> I think paths is better than paths-ignore
yea i agree, but thats a separate problem. For this one, I'm opting to keep changes to the minimal to support multiple spark versions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3810588194
##########
.github/scripts/plan-spark-ci.sh:
##########
@@ -0,0 +1,105 @@
+#!/usr/bin/env 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.
+#
+
+set -euo pipefail
+
+known_spark_versions="$(
Review Comment:
updated.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-6163454782
Thanks @anuragmantri for the suggestion. Yesterday I was in a CoC ASF infra talk and we all agree on how important it is to save GH resources. I like your suggestion and updated the PR as per that.
@kevinjqliu, @manuzhang: Can you check the updated PR again?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3575866742
##########
.github/workflows/spark-ci.yml:
##########
Review Comment:
I think we can remove these comments now.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3471414995
##########
.github/workflows/flink-ci.yml:
##########
@@ -98,7 +97,13 @@ jobs:
# Read-only: java-ci's build-checks (17) is the global canonical writer.
cache-read-only: true
- run: echo -e "$(ip addr show eth0 | grep "inet\b" | awk '{print $2}' | cut -d/ -f1)\t$(hostname -f) $(hostname -s)" | sudo tee -a /etc/hosts
- - run: ./gradlew -DsparkVersions= -DkafkaVersions= -DflinkVersions=${{ matrix.flink }} :iceberg-flink:iceberg-flink-${{ matrix.flink }}:check :iceberg-flink:iceberg-flink-runtime-${{ matrix.flink }}:check -Pquick=true -x javadoc -DtestParallelism=auto
+ - env:
Review Comment:
do we need to change flink ci in this 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3580902630
##########
.github/workflows/spark-ci.yml:
##########
Review Comment:
removed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3580888533
##########
.github/workflows/spark-ci.yml:
##########
Review Comment:
Added new `unsupportedSparkScalaVersions`. So, all exist in one place now. Is this what you had in mind?
Also thought about `supportedSparkScalaVersions`, that becomes a bigger change.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-5322360657
@anuragmantri As I understand it, it's not urgent currently and it's not easy to maintain readability as @kevinjqliu pointed out.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-5373398188
@kevinjqliu: Could you please take another look?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r4330675912
##########
.github/workflows/spark-ci.yml:
##########
@@ -72,7 +72,41 @@ concurrency:
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
jobs:
+ plan:
+ runs-on: ubuntu-24.04
+ outputs:
+ spark: ${{ steps.plan.outputs.spark }}
+ steps:
+ - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+ with:
+ fetch-depth: 2
+ persist-credentials: false
+ - id: plan
+ shell: bash
+ env:
+ EVENT_NAME: ${{ github.event_name }}
+ run: |
+ spark='["3.5","4.0","4.1","4.2"]'
Review Comment:
i guess we have to in some shape or form 🤷
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3575836730
##########
.github/workflows/spark-ci.yml:
##########
@@ -73,7 +73,21 @@ concurrency:
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
jobs:
+ plan:
+ runs-on: ubuntu-24.04
+ outputs:
+ spark_versions: ${{ steps.plan.outputs.spark_versions }}
+ steps:
+ - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
Review Comment:
We are already at `actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0`.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3575871396
##########
.github/workflows/spark-ci.yml:
##########
Review Comment:
My only concern is mixing generated Spark versions with these static configurations. How are we going to maintain them? Is it possible to define them in gradle file like `knowSparkVersions`?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-4974980652
@kevinjqliu and @Baunsgaard : Do you guys have any comments for this 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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-5322868691
I was on holiday. I am back now.
I gave another shot at it as @kevinjqliu suggested.
Please take a look.
The planner now only controls the Spark version axis:
- for PRs, it narrows to one Spark version only when every changed file is under the same known `spark/v<version>/` path
- mixed Spark versions, unknown Spark versions, workflow/script/build changes, non-version Spark paths, missing changed-file detection, and non-PR runs all use the full Spark matrix
I also removed the shared helper and dropped the generated Scala/exclude handling. Spark/Scala compatibility and the explicit PR JDK behavior now stay visible in the workflow matrix, so the PR is only about Spark-version selection.
I validated the planner cases locally, including same-version Spark changes, mixed Spark versions, unknown paths, workflow changes, non-PR runs, and this PR’s own diff.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3471411241
##########
.github/workflows/flink-ci.yml:
##########
@@ -73,15 +73,14 @@ concurrency:
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
jobs:
-
Review Comment:
is this intentional?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3575822510
##########
.github/scripts/plan-spark-ci.sh:
##########
@@ -0,0 +1,82 @@
+#!/usr/bin/env 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.
+#
+
+set -euo pipefail
+
+source .github/scripts/ci-plan-common.sh
+
+known_spark_versions="$(gradle_property knownSparkVersions)"
+IFS=',' read -r -a all_spark_versions <<< "${known_spark_versions}"
Review Comment:
Could we validate that `all_spark_versions` is non-empty before publishing the output? If `knownSparkVersions` is missing or empty, the fallback paths still produce `[]`, so no Spark test combinations can run.
It would also be safer to compute the JSON in a standalone assignment before writing to `GITHUB_OUTPUT`, ensuring `set -euo pipefail` propagates any planner or `jq` failure.
*Disclosure: I used LLM to help review and draft comments.*
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "anuragmantri (via GitHub)" <gi...@apache.org>.
anuragmantri commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-5320423387
@ajantha-bhat. A gentle ping here. Are you still able to continue working on this?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-5470580645
I took a look at how best to break this down. Since this is an optimization, I think its best if the solution itself adds minimal burden on readability and maintainability -- otherwise we should just keep it as is.
I paired with codex and came up with a solution that doesn't require any scripting, it uses github workflow primitives instead. Please take a look at let me know what you think.
https://github.com/apache/iceberg/compare/main...kevinjqliu:iceberg:kevinjqliu/codex-spark-version-ci
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-5943284386
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r4268355729
##########
.github/workflows/spark-ci.yml:
##########
@@ -72,7 +72,41 @@ concurrency:
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
jobs:
+ plan:
+ runs-on: ubuntu-24.04
+ outputs:
+ spark: ${{ steps.plan.outputs.spark }}
+ steps:
+ - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+ with:
+ fetch-depth: 2
+ persist-credentials: false
+ - id: plan
+ shell: bash
+ env:
+ EVENT_NAME: ${{ github.event_name }}
+ run: |
+ spark='["3.5","4.0","4.1","4.2"]'
Review Comment:
I'm still concerned that we need to maintain supported Spark versions in another place.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-5412072941
ping @kevinjqliu
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3551955407
##########
.github/workflows/spark-ci.yml:
##########
@@ -114,13 +128,17 @@ jobs:
cache-read-only: true
- run: echo -e "$(ip addr show eth0 | grep "inet\b" | awk '{print $2}' | cut -d/ -f1)\t$(hostname -f) $(hostname -s)" | sudo tee -a /etc/hosts
- name: Run tests
+ env:
+ SPARK_VERSION: ${{ matrix.spark }}
+ SCALA_VERSION: ${{ matrix.scala }}
+ TEST_GROUP: ${{ matrix.tests }}
Review Comment:
Storing as env variable now. Because if I directly use matrix values like {{matrix.spark}} it is unsafe pattern now. Since matrix values are now fetched from planner script.
Error from zizmor
`may expand into attacker-controllable code`
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-5322884371
Other than this, If you have some other simpler solution for this, feel free to open a PR for the same.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3580900630
##########
.github/workflows/spark-ci.yml:
##########
@@ -73,7 +73,21 @@ concurrency:
cancel-in-progress: ${{ github.event_name == 'pull_request' }}
jobs:
+ plan:
+ runs-on: ubuntu-24.04
+ outputs:
+ spark_versions: ${{ steps.plan.outputs.spark_versions }}
+ steps:
+ - uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
Review Comment:
good catch. PR was open for too long!
##########
.github/scripts/plan-spark-ci.sh:
##########
@@ -0,0 +1,82 @@
+#!/usr/bin/env 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.
+#
+
+set -euo pipefail
+
+source .github/scripts/ci-plan-common.sh
+
+known_spark_versions="$(gradle_property knownSparkVersions)"
+IFS=',' read -r -a all_spark_versions <<< "${known_spark_versions}"
Review Comment:
added.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3584346520
##########
.github/workflows/spark-ci.yml:
##########
@@ -83,21 +99,12 @@ jobs:
matrix:
jvm: [17, 21]
event_name: ['${{ github.event_name }}']
- spark: ['3.5', '4.0', '4.1']
- scala: ['2.12', '2.13']
+ spark: ${{ fromJson(needs.plan.outputs.spark_versions) }}
+ scala: ${{ fromJson(needs.plan.outputs.scala_versions) }}
# Split iceberg-spark (core) from iceberg-spark-extensions/-runtime
# so they run as concurrent jobs rather than serially in one job.
tests: [core, extensions]
- exclude:
- # Spark 3.5 is the first version not failing on Java 21 (https://issues.apache.org/jira/browse/SPARK-42369)
- # Full Java 21 support is coming in Spark 4 (https://issues.apache.org/jira/browse/SPARK-43831)
- - spark: '4.0'
- scala: '2.12'
- - spark: '4.1'
- scala: '2.12'
- # Pull requests run the baseline JDK only.
- - event_name: pull_request
- jvm: 21
+ exclude: ${{ fromJson(needs.plan.outputs.matrix_excludes) }}
Review Comment:
Do we still need this exclude since we generate spark/scala versions at runtime?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-5487303372
> > I think paths is better than paths-ignore
>
> yea i agree, but thats a separate problem. For this one, I'm opting to keep changes to the minimal to support multiple spark versions.
My main point is that with `paths-ignore`, it's getting harder to maintain, since we need to update the paths to ignore when a Spark version is added or removed.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-5299400967
This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3800717681
##########
.github/scripts/plan-spark-ci.sh:
##########
@@ -0,0 +1,105 @@
+#!/usr/bin/env 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.
+#
+
+set -euo pipefail
+
+known_spark_versions="$(
Review Comment:
Please add some comments on where the known spark versions are defined.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "ajantha-bhat (via GitHub)" <gi...@apache.org>.
ajantha-bhat commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3475008441
##########
.github/workflows/flink-ci.yml:
##########
@@ -98,7 +97,13 @@ jobs:
# Read-only: java-ci's build-checks (17) is the global canonical writer.
cache-read-only: true
- run: echo -e "$(ip addr show eth0 | grep "inet\b" | awk '{print $2}' | cut -d/ -f1)\t$(hostname -f) $(hostname -s)" | sudo tee -a /etc/hosts
- - run: ./gradlew -DsparkVersions= -DkafkaVersions= -DflinkVersions=${{ matrix.flink }} :iceberg-flink:iceberg-flink-${{ matrix.flink }}:check :iceberg-flink:iceberg-flink-runtime-${{ matrix.flink }}:check -Pquick=true -x javadoc -DtestParallelism=auto
+ - env:
Review Comment:
It is from the dependent PR as mentioned in the description. It will go away after rebase.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3575871396
##########
.github/workflows/spark-ci.yml:
##########
Review Comment:
My only concern is mixing generated Spark versions with these static configurations. How are we going to maintain them? Is it possible to exclude it in gradle file like `knowSparkVersions`?
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on code in PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#discussion_r3584406163
##########
.github/workflows/spark-ci.yml:
##########
@@ -83,21 +99,12 @@ jobs:
matrix:
jvm: [17, 21]
event_name: ['${{ github.event_name }}']
- spark: ['3.5', '4.0', '4.1']
- scala: ['2.12', '2.13']
+ spark: ${{ fromJson(needs.plan.outputs.spark_versions) }}
+ scala: ${{ fromJson(needs.plan.outputs.scala_versions) }}
# Split iceberg-spark (core) from iceberg-spark-extensions/-runtime
# so they run as concurrent jobs rather than serially in one job.
tests: [core, extensions]
- exclude:
- # Spark 3.5 is the first version not failing on Java 21 (https://issues.apache.org/jira/browse/SPARK-42369)
- # Full Java 21 support is coming in Spark 4 (https://issues.apache.org/jira/browse/SPARK-43831)
- - spark: '4.0'
- scala: '2.12'
- - spark: '4.1'
- scala: '2.12'
- # Pull requests run the baseline JDK only.
- - event_name: pull_request
- jvm: 21
+ exclude: ${{ fromJson(needs.plan.outputs.matrix_excludes) }}
Review Comment:
Sorry, this kind of revert previous idea of keeping running jvm 17 on PR explicitly. I think the previous version is better.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-5498192817
> My main point is that with paths-ignore, it's getting harder to maintain, since we need to update it when a Spark version is added or removed.
agreed! i need to revive that PR 😄 I think the main point is that these workflows are failed open: we can bypass other spark versions IFF the PR contains only changes to that 1 specific spark version. otherwise, we run all.
So we treat it as an optimization and still preserve correctness
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "manuzhang (via GitHub)" <gi...@apache.org>.
manuzhang commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-5473336279
@kevinjqliu for that approach, I think `paths` is better than `paths-ignore`. For example, `spark-4.1-ci.yml` only needs to include `spark/v4.1` and doesn't care about what other versions are under `spark`. I remember you have another PR to refactor it.
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] CI: Select Spark PR matrix by changed version [iceberg]
Posted by "kevinjqliu (via GitHub)" <gi...@apache.org>.
kevinjqliu commented on PR #16946:
URL: https://github.com/apache/iceberg/pull/16946#issuecomment-6302321412
> What do you all think?
when i see this, its a bit concerning to parse out the logic
> changed=$(git diff --name-only HEAD^1 HEAD |
> awk -F/ '$1 == "spark" && $2 ~ /^v/ && NF > 2 {print substr($2, 2); next} {print "*"}' | sort -u)
have yall given this a look? https://github.com/apache/iceberg/compare/main...kevinjqliu:iceberg:kevinjqliu/codex-spark-version-ci
--
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: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org