Skip to content

[ISD-3791] Fix: Add timeouts for ghapi calls - #586

Merged
yhaliaw merged 9 commits into
mainfrom
fix/ghapi-timeout
Jul 11, 2025
Merged

[ISD-3791] Fix: Add timeouts for ghapi calls#586
yhaliaw merged 9 commits into
mainfrom
fix/ghapi-timeout

Conversation

@yhaliaw

@yhaliaw yhaliaw commented Jun 27, 2025

Copy link
Copy Markdown
Collaborator

Overview

Add timeouts for all ghapi calls.
The ghapi uses urllib for making the HTTP requests to GitHub. By default there is no timeout.
This PR sets the timeout and catches the errors of urllib.

Rationale

From an investigation in the stuck process/thread in github-runner-manager, it seems it is stuck at the ghapi calls for HTTP request to GitHub.
Since the HTTP request by ghapi with urllib has no timeout, this is the most likely reason for the stuck process/thread.

EDIT: This is likely not the reason for the stuck process/thread. However, setting timeouts is generally a good practice.

Checklist

@yhaliaw yhaliaw changed the title Add timeouts for ghapi calls [ISD-3791] Fix: Add timeouts for ghapi calls Jun 30, 2025
@yhaliaw
yhaliaw marked this pull request as ready for review July 1, 2025 02:18
Comment thread github-runner-manager/src/github_runner_manager/github_client.py Outdated
@github-actions

Copy link
Copy Markdown
Contributor

Test results for commit a401192

Test coverage for a401192

Wrote XML report to coverage/coverage.xml

Static code analysis report

Run started:2025-07-10 06:40:37.580658

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 2055
  Total lines skipped (#nosec): 2
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 1

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):

@yhaliaw
yhaliaw merged commit 78fa10d into main Jul 11, 2025
109 of 122 checks passed
@yhaliaw
yhaliaw deleted the fix/ghapi-timeout branch July 11, 2025 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants