Skip to content

github-status-test: use a GitHub App for log downloads, with PAT fallback - #8493

Merged
huydhn merged 4 commits into
mainfrom
huydhn/github-status-test-github-app
Aug 13, 2026
Merged

github-status-test: use a GitHub App for log downloads, with PAT fallback#8493
huydhn merged 4 commits into
mainfrom
huydhn/github-status-test-github-app

Conversation

@huydhn

@huydhn huydhn commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Downloads job logs with a GitHub App installation token instead of a PAT from the GITHUB_TOKENS pool, falling back to that pool when the app is rate limited, rejected, or not installed on the repo's owner.

With GITHUB_APP_ID / GITHUB_APP_PRIVATE_KEY unset the behaviour is identical to today, so this rolls back by clearing two env vars — no code change or redeploy.

  • Tokens are minted with PyGithub's GithubIntegration, same as cross_repo_ci_relay, rather than hand-rolling the app JWT.
  • Installation tokens cached per repo owner in module scope, refreshed 5 min before the 1-hour expiry, so warm invocations don't mint one per job.
  • Fallback on 401/403/429. On a rate limit the app is parked until x-ratelimit-reset so later jobs skip it instead of each paying for a doomed request first.
  • Repos outside the installation (e.g. vllm-project/vllm) go straight to the pool.

Two existing bugs fixed, both needed for fallback to work:

  • Non-OK responses were archived as if they were logs, so a rate-limited 403 body would land in S3 as the job's log and be sent to the classifier.
  • except HTTPError caught urllib's, never requests'.

Packaging: the lambda runs on python3.9 but the deploy workflow's setup-python pins no version, so pip install used the runner's default. cryptography (via PyGithub) ships compiled wheels, so the Makefile now pins the target platform/python version — otherwise the zip gets wheels the runtime can't import.

Before this takes effect

Merging deploys the code but changes nothing until the env vars are set.

  1. Set GITHUB_APP_ID and GITHUB_APP_PRIVATE_KEY (base64 PEM) on the function in account 308535385114.
  2. Grant the app actions: read — log downloads currently succeed without it only because pytorch repos are public.
  3. Prefer a dedicated app over pytorch-bot: the 15,000/hr limit is per installation, and Dr. CI plus the HUD already draw on that one.

@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
torchci Ignored Ignored Aug 13, 2026 6:32pm

Request Review

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 11, 2026
…back

Job logs are currently downloaded with a randomly chosen PAT from the
GITHUB_TOKENS pool. PATs are tied to a personal account, need `repo` scope
to reach private repos, and rotate whenever their owner does.

Download logs with a GitHub App installation token instead, falling back to
the existing PAT pool when the app is rate limited, rejected, or not
installed on the repo's owner. Both app env vars unset means the previous
behaviour exactly, so the app can be rolled back by clearing them.

- GITHUB_APP_ID / GITHUB_APP_PRIVATE_KEY (base64 PEM, raw PEM also accepted)
- Tokens are minted with PyGithub's GithubIntegration, matching
  cross_repo_ci_relay, rather than hand-rolling the app JWT
- Installation tokens are cached per repo owner in module scope and
  refreshed 5 minutes early, so warm invocations don't mint one per job
- Fallback triggers on 401/403/429. On a rate limit the app is parked until
  x-ratelimit-reset so later jobs skip it rather than each paying for a
  doomed request first
- Repos outside the installation (e.g. vllm-project/vllm) resolve to no
  installation and go straight to the pool, cached briefly to avoid a
  lookup per job

Two existing bugs fixed, both needed for the fallback to work:

- Non-OK responses were archived as though they were logs, so a rate-limited
  403 body would land in S3 as the job's log and be sent to the classifier.
  Detecting that condition is also what fallback depends on.
- `except HTTPError` caught urllib's, never requests'. It now catches
  requests.RequestException too.

The lambda runs on python3.9 but the deploy workflow's setup-python pins no
version, so `pip install` used the runner's default. cryptography (via
PyGithub) ships compiled wheels, so the Makefile now pins the target platform
and python version; otherwise the zip gets wheels the runtime can't import.
@huydhn
huydhn force-pushed the huydhn/github-status-test-github-app branch from 303c6f8 to dd331e7 Compare August 11, 2026 18:22
@huydhn
huydhn requested a review from izaitsevfb August 11, 2026 18:36
@huydhn
huydhn marked this pull request as ready for review August 11, 2026 18:41
@izaitsevfb

Copy link
Copy Markdown
Contributor

flag from iz2:

Fix before the app is switched on [verified, runnable repro]: _token_cache is keyed by owner, but get_repo_installation() resolves per repo. Fine for an "All repositories" install; broken for "Selected repositories" — which is what scoping it properly produces. Two ways:
• installed repo warms the cache → non-installed sibling reuses its token → GitHub 404 → 404 ∉ FALLBACK_STATUSES → log dropped, no PAT fallback
• sibling queried first → (None, TTL) under "pytorch" → installed repo skips the app 15 min

also:

the 178 new tests never run — tests.yml collects only aws/lambda/tests, and deploy (only job running make prepare) is skipped on PRs.

get_repo_installation() resolves per repo, so an owner-keyed cache is wrong
for a 'Selected repositories' install: one repo's token would be handed to a
sibling, or one repo's 'not installed' result would park the whole owner on
the PAT path for 15 minutes. Key by owner/repo instead.

The test file was never collected -- tests.yml only runs aws/lambda/tests,
and the deploy job is gated to push-on-main. Add a test job to the lambda's
own workflow, matching opensearch-gha-jobs-lambda.yml. pytest goes on the
install line rather than requirements.txt, which the Makefile ships in the
deployment zip.
@huydhn

huydhn commented Aug 13, 2026

Copy link
Copy Markdown
Contributor Author

flag from iz2:

Fix before the app is switched on [verified, runnable repro]: _token_cache is keyed by owner, but get_repo_installation() resolves per repo. Fine for an "All repositories" install; broken for "Selected repositories" — which is what scoping it properly produces. Two ways:
• installed repo warms the cache → non-installed sibling reuses its token → GitHub 404 → 404 ∉ FALLBACK_STATUSES → log dropped, no PAT fallback
• sibling queried first → (None, TTL) under "pytorch" → installed repo skips the app 15 min

also:

the 178 new tests never run — tests.yml collects only aws/lambda/tests, and deploy (only job running make prepare) is skipped on PRs.

Both are fixed. On the first point, the app is installed for all repos, so it would still work, but agree that keying by owner/repo is better.

Tests are now running on PR and before deployment https://github.com/pytorch/test-infra/actions/runs/31658811180

A manual deploy today shipped the new lambda_function.py with a package built
from the old requirements.txt. The zip had no 'github' module, so every
invocation died at import with Runtime.ImportModuleError -- dropping all
webhook payloads, not just the log downloads -- until it was rolled back.

make prepare now starts from clean so a stale packages/ or zip cannot leak
into the artifact, and verifies the built zip actually contains every module
lambda_function.py imports before deploy can run.

Also flag in the README that make deploy is immediately live: the API Gateway
integration points at the unqualified function, so update-function-code goes
straight to $LATEST. The publish-a-version steps in that doc no longer match
how this is wired.
@huydhn
huydhn merged commit 7313fe5 into main Aug 13, 2026
12 checks passed
@huydhn
huydhn deleted the huydhn/github-status-test-github-app branch August 13, 2026 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants