ephraimbuddy commented on code in PR #75420:
URL: https://github.com/apache/airflow/pull/75420#discussion_r4351190887
##########
.github/workflows/ci-amd.yml:
##########
@@ -66,12 +65,88 @@ env:
VERBOSE: "true"
concurrency:
- group: ci-amd-${{ github.event.pull_request.number || github.ref }}
- cancel-in-progress: true
+ # Push runs have a group of their own per branch: a newer push cancels the
older push run (the
+ # newer run then also tests the cancelled run's changes, see
`check-run-needed`), and a push never
+ # cancels a full run of the branch.
+ group: >-
+ ci-amd-${{ github.event.pull_request.number || github.ref }}${{
+ github.event_name == 'push' && '-push' || '' }}
+ # A scheduled canary queues behind a run in progress instead of cancelling
it: it may well be a
+ # manual run of the same commit, which the canary would otherwise cancel and
then skip itself.
+ cancel-in-progress: ${{ github.event_name != 'schedule' }}
jobs:
+ check-run-needed:
+ name: "Check if the run is needed"
+ # A scheduled canary is not needed when the last completed full run of
this workflow on the
+ # branch tested the same commit and succeeded: it would only repeat that
run. A push run tests
+ # the changes since the last successful run of this workflow on the
branch, found here.
+ # This job runs for every event, with no steps for the others: every job
of the run depends on
+ # it, and a skipped job here would skip all of them, whatever their own
`if:` says.
Review Comment:
This isn't quite how `needs` works: dependents of a skipped job are skipped
only when their `if:` has no status function. Only `build-info` needs this job,
so it could be skipped for `pull_request`, with `build-info` using
`!cancelled() && needs.check-run-needed.result != 'failure' &&
needs.check-run-needed.outputs.run-needed != 'false'`.
As it is, every PR run waits for an extra runner before `build-info` starts.
---
Drafted-by: Claude Code (Opus 5.5); reviewed by @ephraimbuddy before posting
##########
.github/workflows/ci-amd.yml:
##########
@@ -1261,6 +1348,7 @@ jobs:
always() &&
!cancelled() &&
github.event_name == 'schedule' &&
+ needs.build-info.result != 'skipped' &&
Review Comment:
If `check-run-needed` fails (e.g. a transient `gh api` error under `set -euo
pipefail`), `build-info` is skipped and this condition treats it as a no-op
canary, so a broken `main` canary never posts to Slack.
Could the lookup fail open (`run-needed=true`), and this gate on
`needs.check-run-needed.outputs.run-needed != 'false'` instead (with
`check-run-needed` added to `needs`)?
---
Drafted-by: Claude Code (Opus 5.5); reviewed by @ephraimbuddy before posting
--
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]
