andygrove commented on code in PR #4950:
URL: https://github.com/apache/datafusion-comet/pull/4950#discussion_r3693413727
##########
.github/workflows/ci.yml:
##########
@@ -263,6 +265,23 @@
spark-full: '4.1.2'
java: 17
+ spark_4_2:
+ name: Spark SQL Tests (Spark 4.2)
+ needs: changes
+ # Main-only by default; PRs need the `run-spark-4.2-tests` label. Spark 4.2
+ # support is still experimental, so keep it off the default PR path.
+ if: |
+ needs.changes.outputs.spark_4_2 == 'true' &&
+ (github.event_name == 'push' ||
+ github.event_name == 'workflow_dispatch' ||
+ (github.event_name == 'pull_request' &&
+ contains(github.event.pull_request.labels.*.name,
'run-spark-4.2-tests')))
+ uses: ./.github/workflows/spark_sql_test_reusable.yml
+ with:
+ spark-short: '4.2'
+ spark-full: '4.2.0'
+ java: 17
+
Review Comment:
Addressed in a289397eb.
Added a top-level `permissions: contents: read` to `ci.yml`, matching the
convention already used by `codeql.yml` and `pyarrow_udf_test.yml`.
The bit that needed care: `ci.yml` calls six reusable workflows, and a
callee inherits the callers permissions and can only narrow them further — so a
blanket read-only default would have broken anything downstream that writes. I
audited all six:
- **`docs.yaml`** commits and pushes the generated site to the `asf-site`
branch (`git push || git push --force`), so it genuinely needs write. The
`docs` job now raises itself to `contents: write`. It already only runs on
push-to-main or `workflow_dispatch`, never on pull requests, so this does not
grant write on PR builds.
- **`pr_build_linux`, `pr_build_macos`, `pr_benchmark_check`,
`spark_sql_test_reusable`, `iceberg_spark_test_reusable`** only build and test
— read is sufficient.
Also checked what else might quietly depend on the old default: no job in
`ci.yml` or any callee uses `GITHUB_TOKEN`, secrets, the `gh` CLI, `git push`
(outside `docs.yaml`), containers or packages. The
`upload-artifact`/`download-artifact` steps are all same-run, which uses the
Actions runtime token rather than `GITHUB_TOKEN`, and none pass `run-id` or
`github-token`, so none need `actions: read`.
One caveat on verification: I could not run `actionlint` locally, because
the project installs it via a curl-to-shell script and I would rather not pipe
a remote script into a shell. I validated that the file parses and that the two
blocks land where intended (top-level `contents: read`, `docs` job `contents:
write`, no other job overriding). The preflight job runs `actionlint` on this
PR, so it will be checked there.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
