felipepessoto commented on PR #12781:
URL: https://github.com/apache/gluten/pull/12781#issuecomment-5347525074
### Tested end-to-end before merge
`issue_comment` workflows are always read from the **default branch**, so
`/delta-test` cannot be exercised from this PR's branch here. I ran it in a
fork instead (fork `main` fast-forwarded to this branch, restored afterwards),
on a throwaway PR that touches **only**
`.github/workflows/util/delta-spark-ut/README.md` — a path the Delta filter
deliberately excludes, i.e. exactly the "filter skipped it" case this feature
exists for.
| # | Check | Result |
|---|---|---|
| 1 | PR on a filter-excluded path creates no Delta run | ✅ no run created |
| 2 | `/delta-test-arm` rejected (the `dfe2e66` fix) | ✅ all 5 jobs skipped |
| 3 | `/delta-test` by the PR author authorises | ✅ `delta-test-requested`
succeeded |
| 4 | Acknowledgement comment posted | ✅ posted with run link |
| 5 | Checks out the PR, not `main` | ✅ `refs/pull/2/merge` |
| 6 | Stash restore under `actions: read` | ✅ succeeded, no 403 |
| 7 | Cache save suppressed on comment runs | ✅ `Save Ccache` → **skipped** |
Two notes on the method:
**#6 shows the `actions: read` addition is load-bearing.** The Stash restore
shells out to the artifacts REST API via `run_checked(..., check=True)`, so a
403 aborts the step outright; it completed cleanly instead.
**#7 is valid despite my cancelling the run.** The step is `always() &&
github.event_name != 'issue_comment'`, and `always()` still evaluates on
cancellation — so `skipped` is the guard firing, not a by-product of the
cancel. I cancelled once the native build started, since the remaining ~2.5 h
only re-tests Delta results that the `pull_request` runs here already cover.
Total cost ~10 runner-minutes rather than ~11 job-hours.
The run log also confirms empirically the branch-scoping argument made above:
```
No stash found for keys ccache-centos7-release-default-<hash>-main
or ccache-centos7-release-default-<hash>-main
```
On a comment run both the head and base lookups mung to **`-main`** — the
exact artifact the nightly restores, and (via the shared ccache key) the one
`velox_backend_x86.yml` restores too. An unguarded save would have overwritten
it, which is what the three guards prevent.
The one thing that cannot be tested pre-merge is the workflow file itself
being read from `main` — by construction, that only takes effect once this is
merged.
--
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]
