[SPARK-60615][INFRA] Restore PR labeler (without pull_request_target) - #59862

LGTM if it works

Review summary

I found no defects in the new workflow or the rule file. Checked against the pinned actions/labeler v7 release (bf12e9b):

• The hours input is validated before use, and scheduled runs default to 12.
• The cut-off timestamp uses the same UTC ISO-8601 format as updated_at, so the string comparison is correct.
• PR numbers reach pr-number one per line, which is the form its parser accepts.
• The labeler reads its rules from the sparse checkout of master's .github, so no PR code is checked out.
• sync-labels: true removes only labels defined in .github/labeler.yml, so labels such as ai-review-requested are kept.
• The current ubuntu-slim software report in actions/runner-images (image 20261026.36.1) lists GitHub CLI 2.103.2, so the gh api listing step has what it needs on that runner.

Two questions for the author follow.

Findings

0 total: 0 P0, 0 P1, 0 P2, 0 P3.

No findings.

Decision challenges

CORE now also labels PRs that only add an error condition

What I verified: CORE now also matches common/utils/**/* and common/utils-java/**/* (lines 71-72). That pulls in the shared error registry, common/utils/src/main/resources/error/error-conditions.json, which any component edits when it adds an error condition. I replayed the old and new rules, using the labeler's minimatch semantics, over the 400 non-merge commits before the merge base (2026-09-16 to 2026-10-17). CORE would apply to 94 commits instead of 69. In 20 of the 25 new matches, error-conditions.json is the only file that matches CORE. 15 of the 21 commits touching that file were [SQL]-only PRs, for example SPARK-60262, a Parquet read fix. The other five new matches look intended: three MavenUtils changes, one python/pyspark/core change and one LogKeys.java change. So roughly one in five CORE-labeled PRs would carry CORE only because of the error registry.

What I can't settle from the repository: is that intended? If PRs that only add an error condition shouldn't get CORE, you could move 'common/utils/**/*' into its own all-globs-to-any-file entry together with '!common/utils/src/main/resources/error/**', the same exclusion style CORE already uses for UI paths. The rest of common/utils would stay under CORE.

One missing configured label stops labeling for the whole batch

The header comment and the PR description say every label in .github/labeler.yml must already exist, because this job cannot create labels. What isn't covered is how far one missing label reaches. All PR numbers in the window go to a single actions/labeler@v7 invocation. In v7 (bf12e9b), labeler.ts handles them in order, and any setLabels error ends the whole invocation, including the 403 it rethrows when the token cannot create a label. So the first PR that needs a missing label (UDF or DECLARATIVE PIPELINES today, or any label added to the config later) stops labeling for every PR after it in the list. The list comes back newest-created first, so that means all the older PRs in the window. That PR stays in the 12-hour window, so every 15-minute run fails at the same point until someone creates the label. Over the 400 non-merge commits before the merge base, UDF alone would have matched 42, so most windows will contain such a PR. The removed per-event workflow ran with the same permissions, but there a missing label failed only that PR's run.

Is that blast radius acceptable, given that both labels will be created before merging? Or would you rather add issues: write to the job's permissions, as the permissions example in the actions/labeler README does, so the action creates any missing label itself? That would remove the TODO and this failure mode, at the cost of a broader token scope in a job that never runs PR code.

Generated by Omnigent on Databricks

Thank you for restoring the labeler, @nchammas. I reviewed the latest commit, abddbc5, and left 11 inline comments. None of them repeats the two decision challenges in the spark-test review, which you already addressed; item 6 is the same kind of case as the first one, in another registry. Here is a summary, ordered by priority.

Workflow

1. .github/workflows/labeler.yml L46: ubuntu-slim jobs are limited to 15 minutes, which a backfill of the 691 unlabeled open PRs is unlikely to fit in.
2. .github/workflows/labeler.yml L80: with sync-labels: true and the 15-minute rescan, a configured label added by hand is removed within about 15 minutes.
3. .github/workflows/labeler.yml L69: only open PRs are listed, so a backfill cannot reach PRs merged or closed during a gap.
4. .github/workflows/labeler.yml L39: a pending manual backfill is cancelled by the next scheduled run in the same concurrency group.
5. .github/workflows/labeler.yml L67: the fixed 12-hour lookback re-checks each updated PR about 48 times, yet still misses gaps longer than 12 hours.

