Repository navigation
Cancel non-needed pull requests CI runs - #16779
Conversation
Merging a pull request, or labelling it "no ci", leaves its queued and in-progress runs consuming shared runner time, although their result can no longer change anything. Cancel the pull_request runs of every open pull request labelled "no ci", and on every push to a release branch also the runs created before the merge of each pull request the pushed commit merged. A run is kept while an open pull request not labelled "no ci" has the same head commit. The workflow runs on every push to a release branch and when a pull request is labelled "no ci". A pull_request run from a fork gets a read-only token, so labelling a fork's pull request skips the job and its runs are cancelled by the next push to a release branch; for the same reason merges are found from the push. The unfinished pull_request runs are listed once per sweep rather than per pull request, to spare the GITHUB_TOKEN rate limit, and pending runs waiting for a concurrency group are cancelled too.
d9e4956 to
74ef77e
Compare
jamesfredley
left a comment
There was a problem hiding this comment.
Not approving. Labelling a pull request no ci can cancel Release Drafter, which this repository keeps running so the autolabeler and the release draft still update.
Backports the 8.0.x change. The pull_request trigger was there so the autolabeler could label pull requests, but the pinned release-drafter 7 entrypoint has no labelling: v7 moved it into the separate release-drafter/release-drafter/autolabeler action, which is not on the ASF approved list. A pull request run therefore applied no labels, and from a fork it could not write the draft either, failing behind continue-on-error on the read-only token. The draft is built from merged pull requests, so a pull request run only rewrote the draft the last push had written. It also shared the release-drafter-<branch> concurrency group with the push runs, so it could cancel a push run still queued or in progress; when the pull request run then failed, or was cancelled by cancel-unneeded-pr-runs.yml, the draft stayed stale until the next push. Drafts are now written only by the pushes that can change them, and the job reads pull requests instead of writing them. AGENTS.md and CONTRIBUTING.md no longer list the Release Drafter as a workflow that the "no ci" label and the draft flag leave running.
jamesfredley
left a comment
There was a problem hiding this comment.
The Release Drafter no longer runs on pull requests, so a no ci label cannot cancel it. A run whose head still belongs to an open pull request is kept. 7.0.x is the right line for this CI change.
matrei
left a comment
There was a problem hiding this comment.
Looks good to me, and it is worth having: merges regularly leave runs going for hours. Of the last 40 merged pull requests, #16727, #16728 and #16729 had runs that finished five to nine hours after the merge, and #16741, #16742 and #16743 kept gradle.yml and the Groovy snapshot canary going for about two more hours.
What I checked:
commits/{sha}/pullsreturns the merged pull request for a commit that is only on a branch below the default: 1529fb6, which is on 7.0.x and not merged up yet, gives #16776 withmerged_atset.ubuntu-slimshipsgh, and the awk program only uses what mawk supports.- The
release-notes.ymlhunks match 8.0.x. The rest of the diff against 8.0.x is its release hold-off step and its newer release-drafter pin.
Neither trigger has run yet. no ci went on at 16:01Z, when the head was d9e4956 and the workflow, then cancel-merged-pr-runs.yml, had only the push trigger, so there is no run of it in the repository. Removing no ci from this pull request and adding it again would exercise the label path before the merge; the push path first runs when this is merged.
| { | ||
| if [[ "$EVENT" == push ]]; then | ||
| gh api "repos/$REPO/commits/$SHA/pulls" \ | ||
| --jq '.[] | select(.merged_at != null) | [.head.repo.full_name, .head.sha, .merged_at] | @tsv' |
There was a problem hiding this comment.
On 7.0.x, 7.1.x and 7.2.x the push after a merge builds without tests, and AGENTS.md says a backport's tests belong on the pull request, labelled run ci. There, the pull request's run is the only test result the change gets on 7.x, so its result can still change something after the merge: a late failure means a follow-up fix. Cancelling it leaves the change untested on that branch.
It hasn't happened yet: every run ci pull request into 7.0.x among the last 40 merges had finished its tests before it was merged. So this is about what the rule does, not about anything it would have cancelled. Keeping the runs of merged pull requests labelled run ci would cover it:
--jq '.[] | select(.merged_at != null and (any(.labels[]; .name == "run ci") | not)) | [.head.repo.full_name, .head.sha, .merged_at] | @tsv'The header comment's "their result can no longer change anything" and the AGENTS.md bullet would then need a sentence on it.
There was a problem hiding this comment.
Done in 45672af with your filter: the sweep leaves out a merged pull request labelled run ci, so its runs finish. The header comment, the AGENTS.md bullet and CONTRIBUTING.md now say why: on a release branch below the default the push after the merge builds without the tests, so the tests the label asked for are the only ones the change gets there. A dry run of the sweep script (with gh run cancel replaced by echo) for the push of #16770's merge into 7.0.x, labelled run ci, no longer lists its head among the unneeded ones, while the push of #16777's merge, unlabelled, still does.
A head commit starting with [run tests] also asks for the tests on a pull request, but the label is the documented way to test a backport, and none of the last 100 merged pull requests used the marker, so I left it out rather than read the head commit of every merged pull request.
| A pull request labelled `no ci` runs no workflow at all, draft or not. Labels are read when a push arrives, so add | ||
| `run ci` (or remove `no ci`) and then push, or mark the draft ready, to get a run. Pushing to a draft still cancels | ||
| the run in flight for it, as every push to a pull request or a branch does. | ||
| * Adding `no ci` to a pull request cancels the runs it has in flight, and merging a pull request cancels the runs |
There was a problem hiding this comment.
There was a problem hiding this comment.
Changed to "merging a pull request into a release branch" in 45672af.
On a release branch below the default branch the push after a merge builds without the tests, and a backport's tests belong on the pull request, labelled "run ci". There the pull request's runs are the only test result the change gets, so their result still matters after the merge: a late failure calls for a follow-up fix. The sweep no longer cancels the runs of a merged pull request labelled "run ci". CONTRIBUTING.md now says that merging cancels runs only on a merge into a release branch, since the push trigger covers only release branches.
Merging a pull request, or labelling it
no ci, leaves its queued and in-progress runs consuming shared runner time, although, except for the tests arun cilabel asked for, their result can no longer change anything. TheCancel Unneeded PR Runsworkflow cancels thepull_requestruns of every open pull request labelledno ci, and on every push to a release branch also the runs created before the merge of each pull request the pushed commit merged. A run is kept while an open pull request not labelledno cihas the same head commit, and the runs of a merged pull request labelledrun ciare kept too: on a release branch below the default, the push after the merge builds without the tests, so the pull request's tests are the only ones the change gets there.The workflow runs on every push to a release branch and when a pull request is labelled
no ci. Apull_requestrun from a fork gets a read-only token, so labelling a fork's pull request skips the job and its runs are cancelled by the next push to a release branch; for the same reason merges are found from the push. The unfinishedpull_requestruns are listed once per sweep rather than per pull request, to spare theGITHUB_TOKENrate limit, and pending runs waiting for a concurrency group are cancelled too.It also backports the 8.0.x removal of the Release Drafter's
pull_requesttrigger. The pinned release-drafter 7 action cannot label pull requests, and the draft is built from merged pull requests, so a pull request run only rewrote the draft, failed on a fork's read-only token, and, sharing the branch's concurrency group, could cancel the push run that writes the draft.See apache/datafusion#25088