yuqi1129 commented on PR #13049: URL: https://github.com/apache/gravitino/pull/13049#issuecomment-6182307324
I tested this end to end. The approach works. Below are the results, then the issues I found and the fixes I tried. ### How I tested - **Real path**: #13847 (draft, DO NOT MERGE) is this PR rebased onto main, opened from `yuqi1129/gravitino`. All CI ran in the fork: 36 jobs passed, 6 were skipped, `Required CI` passed. apache only ran `conflict-marker-check`. - **Status mirroring**: `pull_request_target` and `schedule` read the default branch, so they can't be tested on apache before merge. I used a sandbox fork (`timelyflow/gravitino`) with the upstream references pointed at it, and opened PRs from `yuqi1129` against it. I tested PRs to the default branch and to `branch-1.3`, opening the PR late, and re-runs. ### Problems found (fixes verified in the sandbox) 1. **The first run fails when the branch is pushed before the PR is opened.** Eligibility waits only about 8s for the PR. The usual flow is to push and then open the PR in the UI, so the first run is red and needs a manual re-run. Fix: poll for the PR for about 3 min. If there is still no PR, infer the base from the upstream default branch and the latest 3 `branch-X.Y`, and pick the one with the smallest `ahead_by`. In the upstream repo itself, branches without a PR stay dormant. ([2023709](https://github.com/timelyflow/gravitino/commit/2023709ab095dd02eca74ea88d3fd75de74a9891), [08193a4](https://github.com/timelyflow/gravitino/commit/08193a4e33fe5940d50ffbdf2bbc3e6e469eb3f9), [8d9b5f0](https://github.com/timelyflow/gravitino/commit/8d9b5f0124455fdd2eb4d1b99de5cff29283a6bb)) 2. **A re-run in the fork never shows on the PR.** A completed check run can't go back to `in_progress`, so the PR shows the old result during the re-run. Fix: create a new `Fork CI` check when the run id or run attempt changes, and only update the latest check. ([8338403](https://github.com/timelyflow/gravitino/commit/8338403773f1b407dfbd4dc433ba72a819a664ef)) 3. **One API error stops the whole sweep.** Fix: add `retries: 3` on `github-script` and a try/catch for each PR. The job still fails and lists the PRs that failed. ([bca0095](https://github.com/timelyflow/gravitino/commit/bca00955c6c5bcc4b31a0f1cb30d6a7e2a6753b4); the cron change in that commit is sandbox-only.) 4. **The build timeout is too short for forks.** A fork build hit the 120 min limit (Gradle alone took 1h48m), so I raised it to 180. ([2023709](https://github.com/timelyflow/gravitino/commit/2023709ab095dd02eca74ea88d3fd75de74a9891)) 5. **The `web-ui-tests.yml` paths-filter pin `fbd0ab8` is not on the ASF allowlist.** Switched it to `ceb8a2b8`. ([03c2530](https://github.com/timelyflow/gravitino/commit/03c253082807b20737a7c82e2be37bf0783c17d9)) The PR also conflicts with main in `build.yml` (license step) and needs a rebase. 6. **The updater polls every open PR in every sweep, whether or not anything changed.** I replayed the sweep read-only against the current 312 open PRs: **354 calls per sweep** (293 of them are 404s from forks without `fork-ci.yml`), so about 1,400/h at `*/15`. Once most forks have the workflow, it becomes about 3 calls per PR, roughly 930 per sweep or 3,700/h. The `GITHUB_TOKEN` limit I measured in the sandbox is 5,000/h per repo, and other workflows share it. Fix: keep a small state file between sweeps in `actions/cache`, which does not use the REST budget. A PR is skipped with 0 calls when its head SHA and `updated_at` are unchanged, it had no activity in the last hour, and its check is final (or it never had one). PRs that are running, failed or cancelled in the last 7 days (to catch re-runs), or `action_required` in the last 24 hours are still checked. A check is only written when it changes. Replay against the 312 PRs: first sweep 316 calls, then **4–5 calls per sweep**. T hat replay has no `Fork CI` checks yet. After merge, running PRs and PRs that failed recently are still polled at 2–3 calls each. 86 PRs were updated in the last 7 days, so a sweep is about 200 calls at worst and much lower normally. Each sweep logs its call count and the remaining rate limit. ([6f7309c](https://github.com/timelyflow/gravitino/commit/6f7309c0432eb4efa636d536e873284731dcd560), [586a06e](https://github.com/timelyflow/gravitino/commit/586a06ee03ee42d445a90039edc49e65c63193f6)) 7. **`lance-compatibility-matrix-test.yml` has a weekly schedule but no `github.repository == 'apache/gravitino'` guard.** Every contributor now has to enable Actions on their fork, so this job will run weekly in all of them. It already ran (and failed) in my fork. GitHub also disabled Actions on my sandbox fork with "GitHub disabled workflows on this forked repository to reduce the amount of compute that is wasted". In my case that was likely caused by my own test cron, but unguarded inherited schedules invite the same thing on contributor forks. Fix: add the same guard that `cron-integration-test.yml` has. ### Open concerns for you to decide - **Existing open PRs.** About 290 of the ~310 currently get no check (404, skipped without a message) until the author rebases and pushes. If `Fork CI` is not a required check, they also become mergeable with no CI. - **Release branches.** `pull_request_target` reads the PR's base branch, so each `branch-1.x` needs its own backport. Until then, PRs to those branches keep using apache CI. The backport needs changes for each branch: for example, `branch-1.3` has no `contrib-catalog-test.yml` (the job must be removed or the workflow is invalid) and no `dev/release/test-check-license.sh`. - **Coverage comment.** `coverage-comment.yml` relies on `pull_request` builds and `pr-number.txt`, so it stops working. - **Trust.** The fork workflow can be edited by the contributor, so a green `Fork CI` could be faked by changing the workflow in the PR. Reviewers need to watch `.github/` changes. - **UX.** The status lags by up to 15 min. `details_url` is ignored for checks created by `github-actions`, so reviewers need one extra click through the summary link. -- 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]