Label rules

6. .github/labeler.yml L80: common/config/**/* gives CORE to SQL PRs that only add a config in sql.textproto.
7. .github/labeler.yml L65: CONNECT does not match connector/connect/**, where Spark Connect lives on branch-3.5.
8. .github/labeler.yml L157: the other committer scripts in dev/ get BUILD instead of INFRA.
9. .github/labeler.yml L284: the UDF globs also match SQL golden files and benchmark results.
10. .github/labeler.yml L117: **/README* also matches README files that are data notes or test fixtures.

Efficiency

11. .github/workflows/labeler.yml L68: each run pages through all open PRs and filters on updated_at on the client.

The most important are items 1 and 2: the first backfill after merge, and whether labels added by hand should survive. Items 6-11 are small.

Review summary

I reviewed the scheduled labeler workflow and the rule file at a2b1f89. The two questions from the previous spark-test review are settled. The error-condition registry is now excluded from CORE, and the author says the new labels were created by hand.

I found three small issues, each easy to fix:

• The PR listing pages through issues in ascending update order. When a listing spans more than one page and an item already returned is updated before the next page is fetched, one PR is skipped while the run still succeeds. Listing in descending order avoids this for updates.
• The comment above the 10-minute overlap gives a reason the overlap does not serve. Updates made during the previous run already fall after its start; the overlap covers clock skew or listing lag just before it.
• Both UDF lists put the .sql.out exclusion before the -results.txt one, which goes against the sort order stated at the top of the rule file.

I also ran the cut-off arithmetic, date -u -d "$last_success - 10 minutes", on timestamps in the Z-suffixed form that gh returns. It gives the right UTC time, including across day and year boundaries. One question for the author follows.

Findings

3 total: 0 P0, 0 P1, 0 P2, 3 P3.

Nit (P3)

• Ascending update-order pagination can skip a PR — .github/workflows/labeler.yml:83 — see inline.
• Overlap comment gives the wrong reason for the 10 minutes — .github/workflows/labeler.yml:74 — see inline.
• UDF exclusions break the file's sort order — .github/labeler.yml:300 — see inline.

Re-review status

Prior AI findings: 0 no longer raised, 0 still raised; additional unresolved findings in this review: 3.

New attribution: 3 newly introduced, 0 late catch, 0 previously raised, 0 unattributed.

PR description suggestions

• In "Why are the changes needed?", say that scheduled runs resume from the last successful scheduled run, so a delay such as a GitHub Actions outage is caught up automatically, including PRs closed in the meantime. The backfill command is only needed for the initial labeling of open PRs. This follows up on existing discussions 1, 2.

Decision challenges

Backfill restarts stale.yml's 100-day clock on most open PRs

The backfill (L66-67) lists every open PR and writes labels to each one whose labels change, which updates its updated_at. stale.yml runs actions/stale@v11 with days-before-stale: 100 and days-before-close: 0. It does not set ignore-updates, so it measures inactivity from updated_at.

On the base branch nothing touches idle PRs, so stale.yml closes each one 100 days after its last real update. With this change, the first backfill labels most open PRs (a reviewer counted 691 of 725 without labels), and each of them starts a new 100-day clock. For example, an unlabeled SQL PR that has been idle for 90 days, and would close in about 10 days, stays open for another 100. For roughly 100 days after the backfill, stale.yml closes almost none of the PRs that were open at the time. It then catches up at no more than 30 operations per daily run, the default limit. A later backfill, for example after a rule change, restarts the clock for every PR whose labels it changes. Scheduled runs have no such effect, because they only write to PRs that were just updated anyway.

Is that reset an acceptable price for labeling every open PR at once? If not, the backfill could take a recent since bound, such as 30 days. PRs idle for longer would then keep aging toward closure, but stay unlabeled until their next update.

Generated by Omnigent on Databricks