Skip to content

fix(ci): rate limit and permission errors in trace publishing #17238

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Kangyan-Zhou merged 1 commit into from
Jan 19, 2026
Merged

fix(ci): rate limit and permission errors in trace publishing #17238

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 50 additions & 2 deletions scripts/ci/publish_traces.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,39 @@


def is_rate_limit_error(e):
"""Check if an exception is a GitHub permission/quota error that should not be retried"""
return isinstance(e, HTTPError) and e.code in [403, 429]
"""Check if an exception is a GitHub rate limit error (not permission error)"""
if not isinstance(e, HTTPError):
return False
if e.code == 429:
return True
if e.code == 403:
# 403 can be rate limit OR permission error - check the message
error_body = getattr(e, "error_body", "")
if isinstance(error_body, str):
# Rate limit errors contain specific phrases
rate_limit_phrases = [
"rate limit",
"abuse detection",
"secondary rate limit",
]
return any(phrase in error_body.lower() for phrase in rate_limit_phrases)
return False


def is_permission_error(e):
"""Check if an exception is a GitHub permission error"""
if not isinstance(e, HTTPError) or e.code != 403:
return False
error_body = getattr(e, "error_body", "")
if isinstance(error_body, str):
permission_phrases = [
"resource not accessible",
"must have push access",
"permission",
"denied",
]
return any(phrase in error_body.lower() for phrase in permission_phrases)
return False


def make_github_request(url, token, method="GET", data=None):
Expand Down Expand Up @@ -349,6 +380,14 @@ def publish_traces(traces_dir, run_id, run_number):
"GitHub API rate limit exceeded during blob creation. Skipping trace upload."
)
return
# Check for permission errors - these should fail loudly
if is_permission_error(e):
print(
f"ERROR: Token does not have write permission to {repo_owner}/{repo_name}. "
"Please update the GH_PAT_FOR_NIGHTLY_CI_DATA secret with a token that has "
"'contents: write' permission for the repository."
)
sys.exit(1)
Comment on lines +384 to +390
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This error handling logic for permission errors is duplicated at lines 453-459. To avoid code duplication and improve maintainability, consider extracting this block into a dedicated helper function. You could create a function like _handle_permission_error(repo_owner, repo_name) that prints the message and exits, then call it from both locations.

print(f"Failed to create blobs: {e}")
raise

Expand Down Expand Up @@ -410,6 +449,15 @@ def publish_traces(traces_dir, run_id, run_number):
warnings.warn("GitHub API rate limit exceeded. Skipping trace upload.")
return

# Check for permission errors - these should fail loudly
if is_permission_error(e):
print(
f"ERROR: Token does not have write permission to {repo_owner}/{repo_name}. "
"Please update the GH_PAT_FOR_NIGHTLY_CI_DATA secret with a token that has "
"'contents: write' permission for the repository."
)
sys.exit(1)

if is_retryable and attempt < max_retries - 1:
print(
f"Attempt {attempt + 1}/{max_retries} failed ({error_type}). Retrying in {retry_delay} seconds..."
Expand Down
Loading