Retry translation API requests when rate limited#82
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements exponential backoff retry logic for translation API requests when encountering 429 rate limit errors from the Azure AI translation service.
- Adds configurable retry attempts with exponential backoff for handling rate limit errors
- Improves error handling and logging to provide better feedback on API failures
- Ensures translation operations can recover from temporary rate limiting issues
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| .github/workflows/reusable-translations.yml | Adds retry-attempts input parameter for configuring retry behavior |
| .github/scripts/translate.py | Implements retry logic with exponential backoff and enhanced error handling |
Comments suppressed due to low confidence (1)
.github/scripts/translate.py:30
- [nitpick] The variable name
max_retriesis confusing because it's used as the maximum number of attempts (including the first attempt), not the maximum number of retries. Consider renaming tomax_attemptsfor clarity.
max_retries = RETRY_ATTEMPTS
| error_data = response.json().get("error", {}) | ||
| error_message = error_data.get("message", "") | ||
| error_code = error_data.get("code", "") | ||
| except (ValueError, KeyError, json.JSONDecodeError): |
There was a problem hiding this comment.
The json module is not imported but json.JSONDecodeError is referenced. Either import the json module at the top of the file or use requests.exceptions.JSONDecodeError if available.
| except (ValueError, KeyError, json.JSONDecodeError): | |
| except (ValueError, KeyError, json.JSONDecodeError): # Ensure json.JSONDecodeError is explicitly referenced |
| try: | ||
| response = session.post(url, headers=headers, json=payload) | ||
| response.raise_for_status() | ||
| return [item["translations"][0]["text"] for item in response.json()] | ||
| except requests.exceptions.HTTPError: | ||
| status_code = response.status_code | ||
|
|
||
| try: | ||
| error_data = response.json().get("error", {}) | ||
| error_message = error_data.get("message", "") | ||
| error_code = error_data.get("code", "") |
There was a problem hiding this comment.
The response variable is accessed inside the except block, but it may not be defined if the exception occurs before the response is assigned. Move the response assignment outside the try block or handle the case where response is undefined.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
I've opened #90 to add unit tests. We should be able to introduce a test that confirms this new behavior. |
|
Moving this to draft until #90 is merged so tests can be added. |
1 participant
Proposed changes
This implements an exponential back off for retrying translation API requests when encountering 429 errors indicating rate limiting.
Fixes #80.
Note: to avoid merge conflicts, I've included the changes from #81. That should be considered first. If not merged, then this should be updated to remove the related changes.
Type of Change
Production
Development
Visual
Checklist
Further comments