Copilot commented on code in PR #12388:
URL: https://github.com/apache/gluten/pull/12388#discussion_r3668000437
##########
.github/workflows/velox_backend_x86.yml:
##########
@@ -101,6 +106,80 @@ jobs:
path: ./cpp/build/
if-no-files-found: error
+ # Gate the (expensive) Delta Spark UT suite so per-PR it runs only when the
PR
+ # touches high-signal Delta paths -- the Delta integration code
+ # (backends-velox/src-delta*), the gluten-delta module, or this pipeline's
own
+ # files -- or carries the `run-delta-ci` opt-in label. Changes to general
+ # Velox/core/native code can also affect Delta offload but are touched
+ # constantly, so per-PR they skip it; the nightly full run
(delta_spark_ut.yml
+ # `schedule`) and the opt-in label are the safety nets. This keeps GHA usage
+ # down. NOTE: the label is read from the event that triggered this run, so
add
+ # it before/with a push; labeling an already-finished run needs a new push.
+ delta-changes:
+ runs-on: ubuntu-22.04
+ outputs:
+ run_delta: ${{ steps.filter.outputs.run_delta }}
+ steps:
+ - uses: actions/checkout@v4
+ with:
+ fetch-depth: 0
+ - name: Detect Delta-relevant changes / opt-in label
+ id: filter
+ env:
+ HAS_LABEL: ${{ contains(github.event.pull_request.labels.*.name,
'run-delta-ci') }}
+ BASE_SHA: ${{ github.event.pull_request.base.sha }}
+ HEAD_SHA: ${{ github.event.pull_request.head.sha }}
+ run: |
+ set -euo pipefail
+ # Opt-in label forces the suite even with no Delta-relevant path
change.
+ if [ "$HAS_LABEL" = "true" ]; then
+ echo "run-delta-ci label present -> running Delta suite"
+ echo "run_delta=true" >> "$GITHUB_OUTPUT"; exit 0
+ fi
+ # Fail open if we can't determine the PR range (e.g. a non-PR
trigger):
+ # never silently skip coverage.
+ if [ -z "${BASE_SHA:-}" ] || [ -z "${HEAD_SHA:-}" ]; then
+ echo "no PR base/head sha -> running Delta suite (fail-open)"
+ echo "run_delta=true" >> "$GITHUB_OUTPUT"; exit 0
+ fi
+ BASE=$(git merge-base "$BASE_SHA" "$HEAD_SHA" 2>/dev/null || echo
"$BASE_SHA")
+ echo "diff base=$BASE head=$HEAD_SHA"
+ # Fail open if the diff itself can't be computed (missing objects
after a
+ # force-push race, an unfetched fork head, ...). Piping straight into
+ # `grep -q` inside an `if` would hide that: git's failure leaves
grep with
+ # empty input, so the pipeline exits non-zero exactly as it does for
"no
+ # match" -- and `set -e`/`pipefail` can't help, since a tested
command is
+ # allowed to fail. Capture the diff first so the two cases stay
distinct.
+ if ! CHANGED=$(git diff --name-only "$BASE" "$HEAD_SHA"); then
Review Comment:
The PR diff detection relies on `github.event.pull_request.head.sha`, which
is frequently not present in the local checkout for fork PRs (checkout
typically fetches the PR merge ref in the base repo, not the fork head SHA). In
that case `git diff ... \"$HEAD_SHA\"` will fail and the logic intentionally
‘fail-opens’ to running the expensive Delta suite—effectively disabling the
cost-saving gate for fork PRs. Consider diffing against the checked-out commit
(`github.sha`, which is available) instead of the fork head SHA, e.g. compare
`BASE_SHA` (or merge-base) to the merge commit that Actions checked out. This
keeps the gate effective while retaining the existing fail-open behavior for
genuine diff computation failures.
--
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]
