ci: pr-test-summary job shows red when the GitHub API returns persistent 5xx errors

Open Beginner friendly
  • Is something wrong?
#2,256 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 1 day

Nobody has claimed this yet.

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
72/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
github-actions, python
Domain
ci-cd

Research direction

Start with ci/utils/pr_test_summary.py, especially main() and the retry wrapper for GitHub API calls, then read the PR test summary step in pr.yaml. Confirm how the final exception from a persistent 5xx reaches the process exit. Done means that only HTTP and network errors from the API are downgraded to a logged warning with exit 0, while parsing or other script errors still fail the job.

Written by the indexing model from the issue text.

Description

Problem

The PR test summary (non-blocking) job in pr.yaml sometimes fails with a red check on PRs, for example:

Traceback (most recent call last):
  File "/home/runner/work/cuopt/cuopt/ci/utils/pr_test_summary.py", line 325, in <module>
    main()
...
urllib.error.HTTPError: HTTP Error 502: Bad Gateway

This was noticed in the discussion on #2241 (run https://github.com/NVIDIA/cuopt/actions/runs/41135602235). It was first mistaken for leftover RAPIDS CI telemetry, but it comes from this repo's own ci/utils/pr_test_summary.py.

Why it fails

pr_test_summary.py makes several GitHub API calls: it lists the run's jobs, downloads logs of failed jobs, and creates or updates the PR comment.

  • Transient errors (429, 5xx, network errors) are retried up to 4 attempts with 2**attempt second backoff (#2204). That is roughly 14 seconds in total. If the API keeps returning errors for longer, the last exception is re-raised.
  • The script then exits non-zero, and the run: python3 ci/utils/pr_test_summary.py step has no continue-on-error. #1696 removed it and instead excluded the job from the pr-builder merge gate (and checks has ignored_pr_jobs: "pr-test-summary").
  • The job is therefore not a merge gate, but it still shows as a failed check on the PR. That is confusing for contributors and reviewers, who may take it for a real test failure, and the summary comment is not posted.

Proposal

Make the summary best-effort so it cannot show red because of an API error:

  • Catch persistent API errors at the end of main(), print a warning to the job log, and exit 0.
  • Or set continue-on-error: true on the step.

Real script bugs (for example parsing errors) should still fail the job, so only the network and HTTP error classes would be swallowed.

Related: #2204, #1696, #2241.

Dominant language
Cuda
Stars
1.1k
Forks
249
Avg merge
1d 15h
Merged PRs (30d)
124

Getting set up

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from NVIDIA/cuopt

All issues in NVIDIA/cuopt

Similar issues

More DevOps issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.