mengw15 commented on code in PR #8096: URL: https://github.com/apache/texera/pull/8096#discussion_r3922386051
########## .github/workflows/backport-approval-check.yml: ########## @@ -0,0 +1,334 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Blocks the merge while a `release/*` label on the PR has not been approved by +# that branch's release manager (.github/release-branches.yml). +# +# The point is that a merged PR's labels should be a truthful record. A label +# only nominates a branch, so before this check a manager declined a backport by +# staying silent — and the label stayed on, which read afterwards as "this +# shipped in 1.2" when it had not. Here declining is an action: the manager +# either approves, or removes their label. Since the merge waits for every +# remaining label to be approved, the labels on a merged PR are exactly the +# branches Direct Backport Push then sends the fix to. +# +# The author's job is to ask the manager for that call, not to make it for them. +# Editing labels needs triage access, so outside contributors and Renovate or +# Dependabot PRs cannot remove a label themselves — the failure report says so +# and names who to ask. +# +# `Backport Approvals` is listed in .asf.yaml's required_status_checks, so this +# job must produce a result on EVERY pull request, or a PR that never runs it +# waits forever. That is why it is not conditioned on the title, the labels, or +# the base branch: it always runs and reports success when there is nothing to +# approve. Merge groups are re-evaluated rather than waved through, because a +# queued PR's own checks are no longer consulted once it is queued; a group +# whose PRs cannot be resolved fails closed rather than passing unchecked. Do +# not rename the job — its display name is the required context. + +name: Backport Approval Check + +on: + pull_request: + types: + - opened + - reopened + - synchronize + - labeled + - unlabeled + # Retargeting arrives as `edited`, and the verdict depends on the base + # branch: a backport PR into release/* has nothing to approve, the same + # PR moved onto main does. + - edited + pull_request_review: + types: + - submitted + - dismissed + merge_group: + +# Read-only: this job never writes to the PR, and `pull_request_review` runs +# with the base repository's token even for fork PRs. +permissions: + contents: read + pull-requests: read + +concurrency: + group: backport-approvals-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + backport-approvals: + # Do not rename — this display name is the required status check context + # referenced in .asf.yaml. + name: Backport Approvals + runs-on: ubuntu-latest + steps: + # The base, not the PR head: the head can be arbitrarily old, and neither + # release-branches.yml nor its parser existed before #6941 — a head from + # before that (37 of the open PRs into main as of writing) would fail the + # `run:` step below with a file-not-found and turn this required check red + # with no way for the author to clear it. The base also keeps this reading + # the same config as the two other consumers: backport-auto-label.yml pins + # `base.sha`, and Direct Backport Push reads main after the merge. + # + # The trade-off is that a PR editing release-branches.yml is judged by the + # pre-edit config. Those are `ci:`-typed and carry no `release/*` labels, + # so there is nothing for the stale config to misjudge. + - name: Checkout + uses: actions/checkout@v7 + with: + ref: ${{ github.event.pull_request.base.sha || github.event.merge_group.head_sha }} Review Comment: Verified before taking it, and it holds: #6009 has a `base.sha` dated 2026-07-10, #6436 2026-07-18, #6915 2026-07-26 — all 404 on `release_branches.py`, all still active. Fixed in `4bdfa6638` with your suggestion. Worth naming what this pattern was: I moved off the base last round on a security argument, you moved me back on a staleness argument, and both of us were reasoning about which revision was *trustworthy* rather than which one *exists*. The default branch is the only ref that is always both, and Direct Backport Push was already reading it — the answer was sitting in the sibling the whole time. ########## .github/workflows/backport-approval-check.yml: ########## @@ -0,0 +1,334 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Blocks the merge while a `release/*` label on the PR has not been approved by +# that branch's release manager (.github/release-branches.yml). +# +# The point is that a merged PR's labels should be a truthful record. A label +# only nominates a branch, so before this check a manager declined a backport by +# staying silent — and the label stayed on, which read afterwards as "this +# shipped in 1.2" when it had not. Here declining is an action: the manager +# either approves, or removes their label. Since the merge waits for every +# remaining label to be approved, the labels on a merged PR are exactly the +# branches Direct Backport Push then sends the fix to. +# +# The author's job is to ask the manager for that call, not to make it for them. +# Editing labels needs triage access, so outside contributors and Renovate or +# Dependabot PRs cannot remove a label themselves — the failure report says so +# and names who to ask. +# +# `Backport Approvals` is listed in .asf.yaml's required_status_checks, so this +# job must produce a result on EVERY pull request, or a PR that never runs it +# waits forever. That is why it is not conditioned on the title, the labels, or +# the base branch: it always runs and reports success when there is nothing to +# approve. Merge groups are re-evaluated rather than waved through, because a Review Comment: Taken as suggested in `4bdfa6638` — the reason had gone stale with the ruleset move. ########## .github/workflows/backport-approval-check.yml: ########## @@ -0,0 +1,316 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# Blocks the merge while a `release/*` label on the PR has not been approved by +# that branch's release manager (.github/release-branches.yml). +# +# The point is that a merged PR's labels should be a truthful record. A label +# only nominates a branch, so before this check a manager declined a backport by +# staying silent — and the label stayed on, which read afterwards as "this +# shipped in 1.2" when it had not. Here declining is an action: the manager +# either approves, or removes their label. Since the merge waits for every +# remaining label to be approved, the labels on a merged PR are exactly the +# branches Direct Backport Push then sends the fix to. +# +# The author's job is to ask the manager for that call, not to make it for them. +# Editing labels needs triage access, so outside contributors and Renovate or +# Dependabot PRs cannot remove a label themselves — the failure report says so +# and names who to ask. +# +# `Backport Approvals` is listed in .asf.yaml's required_status_checks, so this +# job must produce a result on EVERY pull request, or a PR that never runs it +# waits forever. That is why it is not conditioned on the title, the labels, or +# the base branch: it always runs and reports success when there is nothing to +# approve. Merge groups are re-evaluated rather than waved through, because a +# queued PR's own checks are no longer consulted once it is queued; a group +# whose PRs cannot be resolved fails closed rather than passing unchecked. Do +# not rename the job — its display name is the required context. + +name: Backport Approval Check + +on: + pull_request: + types: + - opened + - reopened + - synchronize + - labeled + - unlabeled + # Retargeting arrives as `edited`, and the verdict depends on the base + # branch: a backport PR into release/* has nothing to approve, the same + # PR moved onto main does. + - edited + pull_request_review: + types: + - submitted + - dismissed + merge_group: + +# Read-only: this job never writes to the PR, and `pull_request_review` runs +# with the base repository's token even for fork PRs. +permissions: + contents: read + pull-requests: read + +concurrency: + group: backport-approvals-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + backport-approvals: + # Do not rename — this display name is the required status check context + # referenced in .asf.yaml. + name: Backport Approvals + runs-on: ubuntu-latest + steps: + # The PR's own head (or the queued group's), so the config is the one that + # will govern this change once it merges. Checking out the base would not + # be a security boundary anyway: on `pull_request` the workflow definition + # itself comes from the PR. What bounds this is the event — a fork gets a + # read-only token and no secrets, and nothing here writes to the PR. + - name: Checkout + uses: actions/checkout@v7 + with: + ref: ${{ github.event.pull_request.head.sha || github.event.merge_group.head_sha }} + persist-credentials: false + + - name: Read release branches + id: targets + run: | + entries=$(python3 .github/scripts/release_branches.py .github/release-branches.yml) + echo "entries=${entries}" >> "$GITHUB_OUTPUT" + echo "Release branch targets: ${entries}" + + - name: Check release manager approvals + uses: actions/github-script@v9 + env: + ENTRIES: ${{ steps.targets.outputs.entries }} + with: + script: | + const { owner, repo } = context.repo; + const managers = new Map( + JSON.parse(process.env.ENTRIES || "[]").map((e) => [ + e.branch, + e.manager || "", + ]) + ); + + // A merge group carries several PRs and its commits are the squash + // merges, so the number is parsed out of each subject — the same way + // precheck.yml resolves them. + async function pullRequestNumbers() { + if (context.eventName !== "merge_group") { + return [context.payload.pull_request.number]; + } + const mg = context.payload.merge_group; + const { data: comparison } = await github.rest.repos.compareCommits({ + owner, repo, base: mg.base_sha, head: mg.head_sha, + }); + return comparison.commits.map((c) => { + const m = c.commit.message.split("\n", 1)[0].match(/\(#(\d+)\)\s*$/); + if (!m) throw new Error(`no PR number in the message of ${c.sha}`); + return Number(m[1]); + }); + } + + // Latest review state per login. COMMENTED never changes an approval; + // a later CHANGES_REQUESTED or DISMISSED revokes one. listReviews is + // documented to return reviews chronologically, so the last state + // recorded for a login is the one that counts. `null` means the list + // could not be read at all — kept distinct from an empty list, since + // "we did not look" must never be reported as "nobody approved". + async function reviewStates(number) { + try { + const reviews = await github.paginate(github.rest.pulls.listReviews, { + owner, repo, pull_number: number, per_page: 100, + }); + const states = new Map(); + for (const review of reviews) { + const login = review.user?.login; + if (!login || review.state === "COMMENTED") continue; + states.set(login.toLowerCase(), review.state); + } + return states; + } catch (e) { + core.warning( + `listReviews for #${number} failed (${e.status ?? "?"}): ${e.message}` + ); + return null; + } + } + + // Managers gate their own branch and nothing else, so approvals + // compose: an approval from the v1.2 manager alone clears v1.2 and + // leaves v1.3 blocked. + function decide(targets, author, states) { Review Comment: Appreciated. The blocked path is linked in the description alongside the cleared and no-label ones, so the three states are all evidenced rather than asserted. -- 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]
