-
Notifications
You must be signed in to change notification settings - Fork 515
feat: Handle LaunchDarkly rate limiting #5501
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.
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
Changes from all commits
d59fea9
82ceeb5
46bda29
0041268
3003320
c43560e
92796b5
445ebb8
949f7bf
f8189ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,78 @@ | ||
| from typing import Any, Iterator, Optional, TypeVar | ||
| from datetime import timedelta | ||
| from typing import Any, Callable, Generator, Iterator, Optional, TypeVar | ||
|
|
||
| from requests import Session | ||
| import backoff | ||
| from backoff.types import Details | ||
| from django.utils.timezone import now as timezone_now | ||
| from requests import RequestException, Session | ||
| from rest_framework.status import HTTP_429_TOO_MANY_REQUESTS | ||
|
|
||
| from integrations.launch_darkly import types as ld_types | ||
| from integrations.launch_darkly.constants import ( | ||
| BACKOFF_MAX_RETRIES, | ||
| LAUNCH_DARKLY_API_BASE_URL, | ||
| LAUNCH_DARKLY_API_ITEM_COUNT_LIMIT_PER_PAGE, | ||
| LAUNCH_DARKLY_API_VERSION, | ||
| ) | ||
| from integrations.launch_darkly.exceptions import LaunchDarklyRateLimitError | ||
|
|
||
| T = TypeVar("T") | ||
|
|
||
|
|
||
| def launch_darkly_backoff( | ||
| _get_json_response: Callable[..., T], | ||
| ) -> Callable[..., T]: | ||
| # Handle LaunchDarkly rate limiting according to their API documentation: | ||
| # https://launchdarkly.com/docs/api | ||
| # | ||
| # 1. If a request returns a 429 Too Many Requests status code, the client should back off. | ||
| # 2. When backing off, the client retries the request after the time specified in the `Retry-After` header | ||
| # or the `X-Ratelimit-Reset` header, if present, or a default of `BACKOFF_DEFAULT_RETRY_SECONDS`. | ||
| # 3. After `BACKOFF_MAX_RETRIES` retries, we give up. | ||
| # If the last error was a 429 and contained retry information, | ||
| # signal for the import request to be retried later by raising a `LaunchDarklyRateLimitError`. | ||
|
|
||
| def _get_retry_after(exc: RequestException) -> float | None: | ||
| if ( | ||
| (response := exc.response) is not None | ||
| ) and response.status_code == HTTP_429_TOO_MANY_REQUESTS: | ||
| headers = response.headers | ||
| if retry_after := headers.get("Retry-After"): | ||
| return float(retry_after) | ||
| if ratelimit_reset := headers.get("X-Ratelimit-Reset"): | ||
| return float(ratelimit_reset) - timezone_now().timestamp() | ||
| return None | ||
|
|
||
| def _wait_gen() -> Generator[float, None, None]: | ||
| exc: RequestException = yield # type: ignore[misc,assignment] | ||
|
|
||
| while True: | ||
| if retry_after := _get_retry_after(exc): | ||
| yield retry_after | ||
| else: | ||
| return | ||
|
|
||
| def _handle_giveup( | ||
| details: Details, | ||
| ) -> None: | ||
| exc: RequestException = details["exception"] # type: ignore[typeddict-item] | ||
|
|
||
| if retry_after := _get_retry_after(exc): | ||
| raise LaunchDarklyRateLimitError( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It could be nice to log the project/org maybe if it's simple to access them here ?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in 949f7bf. |
||
| retry_at=timezone_now() + timedelta(seconds=retry_after) | ||
| ) | ||
|
|
||
| raise exc | ||
|
|
||
| return backoff.on_exception( | ||
| wait_gen=_wait_gen, | ||
| exception=RequestException, | ||
| jitter=backoff.random_jitter, | ||
| on_giveup=_handle_giveup, | ||
| max_tries=BACKOFF_MAX_RETRIES, | ||
| )(_get_json_response) | ||
|
|
||
|
|
||
| class LaunchDarklyClient: | ||
| def __init__(self, token: str) -> None: | ||
| client_session = Session() | ||
|
|
@@ -23,6 +84,7 @@ def __init__(self, token: str) -> None: | |
| ) | ||
| self.client_session = client_session | ||
|
|
||
| @launch_darkly_backoff | ||
| def _get_json_response( | ||
| self, | ||
| endpoint: str, | ||
|
|
@@ -53,7 +115,7 @@ def _iter_paginated_items( | |
| if additional_params: | ||
| params.update(additional_params) | ||
|
|
||
| response_json = self._get_json_response( # type: ignore[var-annotated] | ||
| response_json: dict[str, Any] = self._get_json_response( | ||
| endpoint=collection_endpoint, | ||
| params=params, | ||
| ) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| from datetime import datetime | ||
|
|
||
|
|
||
| class LaunchDarklyAPIError(Exception): | ||
| """Base exception for LaunchDarkly integration errors.""" | ||
|
|
||
|
|
||
| class LaunchDarklyRateLimitError(LaunchDarklyAPIError): | ||
| """Exception raised when the LaunchDarkly API rate limit is exceeded.""" | ||
|
|
||
| def __init__(self, retry_at: datetime): | ||
| super().__init__() | ||
| self.retry_at = retry_at |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,31 @@ | ||
| import structlog | ||
| from task_processor.decorators import ( | ||
| register_task_handler, | ||
| ) | ||
| from task_processor.exceptions import TaskBackoffError | ||
|
|
||
| from integrations.launch_darkly.exceptions import LaunchDarklyRateLimitError | ||
| from integrations.launch_darkly.models import LaunchDarklyImportRequest | ||
| from integrations.launch_darkly.services import process_import_request | ||
|
|
||
| logger = structlog.get_logger("launch_darkly") | ||
|
|
||
|
|
||
| @register_task_handler() | ||
| def process_launch_darkly_import_request(import_request_id: int) -> None: | ||
| import_request = LaunchDarklyImportRequest.objects.get(id=import_request_id) | ||
| process_import_request(import_request) | ||
| log = logger.bind( | ||
| import_request_id=import_request.id, | ||
| ld_project_key=import_request.ld_project_key, | ||
| organisation_id=import_request.project.organisation_id, | ||
| project_id=import_request.project_id, | ||
| ) | ||
| try: | ||
| process_import_request(import_request) | ||
| except LaunchDarklyRateLimitError as exc: | ||
| log.warning( | ||
| "import-rate-limit-reached", | ||
| retry_at=exc.retry_at, | ||
| error_message=str(exc), | ||
| ) | ||
| raise TaskBackoffError(delay_until=exc.retry_at) from exc |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This type ignore is an annoying one, especially considering the (otherwise excellent)
backofflibrary is not maintained anymore.NB: consider switching all
backoffusage tostamina.