comphead commented on code in PR #7377: URL: https://github.com/apache/datafusion-comet/pull/7377#discussion_r4460103397
########## .github/workflows/close_high_volume_prs.yml: ########## @@ -0,0 +1,183 @@ +# 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. + +# Closes pull requests from accounts that have a very large number of open +# pull requests spread across many repositories, which is the signature of +# automated bulk submission. GitHub's per-repository creation cap (.asf.yaml +# github.pull_requests.creation_cap) limits how many such PRs an account can +# have open here, but cannot see activity in other repositories. +# +# The check runs only for authors without write access: users with write access +# and apache org members are always skipped. Having a PR merged here does not +# exempt an author, since merged PRs have come from automated accounts too. Use +# HIGH_VOLUME_ALLOWLIST for accounts that should never be closed. +# +# This workflow never checks out or runs code from the pull request, which is +# what makes pull_request_target safe to use here. +# +# Tuning (repository variables, all optional): +# HIGH_VOLUME_MAX_OPEN_PRS open PRs across GitHub at or above which to act (default 50) +# HIGH_VOLUME_MAX_REPOS distinct repos among those PRs at or above which to act (default 20) +# HIGH_VOLUME_ALLOWLIST comma-separated logins that are never closed +# HIGH_VOLUME_DRY_RUN "true" to log matches on new PRs without closing them +# +# To scan PRs that are already open, run the workflow manually. It defaults to +# a dry run that only reports what it would close. + +name: Close PRs from high-volume accounts + +on: + pull_request_target: + types: [opened, reopened] + workflow_dispatch: + inputs: + dry_run: + description: "Only report matching PRs, do not close them" + type: boolean + default: true + +permissions: + contents: read + issues: write + pull-requests: write + +concurrency: + group: close-high-volume-prs-${{ github.event.pull_request.number || 'scan' }} + cancel-in-progress: false + +jobs: + check: + if: github.repository == 'apache/datafusion-comet' + runs-on: ubuntu-latest + timeout-minutes: 30 + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + EVENT_NAME: ${{ github.event_name }} + EVENT_ACTION: ${{ github.event.action }} + SENDER: ${{ github.event.sender.login }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + PR_AUTHOR_TYPE: ${{ github.event.pull_request.user.type }} + PR_ASSOCIATION: ${{ github.event.pull_request.author_association }} + MAX_OPEN_PRS: ${{ vars.HIGH_VOLUME_MAX_OPEN_PRS || '50' }} + MAX_REPOS: ${{ vars.HIGH_VOLUME_MAX_REPOS || '20' }} + ALLOWLIST: ${{ vars.HIGH_VOLUME_ALLOWLIST || '' }} + DRY_RUN: ${{ github.event_name == 'workflow_dispatch' && format('{0}', inputs.dry_run) || vars.HIGH_VOLUME_DRY_RUN || 'false' }} + LABEL: high-volume-account + steps: + - name: Check PR authors + shell: bash + run: | + set -euo pipefail + + is_allowlisted() { + local login="${1,,}" u + for u in ${ALLOWLIST//,/ }; do + [[ "${u,,}" == "$login" ]] && return 0 + done + return 1 + } + + # Prints "<open PR count> <distinct repos in the first 100 results>". + open_pr_footprint() { + gh api -X GET search/issues \ + -f q="is:pr is:open author:$1" -f per_page=100 \ + --jq '"\(.total_count) \([.items[].repository_url] | unique | length)"' + } Review Comment: This counts the open PRs an author has now, so a long-lived backlog looks the same as a current burst. In a read-only replay, an active contributor here crosses both thresholds on this count but falls well under 20 repos when only PRs created in the last 30 days are counted (`is:pr author:X created:>=2026-10-04`). The bulk accounts at the top of the list stay far above the threshold on that query. DataFusion's `Prolific Author` label uses activity too: "opened PRs or issues in more than 20 repositories in the last 30 days". Would a `created:` window separate bulk submission from an old backlog more reliably? I only replayed a handful of accounts. ########## .github/workflows/close_high_volume_prs.yml: ########## @@ -0,0 +1,183 @@ +# 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. + +# Closes pull requests from accounts that have a very large number of open +# pull requests spread across many repositories, which is the signature of +# automated bulk submission. GitHub's per-repository creation cap (.asf.yaml +# github.pull_requests.creation_cap) limits how many such PRs an account can +# have open here, but cannot see activity in other repositories. +# +# The check runs only for authors without write access: users with write access +# and apache org members are always skipped. Having a PR merged here does not +# exempt an author, since merged PRs have come from automated accounts too. Use +# HIGH_VOLUME_ALLOWLIST for accounts that should never be closed. +# +# This workflow never checks out or runs code from the pull request, which is +# what makes pull_request_target safe to use here. +# +# Tuning (repository variables, all optional): +# HIGH_VOLUME_MAX_OPEN_PRS open PRs across GitHub at or above which to act (default 50) +# HIGH_VOLUME_MAX_REPOS distinct repos among those PRs at or above which to act (default 20) +# HIGH_VOLUME_ALLOWLIST comma-separated logins that are never closed +# HIGH_VOLUME_DRY_RUN "true" to log matches on new PRs without closing them +# +# To scan PRs that are already open, run the workflow manually. It defaults to +# a dry run that only reports what it would close. + +name: Close PRs from high-volume accounts + +on: + pull_request_target: + types: [opened, reopened] + workflow_dispatch: + inputs: + dry_run: + description: "Only report matching PRs, do not close them" + type: boolean + default: true + +permissions: + contents: read + issues: write + pull-requests: write + +concurrency: + group: close-high-volume-prs-${{ github.event.pull_request.number || 'scan' }} + cancel-in-progress: false + +jobs: + check: + if: github.repository == 'apache/datafusion-comet' + runs-on: ubuntu-latest + timeout-minutes: 30 + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + EVENT_NAME: ${{ github.event_name }} + EVENT_ACTION: ${{ github.event.action }} + SENDER: ${{ github.event.sender.login }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + PR_AUTHOR_TYPE: ${{ github.event.pull_request.user.type }} + PR_ASSOCIATION: ${{ github.event.pull_request.author_association }} + MAX_OPEN_PRS: ${{ vars.HIGH_VOLUME_MAX_OPEN_PRS || '50' }} + MAX_REPOS: ${{ vars.HIGH_VOLUME_MAX_REPOS || '20' }} + ALLOWLIST: ${{ vars.HIGH_VOLUME_ALLOWLIST || '' }} + DRY_RUN: ${{ github.event_name == 'workflow_dispatch' && format('{0}', inputs.dry_run) || vars.HIGH_VOLUME_DRY_RUN || 'false' }} + LABEL: high-volume-account + steps: + - name: Check PR authors + shell: bash + run: | + set -euo pipefail + + is_allowlisted() { + local login="${1,,}" u + for u in ${ALLOWLIST//,/ }; do + [[ "${u,,}" == "$login" ]] && return 0 + done + return 1 + } + + # Prints "<open PR count> <distinct repos in the first 100 results>". + open_pr_footprint() { + gh api -X GET search/issues \ + -f q="is:pr is:open author:$1" -f per_page=100 \ + --jq '"\(.total_count) \([.items[].repository_url] | unique | length)"' + } + + close_pr() { + local number="$1" total="$2" repos="$3" body + body="$(mktemp)" + cat > "$body" <<EOF + Thank you for the contribution. This pull request was closed automatically because the account that opened it currently has $total open pull requests across at least $repos repositories. That pattern usually indicates automated bulk submission, and Comet does not have the review capacity to handle it. + + If you are a person who intends to follow this change through review, please reply here or write to [email protected], and a committer can reopen it. Review Comment: The closing comment explains the pattern but does not link the policy this PR adds. When a DataFusion maintainer closes a PR for the same reason they link the AI policy section (for example apache/datafusion#26988). Could the comment link `https://datafusion.apache.org/comet/contributor-guide/ai-policy.html` so the author can see what is expected and how an appeal works? ########## docs/source/contributor-guide/contributing.md: ########## @@ -34,6 +34,10 @@ We maintain a list of good first issues in GitHub [here](https://github.com/apac To assign yourself an issue, comment `take` on the issue. To unassign yourself, comment `untake`. +## AI-assisted contributions Review Comment: #6734 (also yours, open) adds `ai_assisted_contributions.md`, a "Using AI tools" section in this file and an `AGENTS.md` entry, and links the same DataFusion and ASF guidance. `git merge-tree` with this branch conflicts in `docs/source/contributor-guide/index.md`, and together they would leave two overlapping AI pages. Could they become one page, with the open-PR limit and automatic closure as a section? ########## .github/workflows/close_high_volume_prs.yml: ########## @@ -0,0 +1,183 @@ +# 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. + +# Closes pull requests from accounts that have a very large number of open +# pull requests spread across many repositories, which is the signature of +# automated bulk submission. GitHub's per-repository creation cap (.asf.yaml +# github.pull_requests.creation_cap) limits how many such PRs an account can +# have open here, but cannot see activity in other repositories. +# +# The check runs only for authors without write access: users with write access +# and apache org members are always skipped. Having a PR merged here does not +# exempt an author, since merged PRs have come from automated accounts too. Use +# HIGH_VOLUME_ALLOWLIST for accounts that should never be closed. +# +# This workflow never checks out or runs code from the pull request, which is +# what makes pull_request_target safe to use here. +# +# Tuning (repository variables, all optional): +# HIGH_VOLUME_MAX_OPEN_PRS open PRs across GitHub at or above which to act (default 50) +# HIGH_VOLUME_MAX_REPOS distinct repos among those PRs at or above which to act (default 20) +# HIGH_VOLUME_ALLOWLIST comma-separated logins that are never closed +# HIGH_VOLUME_DRY_RUN "true" to log matches on new PRs without closing them +# +# To scan PRs that are already open, run the workflow manually. It defaults to +# a dry run that only reports what it would close. + +name: Close PRs from high-volume accounts + +on: + pull_request_target: + types: [opened, reopened] + workflow_dispatch: + inputs: + dry_run: + description: "Only report matching PRs, do not close them" + type: boolean + default: true + +permissions: + contents: read + issues: write + pull-requests: write + +concurrency: + group: close-high-volume-prs-${{ github.event.pull_request.number || 'scan' }} + cancel-in-progress: false + +jobs: + check: + if: github.repository == 'apache/datafusion-comet' + runs-on: ubuntu-latest + timeout-minutes: 30 + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + EVENT_NAME: ${{ github.event_name }} + EVENT_ACTION: ${{ github.event.action }} + SENDER: ${{ github.event.sender.login }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + PR_AUTHOR_TYPE: ${{ github.event.pull_request.user.type }} + PR_ASSOCIATION: ${{ github.event.pull_request.author_association }} + MAX_OPEN_PRS: ${{ vars.HIGH_VOLUME_MAX_OPEN_PRS || '50' }} + MAX_REPOS: ${{ vars.HIGH_VOLUME_MAX_REPOS || '20' }} + ALLOWLIST: ${{ vars.HIGH_VOLUME_ALLOWLIST || '' }} + DRY_RUN: ${{ github.event_name == 'workflow_dispatch' && format('{0}', inputs.dry_run) || vars.HIGH_VOLUME_DRY_RUN || 'false' }} + LABEL: high-volume-account + steps: + - name: Check PR authors + shell: bash + run: | + set -euo pipefail + + is_allowlisted() { + local login="${1,,}" u + for u in ${ALLOWLIST//,/ }; do + [[ "${u,,}" == "$login" ]] && return 0 + done + return 1 + } + + # Prints "<open PR count> <distinct repos in the first 100 results>". + open_pr_footprint() { + gh api -X GET search/issues \ + -f q="is:pr is:open author:$1" -f per_page=100 \ + --jq '"\(.total_count) \([.items[].repository_url] | unique | length)"' + } + + close_pr() { + local number="$1" total="$2" repos="$3" body + body="$(mktemp)" + cat > "$body" <<EOF + Thank you for the contribution. This pull request was closed automatically because the account that opened it currently has $total open pull requests across at least $repos repositories. That pattern usually indicates automated bulk submission, and Comet does not have the review capacity to handle it. + + If you are a person who intends to follow this change through review, please reply here or write to [email protected], and a committer can reopen it. + EOF + gh label create "$LABEL" --color ededed \ + --description "Closed automatically: author has many open PRs across many repos" \ + --force > /dev/null + gh pr comment "$number" --body-file "$body" + gh pr edit "$number" --add-label "$LABEL" + gh pr close "$number" + } + + # Prints "close <total> <repos>" or "skip" for an author. Lookup + # failures resolve to "skip" so one bad lookup can't block a scan. + decide() { + local login="$1" association="$2" type="$3" + local footprint total repos + + case "$association" in + OWNER|MEMBER|COLLABORATOR) echo skip; return ;; + esac + if [[ "$type" == "Bot" ]] || is_allowlisted "$login"; then + echo skip; return + fi + + if ! footprint="$(open_pr_footprint "$login")"; then + echo "@$login: search failed" >&2; echo skip; return Review Comment: On a failed search the run ends green with only a log line, so a green run does not tell a maintainer whether the author was checked. This is not only about rate limits. In a read-only replay of this query for the authors of the PRs open here today, two existing accounts got HTTP 422 ("cannot be searched"), and the check would skip them. Could this emit a `::warning::` annotation and add the login to the step summary, so skipped-because-unknown authors are visible in the run? ########## .github/workflows/close_high_volume_prs.yml: ########## @@ -0,0 +1,183 @@ +# 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. + +# Closes pull requests from accounts that have a very large number of open +# pull requests spread across many repositories, which is the signature of +# automated bulk submission. GitHub's per-repository creation cap (.asf.yaml +# github.pull_requests.creation_cap) limits how many such PRs an account can +# have open here, but cannot see activity in other repositories. +# +# The check runs only for authors without write access: users with write access +# and apache org members are always skipped. Having a PR merged here does not +# exempt an author, since merged PRs have come from automated accounts too. Use +# HIGH_VOLUME_ALLOWLIST for accounts that should never be closed. +# +# This workflow never checks out or runs code from the pull request, which is +# what makes pull_request_target safe to use here. +# +# Tuning (repository variables, all optional): +# HIGH_VOLUME_MAX_OPEN_PRS open PRs across GitHub at or above which to act (default 50) +# HIGH_VOLUME_MAX_REPOS distinct repos among those PRs at or above which to act (default 20) +# HIGH_VOLUME_ALLOWLIST comma-separated logins that are never closed +# HIGH_VOLUME_DRY_RUN "true" to log matches on new PRs without closing them +# +# To scan PRs that are already open, run the workflow manually. It defaults to +# a dry run that only reports what it would close. + +name: Close PRs from high-volume accounts + +on: + pull_request_target: + types: [opened, reopened] + workflow_dispatch: + inputs: + dry_run: + description: "Only report matching PRs, do not close them" + type: boolean + default: true + +permissions: + contents: read + issues: write + pull-requests: write + +concurrency: + group: close-high-volume-prs-${{ github.event.pull_request.number || 'scan' }} + cancel-in-progress: false + +jobs: + check: + if: github.repository == 'apache/datafusion-comet' + runs-on: ubuntu-latest + timeout-minutes: 30 + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + GH_REPO: ${{ github.repository }} + EVENT_NAME: ${{ github.event_name }} + EVENT_ACTION: ${{ github.event.action }} + SENDER: ${{ github.event.sender.login }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_AUTHOR: ${{ github.event.pull_request.user.login }} + PR_AUTHOR_TYPE: ${{ github.event.pull_request.user.type }} + PR_ASSOCIATION: ${{ github.event.pull_request.author_association }} + MAX_OPEN_PRS: ${{ vars.HIGH_VOLUME_MAX_OPEN_PRS || '50' }} + MAX_REPOS: ${{ vars.HIGH_VOLUME_MAX_REPOS || '20' }} + ALLOWLIST: ${{ vars.HIGH_VOLUME_ALLOWLIST || '' }} + DRY_RUN: ${{ github.event_name == 'workflow_dispatch' && format('{0}', inputs.dry_run) || vars.HIGH_VOLUME_DRY_RUN || 'false' }} + LABEL: high-volume-account + steps: + - name: Check PR authors + shell: bash + run: | + set -euo pipefail Review Comment: The decision logic lives in an inline script with no test, and the description says the scan and the comment, label and close calls were not exercised. The two problems already raised are in that part. The repo has a pattern for this, `dev/ci/pr-type-label.mjs` with its test and `fork-push-changed-files.sh` with `test-fork-push-changed-files.py`, both run from preflight. Could this move to a script under `dev/ci/`, with a test that puts a stub `gh` on `PATH`? A `check-ci-config.py` pin like the bridge workflow's (no checkout, repository guard, minimal permissions) would also fit a `pull_request_target` workflow with a write token. -- 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]
