mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-07-09 17:28:58 +00:00
fix: actions rate limit (#164)
* ci: add rate limit handling and version verification to GitHub Actions workflow - Add version check after OCR installation to verify successful setup - Implement exponential backoff retry logic for GitHub API rate limits - Add delays between individual comment posts to avoid secondary rate limits - GitHub enforces ~80 content-generating requests per minute; spacing calls helps stay under that threshold with 2-second base delay and up to 3 retries * ci: refine rate limit handling in GitHub Actions workflow - Add `|| true` to ocr version check for error isolation - Narrow rate limit detection to 429 and 403 with rate-limit message matching, avoiding retries on permission/auth failures - Extract hardcoded delay constants into env-configurable variables (OCR_RETRY_BASE_DELAY, OCR_MAX_RETRIES, OCR_SUCCESS_DELAY, OCR_FAILURE_DELAY) with sensible defaults - Document optional environment variables in workflow header * docs: add environment variable configuration guide for retry and delay settings * ci: add rate-limit resilience and version check to GitLab CI pipeline - Add `ocr version || true` after install for diagnostic logging - Add `api_request_with_retry` function with exponential backoff for 429 and 403 (rate-limit message matching) errors - Respect GitLab `Retry-After` header when present - Extract delay constants into CI/CD-configurable variables (OCR_RETRY_BASE_DELAY, OCR_MAX_RETRIES, OCR_SUCCESS_DELAY, OCR_FAILURE_DELAY) with defaults - Add pacing delays between successful/failed discussion posts - Document optional CI/CD variables in pipeline header comments * docs: add retry/delay settings section to GitLab CI README * ci: fix rate-limit retry delay exhaustion handling in CI workflows - GitHub Actions: distinguish exhausted rate-limit retries from other errors, apply SUCCESS_DELAY (2s) instead of FAILURE_DELAY (1s) when retries exhausted - GitLab CI: return structured result from api_request_with_retry to differentiate failure types, apply context-aware delays based on rate-limit exhaustion status - Both: prevent perpetuating rate-limit failures by using longer delays after retry exhaustion * ci: align GitHub Actions rate-limit retry with header-based strategy Derive wait durations from response headers (retry-after, x-ratelimit-reset) instead of fixed exponential backoff, add proactive throttle when remaining quota is low, honor batch-level rate limits before per-comment retry, and add support for transient 5xx/408 errors. * feat(gitlab-ci): enhance rate-limit handling with jitter, max retry delay, and proactive throttling - Add ±25% jitter on retry delays to avoid thundering herd problems - Add OCR_MAX_RETRY_DELAY (default 60s) to cap per-retry wait time - Add OCR_RATE_LIMIT_THRESHOLD (default 10) for proactive throttling based on GitLab RateLimit-Remaining response header - Parse Retry-After header properly (handle non-numeric values) - Apply retry logic to all API requests (notes, versions, discussions) - Parse and log RateLimit-Remaining/Limit headers for observability - Double pacing delay when remaining quota drops below threshold - Update README with new configuration variables and behavior docs * fix(examples): sync rate-limit docs with script defaults and add missing variables - GitHub Actions README: fix OCR_RETRY_BASE_DELAY default from 2000 to 60000 (matching script code and header comments) - GitHub Actions README: add missing OCR_RETRY_MAX_DELAY, OCR_LOW_REMAINING_THRESHOLD, OCR_LOW_REMAINING_SPACING variables - GitHub Actions README: add GitHub Rate Limits doc reference link - GitHub Actions yml header: add OCR_LLM_USE_ANTHROPIC and llm.extra_body notes - GitLab CI yml header: add llm.extra_body note - GitLab CI README: add GitLab Rate Limits doc reference link * ci(examples): unify header lookup and add transient retry backoff - GitHub Actions: extract inline header closure into a reusable getHeader helper; use it in both computeRetryDelayMs and logRateLimitQuota for consistent case-insensitive header access. - GitHub Actions: use a 2s transientBase for 5xx/408 exponential backoff instead of the 60s rate-limit base, since server hiccups are typically short-lived and the longer base stalled CI jobs unnecessarily. - GitLab CI: add a _get_header helper and route all header access (Retry-After, RateLimit-*) through it, matching the GitHub Actions approach. - GitLab CI: add transient retry logic for 5xx/408 errors with a 2s base delay, so server errors no longer fail immediately.
This commit is contained in:
Lei Zhang
GitHub
parent
57273a9ea2
commit
7f22ba867d
4 changed files with 371 additions and 45 deletions
|
|
@ -91,6 +91,22 @@ Use the `--rule` flag to pass a custom rules JSON file:
|
|||
run: ocr review --rule ./my-rules.json --from origin/${{ github.base_ref }} --to origin/${{ github.head_ref }}
|
||||
```
|
||||
|
||||
### Adjust retry and delay settings
|
||||
|
||||
When posting review comments individually (fallback mode), the workflow includes rate-limit handling with exponential backoff. The retry strategy follows GitHub's documented guidance for REST API rate limits — see [Rate limits for the REST API](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api?apiVersion=2026-03-10) for details on primary/secondary rate limits and recommended retry behavior. You can configure the retry and delay behavior via **repository variables** (Settings → Secrets and variables → Actions → Variables):
|
||||
|
||||
| Variable | Default | Description |
|
||||
|----------|---------|-------------|
|
||||
| `OCR_RETRY_BASE_DELAY` | `60000` | Base delay (ms) for exponential backoff when no retry header is present (per GitHub's "at least one minute" recommendation for secondary limits) |
|
||||
| `OCR_RETRY_MAX_DELAY` | `300000` | Maximum delay (ms) cap applied to every computed wait, including retry-after and x-ratelimit-reset, so a far-future reset cannot stall the job past its timeout |
|
||||
| `OCR_MAX_RETRIES` | `3` | Maximum retry attempts per comment when rate-limited |
|
||||
| `OCR_SUCCESS_DELAY` | `2000` | Delay (ms) after a successful comment post to pace subsequent requests |
|
||||
| `OCR_FAILURE_DELAY` | `1000` | Delay (ms) after a non-rate-limit failure to pace subsequent requests |
|
||||
| `OCR_LOW_REMAINING_THRESHOLD` | `3` | When x-ratelimit-remaining is at or below this value, proactively increase request spacing to avoid hitting the limit |
|
||||
| `OCR_LOW_REMAINING_SPACING` | `10000` | Request spacing (ms) used when remaining quota is low |
|
||||
|
||||
These variables are optional — if not configured, sensible defaults are used. Consider increasing delays for repositories with many concurrent workflows or large PRs that generate numerous review comments.
|
||||
|
||||
### Limit concurrency
|
||||
|
||||
Adjust the `--concurrency` flag for large PRs to control the number of concurrent LLM requests:
|
||||
|
|
|
|||
|
|
@ -13,8 +13,34 @@
|
|||
#
|
||||
# Optional secrets:
|
||||
# OCR_LLM_MODEL - Model name (default: gpt-4o)
|
||||
# OCR_LLM_USE_ANTHROPIC - Set to 'true' if using Anthropic Claude models
|
||||
#
|
||||
# Optional variables (for retry/delay tuning):
|
||||
# The retry strategy follows GitHub's documented guidance for REST API rate limits:
|
||||
# https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api
|
||||
# - Primary rate limit exhausted (x-ratelimit-remaining=0): wait until x-ratelimit-reset.
|
||||
# - Secondary rate limit with a retry-after header: wait exactly that long.
|
||||
# - Secondary rate limit with NO header: wait at least one minute, then use
|
||||
# exponential backoff on continued failures.
|
||||
#
|
||||
# OCR_RETRY_BASE_DELAY - Base delay (ms) for exponential backoff when no retry
|
||||
# header is present (default: 60000, per GitHub's
|
||||
# "at least one minute" recommendation for secondary limits).
|
||||
# OCR_RETRY_MAX_DELAY - Maximum delay (ms) cap applied to EVERY computed wait,
|
||||
# including retry-after and x-ratelimit-reset, so a far-future
|
||||
# reset cannot stall the job past its timeout (default: 300000 = 5 min).
|
||||
# OCR_MAX_RETRIES - Max retry attempts per comment when rate-limited (default: 3).
|
||||
# OCR_SUCCESS_DELAY - Delay (ms) between successful comment posts to pace requests (default: 2000).
|
||||
# OCR_FAILURE_DELAY - Delay (ms) after a non-retryable failure to pace subsequent requests (default: 1000).
|
||||
# OCR_LOW_REMAINING_THRESHOLD - When x-ratelimit-remaining is at or below this value,
|
||||
# proactively increase request spacing to avoid hitting the limit
|
||||
# (default: 3; GitHub best practice is to watch the header and slow down).
|
||||
# OCR_LOW_REMAINING_SPACING - Request spacing (ms) used when remaining quota is low
|
||||
# (default: 10000 = 10s).
|
||||
#
|
||||
# Note: GITHUB_TOKEN is automatically provided by GitHub Actions.
|
||||
# Note: The workflow also configures llm.extra_body to '{"thinking": {"type": "disabled"}}'
|
||||
# to disable thinking mode for compatibility with various LLM providers.
|
||||
|
||||
name: OpenCodeReview PR Review
|
||||
|
||||
|
|
@ -76,7 +102,10 @@ jobs:
|
|||
node-version: '20'
|
||||
|
||||
- name: Install OpenCodeReview
|
||||
run: npm install -g @alibaba-group/open-code-review
|
||||
run: |
|
||||
npm install -g @alibaba-group/open-code-review
|
||||
echo "OpenCodeReview installed with version:"
|
||||
ocr version || true
|
||||
|
||||
- name: Configure OCR
|
||||
run: |
|
||||
|
|
@ -226,7 +255,7 @@ jobs:
|
|||
const failedComments = [];
|
||||
|
||||
try {
|
||||
await github.rest.pulls.createReview({
|
||||
const batchRes = await github.rest.pulls.createReview({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
pull_number: prNumber,
|
||||
|
|
@ -237,28 +266,85 @@ jobs:
|
|||
});
|
||||
successCount = reviewComments.length;
|
||||
console.log(`Successfully posted review with ${successCount} inline comments (${commentsWithoutLine.length} in summary)`);
|
||||
logRateLimitQuota(batchRes, 'after batch createReview');
|
||||
} catch (e) {
|
||||
console.log('Failed to post review with inline comments:', e.message);
|
||||
console.log('Falling back to posting comments individually...');
|
||||
console.log('Falling back to posting comments individually with rate-limit-aware retry...');
|
||||
|
||||
// Fallback: post comments one by one
|
||||
// Fallback: post comments one by one with delay to avoid secondary rate limits.
|
||||
// GitHub enforces ~80 content-generating requests per minute; spacing calls
|
||||
// helps stay under that threshold. Retry/wait durations are derived from the
|
||||
// rate-limit response headers per GitHub's documented strategy.
|
||||
const MAX_RETRIES = parseInt(process.env.OCR_MAX_RETRIES, 10) || 3;
|
||||
const SUCCESS_DELAY = parseInt(process.env.OCR_SUCCESS_DELAY, 10) || 2000; // delay after successful post
|
||||
const FAILURE_DELAY = parseInt(process.env.OCR_FAILURE_DELAY, 10) || 1000; // delay after non-retryable failure
|
||||
const LOW_REMAINING_THRESHOLD = parseInt(process.env.OCR_LOW_REMAINING_THRESHOLD, 10) || 3;
|
||||
const LOW_REMAINING_SPACING = parseInt(process.env.OCR_LOW_REMAINING_SPACING, 10) || 10000;
|
||||
|
||||
// If the batch itself was rate-limited, honor its rate-limit headers
|
||||
// (retry-after / x-ratelimit-reset) before retrying per-comment,
|
||||
// otherwise the first per-comment call re-hits the same wall immediately.
|
||||
const batchRetry = computeRetryDelayMs(e, 0);
|
||||
if (batchRetry != null) {
|
||||
const secs = (batchRetry.delayMs / 1000).toFixed(1);
|
||||
console.log(
|
||||
`Batch createReview was rate-limited (HTTP ${e.status}). ` +
|
||||
`Cooling down ${secs}s via '${batchRetry.source}' (${batchRetry.detail}) before per-comment retry.`
|
||||
);
|
||||
await sleep(batchRetry.delayMs);
|
||||
}
|
||||
|
||||
for (const { comment, reviewComment } of reviewComments) {
|
||||
try {
|
||||
await github.rest.pulls.createReview({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
pull_number: prNumber,
|
||||
commit_id: commitSha,
|
||||
body: '',
|
||||
event: 'COMMENT',
|
||||
comments: [reviewComment]
|
||||
});
|
||||
successCount++;
|
||||
console.log(`Successfully posted comment for ${reviewComment.path}`);
|
||||
} catch (innerE) {
|
||||
failedCount++;
|
||||
failedComments.push({ comment, error: innerE.message });
|
||||
console.log(`Failed to post comment for ${reviewComment.path}: ${innerE.message}`);
|
||||
let posted = false;
|
||||
for (let attempt = 0; attempt <= MAX_RETRIES && !posted; attempt++) {
|
||||
try {
|
||||
const res = await github.rest.pulls.createReview({
|
||||
owner: context.repo.owner,
|
||||
repo: context.repo.repo,
|
||||
pull_number: prNumber,
|
||||
commit_id: commitSha,
|
||||
body: '',
|
||||
event: 'COMMENT',
|
||||
comments: [reviewComment]
|
||||
});
|
||||
successCount++;
|
||||
posted = true;
|
||||
console.log(`Successfully posted comment for ${reviewComment.path}`);
|
||||
// Proactive throttle: if remaining quota is low, slow down to
|
||||
// avoid hitting the limit (GitHub best practice: watch the header).
|
||||
const remaining = logRateLimitQuota(res, `after ${reviewComment.path}`);
|
||||
const lowQuota = remaining != null && remaining <= LOW_REMAINING_THRESHOLD;
|
||||
if (lowQuota) {
|
||||
console.log(`[rate-limit] quota low (remaining=${remaining} <= ${LOW_REMAINING_THRESHOLD}); increasing spacing to ${LOW_REMAINING_SPACING}ms.`);
|
||||
await sleep(LOW_REMAINING_SPACING);
|
||||
} else {
|
||||
await sleep(SUCCESS_DELAY);
|
||||
}
|
||||
} catch (innerE) {
|
||||
// Decide whether to retry and how long to wait, based on GitHub's
|
||||
// rate-limit documentation (retry-after / x-ratelimit-* headers).
|
||||
const retryInfo = computeRetryDelayMs(innerE, attempt);
|
||||
const willRetry = retryInfo != null && attempt < MAX_RETRIES;
|
||||
if (willRetry) {
|
||||
const secs = (retryInfo.delayMs / 1000).toFixed(1);
|
||||
console.log(
|
||||
`Rate-limited/transient error on ${reviewComment.path} ` +
|
||||
`(HTTP ${innerE.status}, attempt ${attempt + 1}/${MAX_RETRIES}). ` +
|
||||
`Waiting ${secs}s via '${retryInfo.source}' (${retryInfo.detail}). ` +
|
||||
`Error: ${innerE.message}`
|
||||
);
|
||||
await sleep(retryInfo.delayMs);
|
||||
} else {
|
||||
failedCount++;
|
||||
failedComments.push({ comment, error: innerE.message });
|
||||
const reason = retryInfo == null ? 'non-retryable error' : 'rate-limit retries exhausted';
|
||||
console.log(`Failed to post comment for ${reviewComment.path} (${reason}, HTTP ${innerE.status || 'n/a'}): ${innerE.message}`);
|
||||
// After exhausting retries use the success-style pace delay;
|
||||
// for other errors use the shorter failure pace delay.
|
||||
await sleep(retryInfo == null ? FAILURE_DELAY : SUCCESS_DELAY);
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -288,6 +374,115 @@ jobs:
|
|||
});
|
||||
}
|
||||
|
||||
function sleep(ms) {
|
||||
return new Promise(resolve => setTimeout(resolve, ms));
|
||||
}
|
||||
|
||||
// Case-insensitive header lookup. Octokit normalizes response headers to
|
||||
// lowercase, but this defensive check also handles original casing so that
|
||||
// quota logging and retry delay computation never silently miss a header.
|
||||
function getHeader(headers, name) {
|
||||
const v = headers[name] != null ? headers[name] : headers[name.toLowerCase()];
|
||||
return v != null ? String(v).trim() : undefined;
|
||||
}
|
||||
|
||||
// Decide whether an error is worth retrying and, if so, how long to wait.
|
||||
// Implements GitHub's documented rate-limit retry strategy using the
|
||||
// response headers (retry-after, x-ratelimit-remaining, x-ratelimit-reset).
|
||||
// Returns { delayMs, source, detail } when retryable, or null otherwise.
|
||||
// See: https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api
|
||||
function computeRetryDelayMs(error, attempt) {
|
||||
if (!error) return null;
|
||||
const status = error.status;
|
||||
const message = String(error.message || '');
|
||||
const isRateLimit = status === 429 || (status === 403 && /rate limit|abuse|secondary/i.test(message));
|
||||
const isTransient = (status >= 500 && status < 600) || status === 408;
|
||||
if (!isRateLimit && !isTransient) return null;
|
||||
|
||||
const headers = ((error.response || {}).headers) || {};
|
||||
const header = (name) => getHeader(headers, name);
|
||||
const nowSec = Math.floor(Date.now() / 1000);
|
||||
|
||||
// The absolute maximum wait for any single retry. Header-derived waits
|
||||
// (retry-after / x-ratelimit-reset) are GitHub's recommended durations,
|
||||
// but capping them prevents a far-future reset from stalling the CI job
|
||||
// past its timeout. When we cap, the next retry may re-hit the limit.
|
||||
const cap = parseInt(process.env.OCR_RETRY_MAX_DELAY, 10) || 300000;
|
||||
const base = parseInt(process.env.OCR_RETRY_BASE_DELAY, 10) || 60000;
|
||||
|
||||
// { rawMs, source, detail } describing the recommended wait before cap.
|
||||
let info = null;
|
||||
|
||||
if (isRateLimit) {
|
||||
// (1) Honor "retry-after" when present (seconds, or an HTTP-date).
|
||||
const retryAfter = header('retry-after');
|
||||
if (retryAfter) {
|
||||
const secs = Number(retryAfter);
|
||||
if (!isNaN(secs) && secs >= 0) {
|
||||
info = { rawMs: secs * 1000, source: 'retry-after', detail: `${secs}s (from header)` };
|
||||
} else {
|
||||
const dateMs = Date.parse(retryAfter);
|
||||
if (!isNaN(dateMs)) {
|
||||
info = { rawMs: Math.max(0, dateMs - Date.now()), source: 'retry-after (HTTP-date)', detail: retryAfter };
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// (2) Primary limit exhausted (x-ratelimit-remaining=0): wait until reset.
|
||||
if (!info) {
|
||||
const remaining = header('x-ratelimit-remaining');
|
||||
const reset = header('x-ratelimit-reset');
|
||||
if (reset != null && Number(remaining) === 0) {
|
||||
const rawMs = Math.max(0, Number(reset) - nowSec) * 1000;
|
||||
info = { rawMs, source: 'x-ratelimit-reset', detail: `remaining=0, reset epoch=${reset} (in ${Math.ceil(rawMs / 1000)}s)` };
|
||||
}
|
||||
}
|
||||
|
||||
// (3) Secondary limit with no retry hint: docs say wait at least one
|
||||
// minute, then increase exponentially between retries.
|
||||
if (!info) {
|
||||
const backoff = Math.min(base * Math.pow(2, attempt), cap);
|
||||
const jitter = Math.floor(Math.random() * 1000);
|
||||
info = { rawMs: backoff + jitter, source: 'exponential-backoff', detail: `base=${base}ms*2^${attempt} (cap ${cap}ms) +${jitter}ms jitter` };
|
||||
}
|
||||
} else {
|
||||
// Transient server error (5xx / 408): back off without the 60s floor.
|
||||
// Use a shorter base than the rate-limit path: server hiccups are
|
||||
// typically short-lived, so a 2s initial wait (doubling per retry)
|
||||
// is sufficient and avoids stalling the CI job unnecessarily.
|
||||
const transientBase = 2000;
|
||||
const backoff = Math.min(transientBase * Math.pow(2, attempt), cap);
|
||||
const jitter = Math.floor(Math.random() * 1000);
|
||||
info = { rawMs: backoff + jitter, source: 'transient-backoff', detail: `base=${transientBase}ms*2^${attempt} (cap ${cap}ms) +${jitter}ms jitter (HTTP ${status})` };
|
||||
}
|
||||
|
||||
// Apply the universal cap to header-derived waits too.
|
||||
const delayMs = Math.min(info.rawMs, cap);
|
||||
if (delayMs < info.rawMs) {
|
||||
info.detail += ` [CAPPED to ${cap}ms; GitHub recommended ${Math.ceil(info.rawMs / 1000)}s]`;
|
||||
}
|
||||
return { delayMs, source: info.source, detail: info.detail };
|
||||
}
|
||||
|
||||
// Best-effort logging of remaining rate-limit quota from a successful response.
|
||||
// Returns the parsed x-ratelimit-remaining value (or null) for proactive throttling.
|
||||
function logRateLimitQuota(response, tag) {
|
||||
try {
|
||||
const h = (response && response.headers) || {};
|
||||
const header = (name) => getHeader(h, name);
|
||||
const remaining = header('x-ratelimit-remaining');
|
||||
const limit = header('x-ratelimit-limit');
|
||||
const reset = header('x-ratelimit-reset');
|
||||
if (remaining != null) {
|
||||
console.log(
|
||||
`[rate-limit] ${tag}: remaining=${remaining}/${limit != null ? limit : '?'}` +
|
||||
(reset != null ? `, reset epoch=${reset}` : '')
|
||||
);
|
||||
}
|
||||
return remaining != null ? Number(remaining) : null;
|
||||
} catch (_) { return null; }
|
||||
}
|
||||
|
||||
function formatComment(comment) {
|
||||
let body = comment.content || '';
|
||||
|
||||
|
|
|
|||
|
|
@ -13,6 +13,17 @@
|
|||
# GITLAB_API_TOKEN - GitLab Personal/Project Access Token with "api" scope
|
||||
# (falls back to CI_JOB_TOKEN if not set)
|
||||
#
|
||||
# Optional CI/CD Variables (for retry/delay tuning):
|
||||
# OCR_RETRY_BASE_DELAY - Base delay (ms) for exponential backoff on rate-limit retries (default: 2000)
|
||||
# OCR_MAX_RETRIES - Max retry attempts per comment when rate-limited or transient error (default: 3)
|
||||
# OCR_MAX_RETRY_DELAY - Maximum delay (ms) per single retry, caps Retry-After/backoff (default: 60000)
|
||||
# OCR_SUCCESS_DELAY - Delay (ms) after a successful comment post to pace requests (default: 2000)
|
||||
# OCR_FAILURE_DELAY - Delay (ms) after a non-rate-limit failure to pace subsequent requests (default: 1000)
|
||||
# OCR_RATE_LIMIT_THRESHOLD - Proactively slow down when GitLab RateLimit-Remaining is at/below this value (default: 10, set 0 to disable)
|
||||
#
|
||||
# Note: The pipeline also configures llm.extra_body to '{"thinking": {"type": "disabled"}}'
|
||||
# to disable thinking mode for compatibility with various LLM providers.
|
||||
#
|
||||
# Fork MR Support:
|
||||
# The script uses CI_COMMIT_SHA as the diff target to correctly resolve the
|
||||
# source commit from forked repos. For some GitLab versions, you may need to enable:
|
||||
|
|
@ -34,6 +45,7 @@ code-review:
|
|||
script:
|
||||
# Install OpenCodeReview
|
||||
- npm install -g @alibaba-group/open-code-review
|
||||
- echo "OpenCodeReview installed with version:" && ocr version || true
|
||||
|
||||
# Configure OCR
|
||||
- mkdir -p ~/.open-code-review
|
||||
|
|
@ -62,7 +74,9 @@ code-review:
|
|||
python3 << 'PYTHON_SCRIPT'
|
||||
import json
|
||||
import os
|
||||
import random
|
||||
import sys
|
||||
import time
|
||||
import urllib.request
|
||||
import urllib.error
|
||||
|
||||
|
|
@ -75,6 +89,20 @@ code-review:
|
|||
TARGET_BRANCH = os.environ["CI_MERGE_REQUEST_TARGET_BRANCH_NAME"]
|
||||
COMMIT_SHA = os.environ["CI_COMMIT_SHA"]
|
||||
|
||||
# Configurable retry/delay settings (via CI/CD variables, with sensible defaults)
|
||||
RETRY_BASE_DELAY = int(os.environ.get("OCR_RETRY_BASE_DELAY", "2000")) / 1000 # ms → seconds
|
||||
MAX_RETRIES = int(os.environ.get("OCR_MAX_RETRIES", "3"))
|
||||
MAX_RETRY_DELAY = int(os.environ.get("OCR_MAX_RETRY_DELAY", "60000")) / 1000 # ms → seconds, cap per-retry wait
|
||||
SUCCESS_DELAY = int(os.environ.get("OCR_SUCCESS_DELAY", "2000")) / 1000
|
||||
FAILURE_DELAY = int(os.environ.get("OCR_FAILURE_DELAY", "1000")) / 1000
|
||||
# Base delay (seconds) for exponential backoff on transient server errors (5xx / 408).
|
||||
# Server hiccups are typically short-lived, so a 2s initial wait (doubling per retry)
|
||||
# is sufficient and avoids stalling the CI job unnecessarily.
|
||||
TRANSIENT_BASE_DELAY = 2
|
||||
# Proactive throttling: when RateLimit-Remaining drops to/below this threshold,
|
||||
# the script doubles the pacing delay to avoid hitting 429. Set to 0 to disable.
|
||||
RATE_LIMIT_THRESHOLD = int(os.environ.get("OCR_RATE_LIMIT_THRESHOLD", "10"))
|
||||
|
||||
if not API_TOKEN:
|
||||
print("ERROR: No API token available (GITLAB_API_TOKEN or CI_JOB_TOKEN). Cannot post comments.", file=sys.stderr)
|
||||
sys.exit(1)
|
||||
|
|
@ -85,25 +113,87 @@ code-review:
|
|||
USE_JOB_TOKEN = not os.environ.get("GITLAB_API_TOKEN")
|
||||
AUTH_HEADER = "JOB-TOKEN" if USE_JOB_TOKEN else "PRIVATE-TOKEN"
|
||||
|
||||
def api_request(endpoint, data=None, method="POST"):
|
||||
"""Make a GitLab API request."""
|
||||
url = f"{API_BASE}{endpoint}"
|
||||
headers = {
|
||||
AUTH_HEADER: API_TOKEN,
|
||||
"Content-Type": "application/json"
|
||||
}
|
||||
body = json.dumps(data).encode("utf-8") if data else None
|
||||
req = urllib.request.Request(url, data=body, headers=headers, method=method)
|
||||
def _get_header(headers, name):
|
||||
"""Case-insensitive header lookup.
|
||||
|
||||
urllib normalizes response header keys to title-case (e.g. 'Retry-After'),
|
||||
but this defensive check also handles original casing so that retry delay
|
||||
computation and quota logging never silently miss a header.
|
||||
"""
|
||||
if name in headers:
|
||||
val = headers[name]
|
||||
elif name.lower() in headers:
|
||||
val = headers[name.lower()]
|
||||
else:
|
||||
return None
|
||||
return str(val).strip() if val is not None else None
|
||||
|
||||
def _parse_rate_limit_header(headers, name):
|
||||
"""Safely parse a rate-limit response header (e.g. RateLimit-Remaining) as an int."""
|
||||
val = _get_header(headers, name)
|
||||
if val is None:
|
||||
return None
|
||||
try:
|
||||
with urllib.request.urlopen(req) as resp:
|
||||
return json.loads(resp.read().decode("utf-8"))
|
||||
except urllib.error.HTTPError as e:
|
||||
print(f"API error {e.code}: {e.read().decode('utf-8')}", file=sys.stderr)
|
||||
return int(val)
|
||||
except (ValueError, TypeError):
|
||||
return None
|
||||
|
||||
def api_request_with_retry(endpoint, data=None, method="POST"):
|
||||
"""Make a GitLab API request with retry on rate-limit errors.
|
||||
Returns a dict: {'success': bool, 'data': response or None,
|
||||
'is_rate_limit_exhausted': bool, 'rate_limit_remaining': int or None}
|
||||
"""
|
||||
for attempt in range(MAX_RETRIES + 1):
|
||||
url = f"{API_BASE}{endpoint}"
|
||||
headers = {
|
||||
AUTH_HEADER: API_TOKEN,
|
||||
"Content-Type": "application/json"
|
||||
}
|
||||
body = json.dumps(data).encode("utf-8") if data else None
|
||||
req = urllib.request.Request(url, data=body, headers=headers, method=method)
|
||||
try:
|
||||
with urllib.request.urlopen(req) as resp:
|
||||
resp_data = json.loads(resp.read().decode("utf-8"))
|
||||
remaining = _parse_rate_limit_header(resp.headers, 'RateLimit-Remaining')
|
||||
limit = _parse_rate_limit_header(resp.headers, 'RateLimit-Limit')
|
||||
if remaining is not None and limit is not None:
|
||||
print(f"RateLimit: {remaining}/{limit} remaining for {endpoint}", file=sys.stderr)
|
||||
return {'success': True, 'data': resp_data, 'is_rate_limit_exhausted': False, 'rate_limit_remaining': remaining}
|
||||
except urllib.error.HTTPError as e:
|
||||
error_body = e.read().decode('utf-8')
|
||||
is_rate_limit = e.code == 429 or (e.code == 403 and any(kw in error_body.lower() for kw in ['retry later', 'rate limit', 'too many requests', 'abuse']))
|
||||
# Transient server errors (5xx) and request timeouts (408) are worth retrying
|
||||
# with a short exponential backoff, since they are typically short-lived.
|
||||
is_transient = (500 <= e.code < 600) or e.code == 408
|
||||
rl_remaining = _parse_rate_limit_header(e.headers, 'RateLimit-Remaining')
|
||||
if (is_rate_limit or is_transient) and attempt < MAX_RETRIES:
|
||||
retry_after = _get_header(e.headers, 'Retry-After')
|
||||
if retry_after:
|
||||
try:
|
||||
delay = float(retry_after)
|
||||
except ValueError:
|
||||
delay = RETRY_BASE_DELAY * (2 ** attempt) # fallback if Retry-After is not numeric
|
||||
elif is_transient:
|
||||
# Transient server error: use a shorter base than the rate-limit path.
|
||||
# Server hiccups are typically short-lived, so a 2s initial wait
|
||||
# (doubling per retry) is sufficient.
|
||||
delay = TRANSIENT_BASE_DELAY * (2 ** attempt)
|
||||
else:
|
||||
delay = RETRY_BASE_DELAY * (2 ** attempt)
|
||||
delay = min(delay, MAX_RETRY_DELAY) # cap to prevent long stalls
|
||||
delay = delay * (0.75 + random.random() * 0.5) # ±25% jitter to avoid thundering herd
|
||||
rl_info = f" (RateLimit-Remaining: {rl_remaining})" if rl_remaining is not None else ""
|
||||
reason = "rate limit" if is_rate_limit else f"transient error (HTTP {e.code})"
|
||||
print(f"{reason} hit for {endpoint}, retrying in {delay:.1f}s (attempt {attempt + 1}/{MAX_RETRIES}){rl_info}", file=sys.stderr)
|
||||
time.sleep(delay)
|
||||
else:
|
||||
print(f"API error {e.code}: {error_body}", file=sys.stderr)
|
||||
return {'success': False, 'data': None, 'is_rate_limit_exhausted': is_rate_limit, 'rate_limit_remaining': rl_remaining}
|
||||
return {'success': False, 'data': None, 'is_rate_limit_exhausted': False, 'rate_limit_remaining': None}
|
||||
|
||||
def post_note(body):
|
||||
"""Post a general note/comment on the MR."""
|
||||
return api_request("/notes", {"body": body})
|
||||
"""Post a general note/comment on the MR (with rate-limit retry)."""
|
||||
return api_request_with_retry("/notes", {"body": body})
|
||||
|
||||
def post_discussion(path, line, body, base_sha, start_sha, head_sha):
|
||||
"""Post an inline discussion on a specific file/line in the MR diff."""
|
||||
|
|
@ -120,7 +210,7 @@ code-review:
|
|||
"body": body,
|
||||
"position": position
|
||||
}
|
||||
return api_request("/discussions", data)
|
||||
return api_request_with_retry("/discussions", data)
|
||||
|
||||
def format_comment(comment):
|
||||
"""Format a single review comment as markdown."""
|
||||
|
|
@ -184,13 +274,11 @@ code-review:
|
|||
print("No review comments to post.")
|
||||
sys.exit(0)
|
||||
|
||||
# Get MR diff metadata for position calculation
|
||||
# Get MR diff metadata for position calculation (uses retry to avoid single-point-of-failure)
|
||||
diff_refs = None
|
||||
try:
|
||||
versions_url = f"{API_BASE}/versions"
|
||||
req = urllib.request.Request(versions_url, headers={AUTH_HEADER: API_TOKEN})
|
||||
with urllib.request.urlopen(req) as resp:
|
||||
versions = json.loads(resp.read().decode("utf-8"))
|
||||
versions_resp = api_request_with_retry("/versions", method="GET")
|
||||
if versions_resp and versions_resp.get('success'):
|
||||
versions = versions_resp.get('data', [])
|
||||
if versions:
|
||||
latest = versions[0]
|
||||
diff_refs = {
|
||||
|
|
@ -198,8 +286,8 @@ code-review:
|
|||
"start_sha": latest.get("start_commit_sha", ""),
|
||||
"head_sha": latest.get("head_commit_sha", ""),
|
||||
}
|
||||
except Exception as e:
|
||||
print(f"Warning: Could not fetch MR versions: {e}", file=sys.stderr)
|
||||
if not diff_refs:
|
||||
print("Warning: Could not fetch MR versions. Inline comments will use fallback.", file=sys.stderr)
|
||||
|
||||
# Post inline discussions for each comment
|
||||
success_count = 0
|
||||
|
|
@ -216,10 +304,22 @@ code-review:
|
|||
continue
|
||||
|
||||
result_resp = post_discussion(path, end_line, body, **diff_refs)
|
||||
if result_resp:
|
||||
if result_resp and result_resp.get('success'):
|
||||
success_count += 1
|
||||
# Proactive throttling: slow down when GitLab reports low remaining quota
|
||||
remaining = result_resp.get('rate_limit_remaining')
|
||||
if RATE_LIMIT_THRESHOLD > 0 and remaining is not None and remaining <= RATE_LIMIT_THRESHOLD:
|
||||
pace_delay = SUCCESS_DELAY * 2
|
||||
print(f"Rate limit quota low ({remaining} remaining), increasing pacing delay to {pace_delay:.1f}s", file=sys.stderr)
|
||||
time.sleep(pace_delay)
|
||||
else:
|
||||
time.sleep(SUCCESS_DELAY) # Pace requests to avoid rate limits
|
||||
else:
|
||||
failed_comments.append(comment)
|
||||
# Apply longer delay when rate-limit retries are exhausted, shorter delay for other errors
|
||||
is_rate_limit_exhausted = result_resp.get('is_rate_limit_exhausted', False) if result_resp else False
|
||||
post_fail_delay = SUCCESS_DELAY if is_rate_limit_exhausted else FAILURE_DELAY
|
||||
time.sleep(post_fail_delay)
|
||||
|
||||
print(f"Successfully posted {success_count}/{len(comments)} inline comments.")
|
||||
|
||||
|
|
|
|||
|
|
@ -98,6 +98,21 @@ script:
|
|||
- ocr review --rule ./my-rules.json --from origin/$CI_MERGE_REQUEST_TARGET_BRANCH_NAME --to $CI_COMMIT_SHA
|
||||
```
|
||||
|
||||
### Adjust retry and delay settings
|
||||
|
||||
When posting review discussions, the script includes rate-limit handling with exponential backoff (with jitter), `Retry-After` header support, and proactive throttling based on GitLab's `RateLimit-Remaining` response header. All API requests — including summary notes and MR version fetches — use the same retry logic. See [GitLab Rate Limits](https://docs.gitlab.com/security/rate_limits/) for details on GitLab's rate-limiting policies and recommended handling. You can configure the retry and delay behavior via **CI/CD Variables** (Settings → CI/CD → Variables):
|
||||
|
||||
| Variable | Default | Description |
|
||||
|----------|---------|-------------|
|
||||
| `OCR_RETRY_BASE_DELAY` | `2000` | Base delay (ms) for exponential backoff when a rate-limit error is hit |
|
||||
| `OCR_MAX_RETRIES` | `3` | Maximum retry attempts per discussion when rate-limited |
|
||||
| `OCR_MAX_RETRY_DELAY` | `60000` | Maximum delay (ms) per single retry, caps both `Retry-After` and backoff |
|
||||
| `OCR_SUCCESS_DELAY` | `2000` | Delay (ms) after a successful discussion post to pace subsequent requests |
|
||||
| `OCR_FAILURE_DELAY` | `1000` | Delay (ms) after a non-rate-limit failure to pace subsequent requests |
|
||||
| `OCR_RATE_LIMIT_THRESHOLD` | `10` | Proactively slow down when GitLab `RateLimit-Remaining` is at/below this value (set `0` to disable) |
|
||||
|
||||
These variables are optional — if not configured, sensible defaults are used. Consider increasing delays for self-hosted GitLab instances with aggressive rate-limit configurations or for large MRs that generate numerous review comments. The `OCR_RATE_LIMIT_THRESHOLD` variable enables proactive throttling: when GitLab reports low remaining quota in the `RateLimit-Remaining` response header, the script automatically doubles the pacing delay to avoid hitting 429 errors.
|
||||
|
||||
### Limit concurrency
|
||||
|
||||
Adjust the `--concurrency` flag for large MRs to control the number of concurrent LLM requests:
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue