fix: add retries and URL encoding to Unmanaged Keys Audit workflow (#39188)#39196
fix: add retries and URL encoding to Unmanaged Keys Audit workflow (#39188)#39196raman118 wants to merge 2 commits into
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses reliability issues in the Unmanaged Service Accounts Keys Audit workflow. By introducing a resilient request mechanism and fixing how search queries are encoded, the changes mitigate frequent failures caused by transient network issues and malformed API requests, significantly improving the stability of the automated audit process. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the service account retrieval to handle paginated results, introduces robust retry logic with exponential backoff for GitHub API requests, and refactors issue searching to use query parameters. It also adds comprehensive unit tests. The review feedback suggests several important improvements: avoiding retries on non-transient 4xx client errors to save API quota, safely parsing the 'Retry-After' header to prevent potential ValueErrors, and escaping double quotes in the issue title to avoid malformed search queries.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| except requests.exceptions.RequestException as e: | ||
| if attempt == max_retries - 1: | ||
| raise | ||
| self.logger.warning( | ||
| f"GitHub API request exception on {endpoint}: {e}. " | ||
| f"Retrying in {backoff} seconds... (Attempt {attempt + 1}/{max_retries})" | ||
| ) | ||
| time.sleep(backoff) | ||
| backoff *= 2 |
There was a problem hiding this comment.
Currently, any non-transient client error (such as 400 Bad Request, 401 Unauthorized, or 404 Not Found) will raise an HTTPError via response.raise_for_status(), which is then caught by the generic except requests.exceptions.RequestException block and retried up to 5 times.
Retrying non-transient client errors is inefficient, wastes API quota, and unnecessarily delays workflow execution since these errors will not resolve on subsequent retries. We should immediately raise the exception for non-transient client errors (4xx status codes, excluding rate limits/timeouts like 403, 408, and 429).
| except requests.exceptions.RequestException as e: | |
| if attempt == max_retries - 1: | |
| raise | |
| self.logger.warning( | |
| f"GitHub API request exception on {endpoint}: {e}. " | |
| f"Retrying in {backoff} seconds... (Attempt {attempt + 1}/{max_retries})" | |
| ) | |
| time.sleep(backoff) | |
| backoff *= 2 | |
| except requests.exceptions.RequestException as e: | |
| if isinstance(e, requests.exceptions.HTTPError) and e.response is not None: | |
| if e.response.status_code not in [403, 408, 429] and e.response.status_code < 500: | |
| raise | |
| if attempt == max_retries - 1: | |
| raise | |
| self.logger.warning( | |
| f"GitHub API request exception on {endpoint}: {e}. " | |
| f"Retrying in {backoff} seconds... (Attempt {attempt + 1}/{max_retries})" | |
| ) | |
| time.sleep(backoff) | |
| backoff *= 2 |
| retry_after = response.headers.get("Retry-After") | ||
| if retry_after: | ||
| sleep_seconds = int(retry_after) | ||
| else: | ||
| sleep_seconds = backoff |
There was a problem hiding this comment.
The Retry-After header returned by APIs can occasionally be malformed, empty, or formatted as an HTTP date string instead of an integer. Attempting to directly cast it using int(retry_after) without handling potential exceptions can lead to an unhandled ValueError that crashes the workflow.
It is safer to wrap the conversion in a try-except block and fall back to the default backoff value if parsing fails.
| retry_after = response.headers.get("Retry-After") | |
| if retry_after: | |
| sleep_seconds = int(retry_after) | |
| else: | |
| sleep_seconds = backoff | |
| retry_after = response.headers.get("Retry-After") | |
| try: | |
| sleep_seconds = int(retry_after) if retry_after else backoff | |
| except ValueError: | |
| sleep_seconds = backoff |
| """ | ||
| endpoint = f"search/issues?q=is:issue+repo:{self.github_repo}+in:title+{title}+is:open" | ||
| response = self._make_github_request("GET", endpoint) | ||
| q = f'is:issue repo:{self.github_repo} in:title "{title}" is:open' |
There was a problem hiding this comment.
If the title parameter contains double quotes (e.g., [SECURITY] Action Required: "Unmanaged" Keys), constructing the query string directly will result in a malformed GitHub search query. This can cause the search to fail or return incorrect results.
To prevent this, we should escape any double quotes in the title before embedding it in the query string.
| q = f'is:issue repo:{self.github_repo} in:title "{title}" is:open' | |
| escaped_title = title.replace('"', '\\"') | |
| q = f'is:issue repo:{self.github_repo} in:title "{escaped_title}" is:open' |
|
Assigning reviewers: R: @damccorm added as fallback since no labels match configuration Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
1 participant
Fixes #39188
The Unmanaged Service Accounts Keys Audit job was failing over 50% of the time
due to transient GitHub API failures and improperly encoded query parameters.
Changes:
These changes address the flaky behavior documented in the workflow run history
and Grafana dashboard.