Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

# 3.4.1
* Retry transient 401 responses in `authed_get` by including `BadCredentialsException` in backoff retries. [#234](https://github.com/singer-io/tap-github/pull/234)
* Rationale: GitHub has documented intermittent API-side auth/routing incidents where valid requests can return temporary 401 and succeed on retry.

# 3.4.0
* Exclude 403-forbidden streams from discovery [#229](https://github.com/singer-io/tap-github/pull/229)
* Bump dependencies for compliance
Expand Down
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from setuptools import setup, find_packages

setup(name='tap-github',
version='3.4.0',
version='3.4.1',
description='Singer.io tap for extracting data from the GitHub API',
author='Stitch',
url='http://singer.io',
Expand Down
6 changes: 5 additions & 1 deletion tap_github/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,11 @@ def set_auth_in_session(self):
# pylint: disable=dangerous-default-value
# During 'Timeout' error there is also possibility of 'ConnectionError',
# hence added backoff for 'ConnectionError' too.
@backoff.on_exception(backoff.expo, (requests.Timeout, requests.ConnectionError, Server5xxError, TooManyRequests), max_tries=5, factor=2)
# GitHub has documented intermittent transient 401 responses where retrying
# eventually succeeds during API-side routing incidents. Retrying
# BadCredentialsException helps discovery/sync survive these short-lived
# faults while still failing after max_tries for persistent invalid tokens.
@backoff.on_exception(backoff.expo, (requests.Timeout, requests.ConnectionError, Server5xxError, TooManyRequests, BadCredentialsException), max_tries=5, factor=2)
def authed_get(self, source, url, headers={}, stream="", should_skip_404 = True):
"""
Call rest API and return the response in case of status code 200.
Expand Down
5 changes: 3 additions & 2 deletions tests/unittests/test_exception_handling.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def test_json_decoder_error(self, mocked_parse_args, mocked_request, mock_verify

@parameterized.expand([
[400, "The request is missing or has a bad parameter.", BadRequestException, '', {}, 1],
[401, "Invalid authorization credentials.", BadCredentialsException, '', {}, 1],
[401, "Invalid authorization credentials.", BadCredentialsException, '', {}, 5],
[403, "User doesn't have permission to access the resource.", AuthException, '', {}, 1],
[500, "An error has occurred at Github's end.", InternalServerError, '', {}, 5],
[301, "The resource you are looking for is moved to another URL.", tap_github.client.MovedPermanentlyError, '', {}, 1],
Expand All @@ -76,7 +76,8 @@ def test_json_decoder_error(self, mocked_parse_args, mocked_request, mock_verify
def test_error_message_and_call_count(self, mocked_parse_args, mocked_request, mock_verify_access, mock_sleep, erro_code, error_msg, error_class, content, json_msg, call_count):
"""
- Verify that `authed_get` raises an error with the proper message for different error codes.
- Verify that tap retries 5 times for Server5xxError and RateLimitExceeded error.
- Verify that tap retries 5 times for Server5xxError, TooManyRequests,
and transient 401 BadCredentialsException.
"""
Comment on lines 77 to 81

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the fix

mocked_request.return_value = get_response(erro_code, json = json_msg, raise_error = True, content = content)
test_client = GithubClient(self.config)
Expand Down