Skip to content

Commit

Permalink
♻️ separate retry logic
Browse files Browse the repository at this point in the history
  • Loading branch information
yanyongyu committed Jan 16, 2024
1 parent 2b7c72c commit 2267132
Show file tree
Hide file tree
Showing 5 changed files with 102 additions and 50 deletions.
20 changes: 16 additions & 4 deletions githubkit/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

import httpx

from .typing import RetryHandler
from .retry import RETRY_DEFAULT
from .typing import RetryDecisionFunc


@dataclass(frozen=True)
Expand All @@ -15,7 +16,7 @@ class Config:
follow_redirects: bool
timeout: httpx.Timeout
http_cache: bool
auto_retry: Union[bool, RetryHandler]
auto_retry: Optional[RetryDecisionFunc]

def dict(self) -> Dict[str, Any]:
return asdict(self)
Expand Down Expand Up @@ -62,6 +63,17 @@ def build_timeout(
return timeout if isinstance(timeout, httpx.Timeout) else httpx.Timeout(timeout)


def build_auto_retry(
auto_retry: Union[bool, RetryDecisionFunc] = True
) -> Optional[RetryDecisionFunc]:
if auto_retry is True:
return RETRY_DEFAULT
elif auto_retry:
return auto_retry

Check warning on line 72 in githubkit/config.py

View check run for this annotation

Codecov / codecov/patch

githubkit/config.py#L71-L72

Added lines #L71 - L72 were not covered by tests
else:
return None

Check warning on line 74 in githubkit/config.py

View check run for this annotation

Codecov / codecov/patch

githubkit/config.py#L74

Added line #L74 was not covered by tests


def get_config(
base_url: Optional[Union[str, httpx.URL]] = None,
accept_format: Optional[str] = None,
Expand All @@ -70,7 +82,7 @@ def get_config(
follow_redirects: bool = True,
timeout: Optional[Union[float, httpx.Timeout]] = None,
http_cache: bool = True,
auto_retry: Union[bool, RetryHandler] = True,
auto_retry: Union[bool, RetryDecisionFunc] = True,
) -> Config:
return Config(
build_base_url(base_url),
Expand All @@ -79,5 +91,5 @@ def get_config(
follow_redirects,
build_timeout(timeout),
http_cache,
auto_retry,
build_auto_retry(auto_retry),
)
48 changes: 7 additions & 41 deletions githubkit/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
import httpx
import hishel

from .log import logger
from .response import Response
from .compat import to_jsonable_python
from .config import Config, get_config
Expand All @@ -31,18 +30,16 @@
URLTypes,
CookieTypes,
HeaderTypes,
RetryOption,
ContentTypes,
RequestFiles,
RetryHandler,
QueryParamTypes,
RetryDecisionFunc,
)
from .exception import (
RequestError,
RequestFailed,
RequestTimeout,
GitHubException,
RateLimitExceeded,
PrimaryRateLimitExceeded,
SecondaryRateLimitExceeded,
)
Expand Down Expand Up @@ -95,7 +92,7 @@ def __init__(
follow_redirects: bool = True,
timeout: Optional[Union[float, httpx.Timeout]] = None,
http_cache: bool = True,
auto_retry: Union[bool, RetryHandler] = True,
auto_retry: Union[bool, RetryDecisionFunc] = True,
):
...

Expand All @@ -112,7 +109,7 @@ def __init__(
follow_redirects: bool = True,
timeout: Optional[Union[float, httpx.Timeout]] = None,
http_cache: bool = True,
auto_retry: Union[bool, RetryHandler] = True,
auto_retry: Union[bool, RetryDecisionFunc] = True,
):
...

Expand All @@ -129,7 +126,7 @@ def __init__(
follow_redirects: bool = True,
timeout: Optional[Union[float, httpx.Timeout]] = None,
http_cache: bool = True,
auto_retry: Union[bool, RetryHandler] = True,
auto_retry: Union[bool, RetryDecisionFunc] = True,
):
...

Expand All @@ -145,7 +142,7 @@ def __init__(
follow_redirects: bool = True,
timeout: Optional[Union[float, httpx.Timeout]] = None,
http_cache: bool = True,
auto_retry: Union[bool, RetryHandler] = True,
auto_retry: Union[bool, RetryDecisionFunc] = True,
):
auth = auth or UnauthAuthStrategy() # type: ignore
self.auth: A = ( # type: ignore
Expand Down Expand Up @@ -403,33 +400,6 @@ def _extract_retry_after(self, response: Response) -> timedelta:
# wait for at least one minute before retrying
return timedelta(seconds=60)

def _handle_retry(self, exc: GitHubException, retry_count: int) -> RetryOption:
# retry once for rate limit exceeded
# https://github.com/octokit/octokit.js/blob/450c662e4f29b63a6dbe7592ba5ef53d5fa01d88/src/octokit.ts#L34-L65
if retry_count < 1 and isinstance(exc, RateLimitExceeded):
logger.warning(
f"{exc.__class__.__name__} for request "
f"{exc.request.method} {exc.request.url}",
exc_info=exc,
)
return RetryOption(True, exc.retry_after)

# retry three times for server error
# https://github.com/octokit/plugin-retry.js/blob/3b9897a58d8b2c37cfba6a8df78e4619d346098a/src/error-request.ts#L11-L14
if (
retry_count < 3
and isinstance(exc, RequestFailed)
and exc.response.status_code >= 500
):
logger.warning(
f"Server error encountered for request "
f"{exc.request.method} {exc.request.url}",
exc_info=exc,
)
return RetryOption(True, timedelta(seconds=(retry_count + 1) ** 2))

return RetryOption(False)

# sync request and check
def request(
self,
Expand Down Expand Up @@ -462,10 +432,8 @@ def request(
)
return self._check(raw_resp, response_model, error_models)
except GitHubException as e:
if self.config.auto_retry is False:
if self.config.auto_retry is None:
raise

Check warning on line 436 in githubkit/core.py

View check run for this annotation

Codecov / codecov/patch

githubkit/core.py#L434-L436

Added lines #L434 - L436 were not covered by tests
elif self.config.auto_retry is True:
do_retry, retry_after = self._handle_retry(e, retry_count)
else:
do_retry, retry_after = self.config.auto_retry(e, retry_count)

Check warning on line 438 in githubkit/core.py

View check run for this annotation

Codecov / codecov/patch

githubkit/core.py#L438

Added line #L438 was not covered by tests

Expand Down Expand Up @@ -507,10 +475,8 @@ async def arequest(
)
return self._check(raw_resp, response_model, error_models)
except GitHubException as e:
if self.config.auto_retry is False:
if self.config.auto_retry is None:
raise

Check warning on line 479 in githubkit/core.py

View check run for this annotation

Codecov / codecov/patch

githubkit/core.py#L477-L479

Added lines #L477 - L479 were not covered by tests
elif self.config.auto_retry is True:
do_retry, retry_after = self._handle_retry(e, retry_count)
else:
do_retry, retry_after = self.config.auto_retry(e, retry_count)

Check warning on line 481 in githubkit/core.py

View check run for this annotation

Codecov / codecov/patch

githubkit/core.py#L481

Added line #L481 was not covered by tests

Expand Down
8 changes: 4 additions & 4 deletions githubkit/github.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@
from .core import GitHubCore
from .response import Response
from .paginator import Paginator
from .typing import RetryHandler
from .auth import BaseAuthStrategy
from .typing import RetryDecisionFunc
from .versions import RestVersionSwitcher, WebhooksVersionSwitcher
from .graphql import GraphQLResponse, build_graphql_request, parse_graphql_response

Expand Down Expand Up @@ -86,7 +86,7 @@ def __init__(
follow_redirects: bool = True,
timeout: Optional[Union[float, httpx.Timeout]] = None,
http_cache: bool = True,
auto_retry: Union[bool, RetryHandler] = True,
auto_retry: Union[bool, RetryDecisionFunc] = True,
):
...

Expand All @@ -103,7 +103,7 @@ def __init__(
follow_redirects: bool = True,
timeout: Optional[Union[float, httpx.Timeout]] = None,
http_cache: bool = True,
auto_retry: Union[bool, RetryHandler] = True,
auto_retry: Union[bool, RetryDecisionFunc] = True,
):
...

Expand All @@ -120,7 +120,7 @@ def __init__(
follow_redirects: bool = True,
timeout: Optional[Union[float, httpx.Timeout]] = None,
http_cache: bool = True,
auto_retry: Union[bool, RetryHandler] = True,
auto_retry: Union[bool, RetryDecisionFunc] = True,
):
...

Expand Down
74 changes: 74 additions & 0 deletions githubkit/retry.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
from datetime import timedelta

from .log import logger
from .typing import RetryOption as RetryOption
from .typing import RetryDecisionFunc as RetryDecisionFunc
from .exception import RequestFailed, GitHubException, RateLimitExceeded


class RetryRateLimit:
__slots__ = ("max_retry_count",)

def __init__(self, max_retry_count: int = 1) -> None:
self.max_retry_count = max_retry_count

def __call__(self, exc: GitHubException, retry_count: int) -> RetryOption:
# retry once for rate limit exceeded
# https://github.com/octokit/octokit.js/blob/450c662e4f29b63a6dbe7592ba5ef53d5fa01d88/src/octokit.ts#L34-L65
if retry_count < self.max_retry_count and isinstance(exc, RateLimitExceeded):
logger.warning(

Check warning on line 19 in githubkit/retry.py

View check run for this annotation

Codecov / codecov/patch

githubkit/retry.py#L18-L19

Added lines #L18 - L19 were not covered by tests
f"{exc.__class__.__name__} for request "
f"{exc.request.method} {exc.request.url}",
exc_info=exc,
)
return RetryOption(True, exc.retry_after)

Check warning on line 24 in githubkit/retry.py

View check run for this annotation

Codecov / codecov/patch

githubkit/retry.py#L24

Added line #L24 was not covered by tests

return RetryOption(False)

Check warning on line 26 in githubkit/retry.py

View check run for this annotation

Codecov / codecov/patch

githubkit/retry.py#L26

Added line #L26 was not covered by tests


RETRY_RATE_LIMIT = RetryRateLimit()


class RetryServerError:
__slots__ = ("max_retry_count",)

def __init__(self, max_retry_count: int = 3) -> None:
self.max_retry_count = max_retry_count

def __call__(self, exc: GitHubException, retry_count: int) -> RetryOption:
# retry three times for server error
# https://github.com/octokit/plugin-retry.js/blob/3b9897a58d8b2c37cfba6a8df78e4619d346098a/src/error-request.ts#L11-L14
if (

Check warning on line 41 in githubkit/retry.py

View check run for this annotation

Codecov / codecov/patch

githubkit/retry.py#L41

Added line #L41 was not covered by tests
retry_count < self.max_retry_count
and isinstance(exc, RequestFailed)
and exc.response.status_code >= 500
):
logger.warning(

Check warning on line 46 in githubkit/retry.py

View check run for this annotation

Codecov / codecov/patch

githubkit/retry.py#L46

Added line #L46 was not covered by tests
f"Server error encountered for request "
f"{exc.request.method} {exc.request.url}",
exc_info=exc,
)
return RetryOption(True, timedelta(seconds=(retry_count + 1) ** 2))

Check warning on line 51 in githubkit/retry.py

View check run for this annotation

Codecov / codecov/patch

githubkit/retry.py#L51

Added line #L51 was not covered by tests

return RetryOption(False)

Check warning on line 53 in githubkit/retry.py

View check run for this annotation

Codecov / codecov/patch

githubkit/retry.py#L53

Added line #L53 was not covered by tests


RETRY_SERVER_ERROR = RetryServerError()


class RetryChainDecision:
__slots__ = ("decision_funcs",)

def __init__(self, *decision_funcs: RetryDecisionFunc) -> None:
self.decision_funcs = decision_funcs

def __call__(self, exc: GitHubException, retry_count: int) -> RetryOption:
for decision_func in self.decision_funcs:
retry_option = decision_func(exc, retry_count)
if retry_option.do_retry:
return retry_option

Check warning on line 69 in githubkit/retry.py

View check run for this annotation

Codecov / codecov/patch

githubkit/retry.py#L66-L69

Added lines #L66 - L69 were not covered by tests

return RetryOption(False)

Check warning on line 71 in githubkit/retry.py

View check run for this annotation

Codecov / codecov/patch

githubkit/retry.py#L71

Added line #L71 was not covered by tests


RETRY_DEFAULT = RetryChainDecision(RETRY_RATE_LIMIT, RETRY_SERVER_ERROR)
2 changes: 1 addition & 1 deletion githubkit/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,4 @@ class RetryOption(NamedTuple):
retry_after: Optional[timedelta] = None


RetryHandler: TypeAlias = Callable[[GitHubException, int], RetryOption]
RetryDecisionFunc: TypeAlias = Callable[[GitHubException, int], RetryOption]

0 comments on commit 2267132

Please sign in to comment.