Skip to content

Commit 7e7b323

Browse files
ci: stop secondary fork-PR checks from failing (#831) (#852)
The fork verification (#849) showed 3 checks still red on fork PRs — pre-existing (also red on the original #671), separate from the cache fix, all rooted in GitHub's fork restrictions: - skip-integration-tests-pr / skip-kernel-e2e-pr post synthetic-success check-runs, needing checks:write. On fork PRs GitHub forces GITHUB_TOKEN to read-only regardless of the declared permission, so checks.create 403s ('Resource not accessible by integration'). Swallow the 403 ONLY for forks (github.event.pull_request.head.repo.fork) so the poster doesn't show a spurious failure; the REAL required checks are posted by the merge_group run (base context, full perms) when a maintainer queues the PR — the fork's code is genuinely integration-tested there. Non-fork/other errors still fail loudly. - test-with-coverage runs the e2e suite against a live warehouse and needs the azure-prod secrets, which forks never get (Secret source: None) — so it could only fail on a fork. Skip it on fork PRs; the real coverage gate runs on the merge_group transient branch (secrets present), which is what protects main. Same-repo PRs and merge_group are unaffected by all three changes. Refs #831 Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
1 parent 37320a5 commit 7e7b323

3 files changed

Lines changed: 65 additions & 24 deletions

File tree

.github/workflows/code-coverage.yml

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,15 @@ concurrency:
3737

3838
jobs:
3939
test-with-coverage:
40+
# This job runs the e2e suite against a live warehouse, so it needs the
41+
# azure-prod secrets. FORK PRs never receive those secrets (Secret source:
42+
# None), so on a fork the job can only fail. Skip it on fork PRs — the real
43+
# coverage gate runs on the merge_group's transient branch (base context,
44+
# secrets present), which is what actually protects main. Same-repo PRs and
45+
# merge_group are unaffected.
46+
if: >-
47+
github.event_name != 'pull_request' ||
48+
github.event.pull_request.head.repo.full_name == github.repository
4049
runs-on:
4150
group: databricks-protected-runner-group
4251
labels: linux-ubuntu-latest

.github/workflows/kernel-e2e.yml

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -106,19 +106,35 @@ jobs:
106106
with:
107107
github-token: ${{ github.token }}
108108
script: |
109-
await github.rest.checks.create({
110-
owner: context.repo.owner,
111-
repo: context.repo.repo,
112-
name: 'Kernel E2E',
113-
head_sha: context.payload.pull_request.head.sha,
114-
status: 'completed',
115-
conclusion: 'success',
116-
completed_at: new Date().toISOString(),
117-
output: {
118-
title: 'Skipped on PR — runs in merge queue',
119-
summary: 'Kernel E2E is skipped on PRs and runs as a required gate in the merge queue. Add the `kernel-e2e` label to preview on this PR.'
109+
// On FORK PRs GitHub forces GITHUB_TOKEN to read-only regardless of
110+
// the declared `checks: write`, so checks.create 403s ("Resource
111+
// not accessible by integration"). That's expected — a fork can't
112+
// post check-runs on the base repo. Swallow the 403 for forks so
113+
// this poster doesn't show a spurious failure; the real Kernel E2E
114+
// required check is posted by the merge_group run (full perms) when
115+
// a maintainer queues the PR. Any other error still fails loudly.
116+
const isFork = context.payload.pull_request.head.repo.fork;
117+
try {
118+
await github.rest.checks.create({
119+
owner: context.repo.owner,
120+
repo: context.repo.repo,
121+
name: 'Kernel E2E',
122+
head_sha: context.payload.pull_request.head.sha,
123+
status: 'completed',
124+
conclusion: 'success',
125+
completed_at: new Date().toISOString(),
126+
output: {
127+
title: 'Skipped on PR — runs in merge queue',
128+
summary: 'Kernel E2E is skipped on PRs and runs as a required gate in the merge queue. Add the `kernel-e2e` label to preview on this PR.'
129+
}
130+
});
131+
} catch (e) {
132+
if (isFork && e.status === 403) {
133+
core.notice('Fork PR: cannot post the Kernel E2E check-run (read-only token). It will be posted by the merge queue at merge time.');
134+
} else {
135+
throw e;
120136
}
121-
});
137+
}
122138
123139
# ───────────────────────────────────────────────────────────────
124140
# Detect whether kernel-relevant files changed. Used by both the

.github/workflows/trigger-integration-tests.yml

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -137,21 +137,37 @@ jobs:
137137
with:
138138
github-token: ${{ github.token }}
139139
script: |
140+
// On FORK PRs GitHub forces GITHUB_TOKEN to read-only regardless of
141+
// the declared `checks: write`, so checks.create 403s ("Resource
142+
// not accessible by integration"). Expected — a fork can't post
143+
// check-runs on the base repo. Swallow the 403 for forks so this
144+
// poster doesn't show a spurious failure; the real Python Proxy
145+
// Tests required checks are posted by the merge_group run (full
146+
// perms) when a maintainer queues the PR. Other errors fail loudly.
147+
const isFork = context.payload.pull_request.head.repo.fork;
140148
const MODES = ['thrift', 'kernel'];
141149
for (const mode of MODES) {
142-
await github.rest.checks.create({
143-
owner: context.repo.owner,
144-
repo: context.repo.repo,
145-
name: `Python Proxy Tests / ${mode}`,
146-
head_sha: context.payload.pull_request.head.sha,
147-
status: 'completed',
148-
conclusion: 'success',
149-
completed_at: new Date().toISOString(),
150-
output: {
151-
title: 'Skipped on PR — runs in merge queue',
152-
summary: `Python Proxy Tests (${mode}) are skipped on PRs and run as a required gate in the merge queue. Add the \`integration-test\` label to preview them on this PR.`
150+
try {
151+
await github.rest.checks.create({
152+
owner: context.repo.owner,
153+
repo: context.repo.repo,
154+
name: `Python Proxy Tests / ${mode}`,
155+
head_sha: context.payload.pull_request.head.sha,
156+
status: 'completed',
157+
conclusion: 'success',
158+
completed_at: new Date().toISOString(),
159+
output: {
160+
title: 'Skipped on PR — runs in merge queue',
161+
summary: `Python Proxy Tests (${mode}) are skipped on PRs and run as a required gate in the merge queue. Add the \`integration-test\` label to preview them on this PR.`
162+
}
163+
});
164+
} catch (e) {
165+
if (isFork && e.status === 403) {
166+
core.notice(`Fork PR: cannot post the Python Proxy Tests / ${mode} check-run (read-only token). It will be posted by the merge queue at merge time.`);
167+
} else {
168+
throw e;
153169
}
154-
});
170+
}
155171
}
156172
157173
# =============================================================================

0 commit comments

Comments
 (0)