Skip to content

Commit

Permalink
On github errors, retry once, and return a result or an exception
Browse files Browse the repository at this point in the history
This commit is meant to fix point 5. on analysis at https://gist.github.com/majamassarini/85033b59607a3f3da570cf5aaf2b8c7a

raise_on_status (in urllib3.Retry) meaning is:
Similar meaning to raise_on_redirect: whether we should raise an exception, or return a response, if status falls in status_forcelist range and retries have been exhausted.

I would force to have or a result or an exception for not making Celery task hanging.
  • Loading branch information
majamassarini committed Oct 2, 2024
1 parent 0042934 commit f2b5830
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 4 deletions.
5 changes: 2 additions & 3 deletions ogr/services/github/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ def __init__(
github_app_private_key_path: Optional[str] = None,
tokman_instance_url: Optional[str] = None,
github_authentication: GithubAuthentication = None,
max_retries: Union[int, Retry] = 0,
max_retries: Union[int, Retry] = 1,
**kwargs,
):
"""
Expand All @@ -69,12 +69,11 @@ def __init__(
else:
self._max_retries = Retry(
total=int(max_retries),
read=0,
# Retry mechanism active for these HTTP methods:
allowed_methods=["DELETE", "GET", "PATCH", "POST", "PUT"],
# Only retry on following HTTP status codes
status_forcelist=[500, 503, 403, 401],
raise_on_status=False,
raise_on_status=True,
)

if not self._default_auth_method:
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/test_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -465,7 +465,7 @@ def test_get_instances_from_dict_multiple_auth(instances_in_dict, result_instanc
"token": "abcd",
},
},
0,
1,
),
(
{
Expand Down

0 comments on commit f2b5830

Please sign in to comment.