From d2b2c5bc8a2aff8dd2df5f9f8cf0f8afc568e292 Mon Sep 17 00:00:00 2001 From: Vi6hal <20889199+Vi6hal@users.noreply.github.com> Date: Thu, 27 Feb 2025 04:52:09 +0000 Subject: [PATCH 01/12] added impl to retry 401 error --- tap_xero/client.py | 14 ++++++++++++-- tap_xero/context.py | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index 17386d5..ddcfeb0 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -28,6 +28,8 @@ def __init__(self, message=None, response=None): class XeroBadRequestError(XeroError): pass +class XeroTokenExpiredError(XeroError): + pass class XeroUnauthorizedError(XeroError): pass @@ -178,9 +180,10 @@ def retry_after_wait_gen(): yield math.floor(float(sleep_time_str)) class XeroClient(): - def __init__(self, config): + def __init__(self, config, config_path=None): self.session = requests.Session() self.user_agent = config.get("user_agent") + self.config_path = config_path self.tenant_id = None self.access_token = None @@ -206,9 +209,12 @@ def refresh_credentials(self, config, config_path): # Write to config file config['refresh_token'] = resp["refresh_token"] + config['access_token'] = resp["access_token"] update_config_file(config, config_path) self.access_token = resp["access_token"] self.tenant_id = config['tenant_id'] + self.config = config + LOGGER.info("access_token refreshed") @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroInternalError), max_tries=3) @@ -233,7 +239,7 @@ def check_platform_access(self, config, config_path): raise_for_error(response) - @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroInternalError), max_tries=3) + @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroInternalError, XeroTokenExpiredError), max_tries=3) @backoff.on_exception(retry_after_wait_gen, XeroTooManyInMinuteError, giveup=is_not_status_code_fn([429]), jitter=None, max_tries=3) def filter(self, tap_stream_id, since=None, **params): xero_resource_name = tap_stream_id.title().replace("_", "") @@ -249,6 +255,10 @@ def filter(self, tap_stream_id, since=None, **params): request = requests.Request("GET", url, headers=headers, params=params) response = self.session.send(request.prepare()) + if response.status_code == 401: + self.refresh_credentials(self.config, self.config_path) + raise XeroTokenExpiredError + if response.status_code != 200: raise_for_error(response) return None diff --git a/tap_xero/context.py b/tap_xero/context.py index fabf002..b0ff4d4 100644 --- a/tap_xero/context.py +++ b/tap_xero/context.py @@ -9,7 +9,7 @@ def __init__(self, config, state, catalog, config_path): self.config_path = config_path self.state = state self.catalog = catalog - self.client = XeroClient(config) + self.client = XeroClient(config, config_path=config_path) def refresh_credentials(self): self.client.refresh_credentials(self.config, self.config_path) From ccde23b51eb61e20f0b1b1a9b1fcdd8b807d56c1 Mon Sep 17 00:00:00 2001 From: Vi6hal <20889199+Vi6hal@users.noreply.github.com> Date: Wed, 12 Mar 2025 02:54:50 +0000 Subject: [PATCH 02/12] updated unit tests --- CHANGELOG.md | 3 +++ setup.py | 2 +- tap_xero/client.py | 7 +++---- tests/unittests/test_exception_handling.py | 4 +++- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6ca8ac..ec2b024 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ # Changelog +## 2.4.0 + * Added Retry and exception handling for 401 errors + ## 2.3.1 * Add new fields into the schema of credit_notes stream [#117](https://github.com/singer-io/tap-xero/pull/117) diff --git a/setup.py b/setup.py index 2380f5a..768977c 100755 --- a/setup.py +++ b/setup.py @@ -2,7 +2,7 @@ from setuptools import setup, find_packages setup(name="tap-xero", - version="2.3.1", + version="2.4.0", description="Singer.io tap for extracting data from the Xero API", author="Stitch", url="http://singer.io", diff --git a/tap_xero/client.py b/tap_xero/client.py index ddcfeb0..9fe1dde 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -28,8 +28,6 @@ def __init__(self, message=None, response=None): class XeroBadRequestError(XeroError): pass -class XeroTokenExpiredError(XeroError): - pass class XeroUnauthorizedError(XeroError): pass @@ -183,6 +181,7 @@ class XeroClient(): def __init__(self, config, config_path=None): self.session = requests.Session() self.user_agent = config.get("user_agent") + self.config = config self.config_path = config_path self.tenant_id = None self.access_token = None @@ -239,7 +238,7 @@ def check_platform_access(self, config, config_path): raise_for_error(response) - @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroInternalError, XeroTokenExpiredError), max_tries=3) + @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroInternalError, XeroUnauthorizedError), max_tries=3) @backoff.on_exception(retry_after_wait_gen, XeroTooManyInMinuteError, giveup=is_not_status_code_fn([429]), jitter=None, max_tries=3) def filter(self, tap_stream_id, since=None, **params): xero_resource_name = tap_stream_id.title().replace("_", "") @@ -257,7 +256,7 @@ def filter(self, tap_stream_id, since=None, **params): if response.status_code == 401: self.refresh_credentials(self.config, self.config_path) - raise XeroTokenExpiredError + raise_for_error(response) if response.status_code != 200: raise_for_error(response) diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py index 74feadd..6fa5a79 100644 --- a/tests/unittests/test_exception_handling.py +++ b/tests/unittests/test_exception_handling.py @@ -196,7 +196,8 @@ def test_badrequest_400_error(self, mocked_session, mocked_badrequest_400_error) @mock.patch('requests.Request', side_effect=mocked_unauthorized_401_error) - def test_unauthorized_401_error(self, mocked_session, mocked_unauthorized_401_error): + @mock.patch('tap_xero.XeroClient.refresh_credentials',) + def test_unauthorized_401_error(self, mocked_session, mocked_refresh_credentials, mocked_unauthorized_401_error): config = {} tap_stream_id = "contacts" @@ -210,6 +211,7 @@ def test_unauthorized_401_error(self, mocked_session, mocked_unauthorized_401_er expected_error_message = "HTTP-error-code: 401, Error: Invalid authorization credentials." # Verifying the message formed for the custom exception + self.assertEqual(mocked_refresh_credentials.call_count, 3) self.assertEquals(str(e), expected_error_message) pass From c4c92ddf7a127d26f214129ec33c0d570c83f2a1 Mon Sep 17 00:00:00 2001 From: Vi6hal <20889199+Vi6hal@users.noreply.github.com> Date: Wed, 26 Mar 2025 11:32:37 +0000 Subject: [PATCH 03/12] updated client and tests --- tap_xero/client.py | 18 +++-- tap_xero/context.py | 4 +- tests/unittests/test_exception_handling.py | 90 ++++++++++++++-------- 3 files changed, 73 insertions(+), 39 deletions(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index 9fe1dde..270eca5 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -178,7 +178,7 @@ def retry_after_wait_gen(): yield math.floor(float(sleep_time_str)) class XeroClient(): - def __init__(self, config, config_path=None): + def __init__(self, config, config_path): self.session = requests.Session() self.user_agent = config.get("user_agent") self.config = config @@ -186,7 +186,10 @@ def __init__(self, config, config_path=None): self.tenant_id = None self.access_token = None - def refresh_credentials(self, config, config_path): + def refresh_credentials(self): + + config = self.config + config_path = self.config_path header_token = b64encode((config["client_id"] + ":" + config["client_secret"]).encode('utf-8')) @@ -207,21 +210,22 @@ def refresh_credentials(self, config, config_path): resp = resp.json() # Write to config file + self.access_token = resp["access_token"] + self.tenant_id = config['tenant_id'] config['refresh_token'] = resp["refresh_token"] config['access_token'] = resp["access_token"] + self.config = config update_config_file(config, config_path) - self.access_token = resp["access_token"] self.tenant_id = config['tenant_id'] - self.config = config LOGGER.info("access_token refreshed") @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroInternalError), max_tries=3) @backoff.on_exception(retry_after_wait_gen, XeroTooManyInMinuteError, giveup=is_not_status_code_fn([429]), jitter=None, max_tries=3) - def check_platform_access(self, config, config_path): + def check_platform_access(self): # Validating the authentication of the provided configuration - self.refresh_credentials(config, config_path) + self.refresh_credentials() headers = { "Authorization": "Bearer " + self.access_token, @@ -255,7 +259,7 @@ def filter(self, tap_stream_id, since=None, **params): response = self.session.send(request.prepare()) if response.status_code == 401: - self.refresh_credentials(self.config, self.config_path) + self.refresh_credentials() raise_for_error(response) if response.status_code != 200: diff --git a/tap_xero/context.py b/tap_xero/context.py index b0ff4d4..bf35bee 100644 --- a/tap_xero/context.py +++ b/tap_xero/context.py @@ -12,10 +12,10 @@ def __init__(self, config, state, catalog, config_path): self.client = XeroClient(config, config_path=config_path) def refresh_credentials(self): - self.client.refresh_credentials(self.config, self.config_path) + self.client.refresh_credentials() def check_platform_access(self): - self.client.check_platform_access(self.config, self.config_path) + self.client.check_platform_access() def get_bookmark(self, path): return bks_.get_bookmark(self.state, *path) diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py index 6fa5a79..cd9264e 100644 --- a/tests/unittests/test_exception_handling.py +++ b/tests/unittests/test_exception_handling.py @@ -4,6 +4,7 @@ from unittest import mock import decimal import json +import io def mocked_session(*args, **kwargs): @@ -147,7 +148,8 @@ def test_json_decode_exception(self, mocked_session, mocked_jsondecode_failing_r config = {} tap_stream_id = "contacts" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" try: @@ -164,7 +166,8 @@ def test_normal_filter_execution(self, mocked_session, mocked_jsondecode_success config = {} tap_stream_id = "contacts" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" try: @@ -181,7 +184,8 @@ def test_badrequest_400_error(self, mocked_session, mocked_badrequest_400_error) config = {} tap_stream_id = "contacts" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -199,9 +203,10 @@ def test_badrequest_400_error(self, mocked_session, mocked_badrequest_400_error) @mock.patch('tap_xero.XeroClient.refresh_credentials',) def test_unauthorized_401_error(self, mocked_session, mocked_refresh_credentials, mocked_unauthorized_401_error): config = {} + tap_stream_id = "contacts" - - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -221,7 +226,8 @@ def test_forbidden_403_exception(self, mocked_session, mocked_forbidden_403_exce config = {} tap_stream_id = "contacts" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -240,7 +246,8 @@ def test_notfound_404_error(self, mocked_session, mocked_notfound_404_error): config = {} tap_stream_id = "contacts" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -258,7 +265,8 @@ def test_precondition_failed_412_error(self, mocked_session, mocked_precondition config = {} tap_stream_id = "contacts" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -276,7 +284,8 @@ def test_internalservererror_500_error(self, mocked_session, mocked_internalserv config = {} tap_stream_id = "contacts" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -295,7 +304,8 @@ def test_notimplemented_501_error(self, mocked_session, mocked_notimplemented_50 config = {} tap_stream_id = "contacts" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -313,7 +323,8 @@ def test_not_available_503_error(self, mocked_session, mocked_not_available_503_ config = {} tap_stream_id = "contacts" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -332,7 +343,8 @@ def test_too_many_requests_429_in_day_error(self, mocked_session, mocked_failed_ config = {} tap_stream_id = "contacts" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -352,7 +364,8 @@ def test_too_many_requests_429_in_minute_error(self, mocked_session, mocked_fail config = {} tap_stream_id = "contacts" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -372,7 +385,8 @@ def test_too_many_requests_in_day_429_not_backoff_behavior(self, mocked_session, config = {} tap_stream_id = "contacts" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" try: @@ -390,7 +404,8 @@ def test_too_many_requests_in_minute_429_backoff_behavior(self, mocked_session, config = {} tap_stream_id = "contacts" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" try: @@ -406,7 +421,8 @@ def test_internalservererror_500_backoff_behaviour(self, mocked_session, mocked_ config = {} tap_stream_id = "contacts" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" try: @@ -432,7 +448,8 @@ def test_check_unauthorized_401_error_in_discovery_mode(self, mocked_unauthorize } config_path = "" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) try: xero_client.check_platform_access(config, config_path) @@ -449,7 +466,8 @@ def test_check_forbidden_403_error_in_discovery_mode(self, mocked_refresh_creden config = {} config_path = "" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -468,7 +486,8 @@ def test_badrequest_400_error_in_discovery_mode(self, mocked_refresh_credentials config = {} config_path = "" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -487,7 +506,8 @@ def test_notfound_404_error_in_discovery_mode(self, mocked_refresh_credentials, config = {} config_path = "" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -506,7 +526,8 @@ def test_precondition_failed_412_error_in_discovery_mode(self, mocked_refresh_cr config = {} config_path = "" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -525,7 +546,8 @@ def test_internalservererror_500_error_in_discovery_mode(self, mocked_refresh_cr config = {} config_path = "" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -544,7 +566,8 @@ def test_notimplemented_501_error_in_discovery_mode(self, mocked_refresh_credent config = {} config_path = "" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -563,7 +586,8 @@ def test_not_available_503_error_in_discovery_mode(self, mocked_refresh_credenti config = {} config_path = "" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -582,7 +606,8 @@ def test_too_many_requests_in_day_429_error_in_discovery_mode(self, mocked_refre config = {} config_path = "" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -601,7 +626,8 @@ def test_too_many_requests_in_minute_429_error_in_discovery_mode(self, mocked_re config = {} config_path = "" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -620,7 +646,8 @@ def test_too_many_requests_in_day_429_not_backoff_behavior_discovery_mode(self, config = {} config_path = "" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -642,7 +669,8 @@ def test_too_many_requests_in_minute_429_backoff_behavior_discovery_mode(self, m config = {} config_path = "" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -663,7 +691,8 @@ def test_internalservererror_500_backoff_behaviour_discovery_mode(self, mocked_r config = {} config_path = "" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -691,7 +720,8 @@ def test_check_success_200_in_discovery_mode(self, mock_successful_session_post, } config_path = "" - xero_client = client_.XeroClient(config) + config_path = io.StringIO() + xero_client = client_.XeroClient(config, config_path) expected_access_token = "123" expected_refresh_token = "345" From 8cfb550769dbcd3ff503034448b1faa7cc34b90c Mon Sep 17 00:00:00 2001 From: Vi6hal <20889199+Vi6hal@users.noreply.github.com> Date: Wed, 26 Mar 2025 11:38:55 +0000 Subject: [PATCH 04/12] updated init file --- tap_xero/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tap_xero/__init__.py b/tap_xero/__init__.py index 529790a..49355ce 100644 --- a/tap_xero/__init__.py +++ b/tap_xero/__init__.py @@ -64,8 +64,8 @@ def load_metadata(stream, schema): return metadata.to_list(mdata) -def ensure_credentials_are_valid(config): - XeroClient(config).filter("currencies") +def ensure_credentials_are_valid(config, config_path): + XeroClient(config, config_path).filter("currencies") def discover(ctx): ctx.check_platform_access() From 183bb2653fd17c5128033c0b703b4d3ed2345a4d Mon Sep 17 00:00:00 2001 From: Vi6hal <20889199+Vi6hal@users.noreply.github.com> Date: Thu, 27 Mar 2025 05:31:55 +0000 Subject: [PATCH 05/12] updated exception handling tests --- tests/unittests/test_exception_handling.py | 28 +++++++++++----------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py index cd9264e..776e4f6 100644 --- a/tests/unittests/test_exception_handling.py +++ b/tests/unittests/test_exception_handling.py @@ -452,7 +452,7 @@ def test_check_unauthorized_401_error_in_discovery_mode(self, mocked_unauthorize xero_client = client_.XeroClient(config, config_path) try: - xero_client.check_platform_access(config, config_path) + xero_client.check_platform_access() except client_.XeroUnauthorizedError as e: expected_message = "HTTP-error-code: 401, Error: Invalid authorization credentials." self.assertEqual(str(e) ,expected_message) @@ -472,7 +472,7 @@ def test_check_forbidden_403_error_in_discovery_mode(self, mocked_refresh_creden xero_client.tenant_id = "123" try: - xero_client.check_platform_access(config, config_path) + xero_client.check_platform_access() except client_.XeroForbiddenError as e: expected_message = "HTTP-error-code: 403, Error: User doesn't have permission to access the resource." self.assertEqual(str(e) ,expected_message) @@ -492,7 +492,7 @@ def test_badrequest_400_error_in_discovery_mode(self, mocked_refresh_credentials xero_client.tenant_id = "123" try: - xero_client.check_platform_access(config, config_path) + xero_client.check_platform_access() except client_.XeroBadRequestError as e: expected_message = "HTTP-error-code: 400, Error: A validation exception has occurred." self.assertEqual(str(e), expected_message) @@ -512,7 +512,7 @@ def test_notfound_404_error_in_discovery_mode(self, mocked_refresh_credentials, xero_client.tenant_id = "123" try: - xero_client.check_platform_access(config, config_path) + xero_client.check_platform_access() except client_.XeroNotFoundError as e: expected_message = "HTTP-error-code: 404, Error: The resource you have specified cannot be found." self.assertEqual(str(e), expected_message) @@ -532,7 +532,7 @@ def test_precondition_failed_412_error_in_discovery_mode(self, mocked_refresh_cr xero_client.tenant_id = "123" try: - xero_client.check_platform_access(config, config_path) + xero_client.check_platform_access() except client_.XeroPreConditionFailedError as e: expected_message = "HTTP-error-code: 412, Error: One or more conditions given in the request header fields were invalid." self.assertEqual(str(e), expected_message) @@ -552,7 +552,7 @@ def test_internalservererror_500_error_in_discovery_mode(self, mocked_refresh_cr xero_client.tenant_id = "123" try: - xero_client.check_platform_access(config, config_path) + xero_client.check_platform_access() except client_.XeroInternalError as e: expected_message = "HTTP-error-code: 500, Error: An unhandled error with the Xero API. Contact the Xero API team if problems persist." self.assertEqual(str(e), expected_message) @@ -572,7 +572,7 @@ def test_notimplemented_501_error_in_discovery_mode(self, mocked_refresh_credent xero_client.tenant_id = "123" try: - xero_client.check_platform_access(config, config_path) + xero_client.check_platform_access() except client_.XeroNotImplementedError as e: expected_message = "HTTP-error-code: 501, Error: The method you have called has not been implemented." self.assertEqual(str(e), expected_message) @@ -592,7 +592,7 @@ def test_not_available_503_error_in_discovery_mode(self, mocked_refresh_credenti xero_client.tenant_id = "123" try: - xero_client.check_platform_access(config, config_path) + xero_client.check_platform_access() except client_.XeroNotAvailableError as e: expected_message = "HTTP-error-code: 503, Error: API service is currently unavailable." self.assertEqual(str(e), expected_message) @@ -612,7 +612,7 @@ def test_too_many_requests_in_day_429_error_in_discovery_mode(self, mocked_refre xero_client.tenant_id = "123" try: - xero_client.check_platform_access(config, config_path) + xero_client.check_platform_access() except client_.XeroTooManyError as e: expected_message = "HTTP-error-code: 429, Error: The API rate limit for your organisation/application pairing has been exceeded. Please retry after 1000 seconds" self.assertEqual(str(e), expected_message) @@ -632,7 +632,7 @@ def test_too_many_requests_in_minute_429_error_in_discovery_mode(self, mocked_re xero_client.tenant_id = "123" try: - xero_client.check_platform_access(config, config_path) + xero_client.check_platform_access() except client_.XeroTooManyInMinuteError as e: expected_message = "HTTP-error-code: 429, Error: The API rate limit for your organisation/application pairing has been exceeded. Please retry after 5 seconds" self.assertEqual(str(e), expected_message) @@ -652,7 +652,7 @@ def test_too_many_requests_in_day_429_not_backoff_behavior_discovery_mode(self, xero_client.tenant_id = "123" try: - xero_client.check_platform_access(config, config_path) + xero_client.check_platform_access() except (requests.HTTPError, client_.XeroTooManyError) as e: pass @@ -675,7 +675,7 @@ def test_too_many_requests_in_minute_429_backoff_behavior_discovery_mode(self, m xero_client.tenant_id = "123" try: - xero_client.check_platform_access(config, config_path) + xero_client.check_platform_access() except (requests.HTTPError, client_.XeroTooManyInMinuteError) as e: pass @@ -697,7 +697,7 @@ def test_internalservererror_500_backoff_behaviour_discovery_mode(self, mocked_r xero_client.tenant_id = "123" try: - xero_client.check_platform_access(config, config_path) + xero_client.check_platform_access() except (requests.HTTPError, client_.XeroInternalError) as e: pass @@ -725,7 +725,7 @@ def test_check_success_200_in_discovery_mode(self, mock_successful_session_post, expected_access_token = "123" expected_refresh_token = "345" - xero_client.check_platform_access(config, config_path) + xero_client.check_platform_access() self.assertEqual(xero_client.access_token, expected_access_token) self.assertEqual(config["refresh_token"], expected_refresh_token) From a0a15191d4335406cdecc1873c8b3ccb0c126b6a Mon Sep 17 00:00:00 2001 From: Vi6hal <20889199+Vi6hal@users.noreply.github.com> Date: Thu, 27 Mar 2025 05:49:27 +0000 Subject: [PATCH 06/12] update api client --- tap_xero/client.py | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index 270eca5..9761cc6 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -183,15 +183,13 @@ def __init__(self, config, config_path): self.user_agent = config.get("user_agent") self.config = config self.config_path = config_path - self.tenant_id = None + self.tenant_id = config['tenant_id'] self.access_token = None def refresh_credentials(self): - config = self.config - config_path = self.config_path - header_token = b64encode((config["client_id"] + ":" + config["client_secret"]).encode('utf-8')) + header_token = b64encode((self.config["client_id"] + ":" + self.config["client_secret"]).encode('utf-8')) headers = { "Authorization": "Basic " + header_token.decode('utf-8'), @@ -200,24 +198,20 @@ def refresh_credentials(self): post_body = { "grant_type": "refresh_token", - "refresh_token": config["refresh_token"], + "refresh_token": self.config["refresh_token"], } resp = self.session.post("https://identity.xero.com/connect/token", headers=headers, data=post_body) if resp.status_code != 200: raise_for_error(resp) else: - resp = resp.json() + resp = resp.json() + self.access_token = resp["access_token"] # Write to config file - self.access_token = resp["access_token"] - self.tenant_id = config['tenant_id'] - config['refresh_token'] = resp["refresh_token"] - config['access_token'] = resp["access_token"] - self.config = config + self.config['refresh_token'] = resp["refresh_token"] + self.config['access_token'] = resp["access_token"] update_config_file(config, config_path) - self.tenant_id = config['tenant_id'] - LOGGER.info("access_token refreshed") @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroInternalError), max_tries=3) From 320898d4d2011b0d636cd180202e3b7aa45bc5a2 Mon Sep 17 00:00:00 2001 From: Vi6hal <20889199+Vi6hal@users.noreply.github.com> Date: Thu, 27 Mar 2025 06:11:03 +0000 Subject: [PATCH 07/12] updated int. test --- tap_xero/client.py | 1 - tests/unittests/test_exception_handling.py | 58 ++++++---------------- 2 files changed, 15 insertions(+), 44 deletions(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index 9761cc6..a231a46 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -188,7 +188,6 @@ def __init__(self, config, config_path): def refresh_credentials(self): - header_token = b64encode((self.config["client_id"] + ":" + self.config["client_secret"]).encode('utf-8')) headers = { diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py index 776e4f6..a270654 100644 --- a/tests/unittests/test_exception_handling.py +++ b/tests/unittests/test_exception_handling.py @@ -148,7 +148,7 @@ def test_json_decode_exception(self, mocked_session, mocked_jsondecode_failing_r config = {} tap_stream_id = "contacts" - config_path = io.StringIO() + config_path = "" xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -166,7 +166,7 @@ def test_normal_filter_execution(self, mocked_session, mocked_jsondecode_success config = {} tap_stream_id = "contacts" - config_path = io.StringIO() + config_path = "" xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -184,7 +184,7 @@ def test_badrequest_400_error(self, mocked_session, mocked_badrequest_400_error) config = {} tap_stream_id = "contacts" - config_path = io.StringIO() + config_path = "" xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -205,7 +205,7 @@ def test_unauthorized_401_error(self, mocked_session, mocked_refresh_credential config = {} tap_stream_id = "contacts" - config_path = io.StringIO() + config_path = "" xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -226,7 +226,7 @@ def test_forbidden_403_exception(self, mocked_session, mocked_forbidden_403_exce config = {} tap_stream_id = "contacts" - config_path = io.StringIO() + config_path = "" xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -246,7 +246,7 @@ def test_notfound_404_error(self, mocked_session, mocked_notfound_404_error): config = {} tap_stream_id = "contacts" - config_path = io.StringIO() + config_path = "" xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -265,7 +265,7 @@ def test_precondition_failed_412_error(self, mocked_session, mocked_precondition config = {} tap_stream_id = "contacts" - config_path = io.StringIO() + config_path = "" xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -284,7 +284,7 @@ def test_internalservererror_500_error(self, mocked_session, mocked_internalserv config = {} tap_stream_id = "contacts" - config_path = io.StringIO() + config_path = "" xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -304,7 +304,7 @@ def test_notimplemented_501_error(self, mocked_session, mocked_notimplemented_50 config = {} tap_stream_id = "contacts" - config_path = io.StringIO() + config_path = "" xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -323,7 +323,7 @@ def test_not_available_503_error(self, mocked_session, mocked_not_available_503_ config = {} tap_stream_id = "contacts" - config_path = io.StringIO() + config_path = "" xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -343,7 +343,7 @@ def test_too_many_requests_429_in_day_error(self, mocked_session, mocked_failed_ config = {} tap_stream_id = "contacts" - config_path = io.StringIO() + config_path = "" xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -364,7 +364,7 @@ def test_too_many_requests_429_in_minute_error(self, mocked_session, mocked_fail config = {} tap_stream_id = "contacts" - config_path = io.StringIO() + config_path = "" xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -385,7 +385,7 @@ def test_too_many_requests_in_day_429_not_backoff_behavior(self, mocked_session, config = {} tap_stream_id = "contacts" - config_path = io.StringIO() + config_path = "" xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -404,7 +404,7 @@ def test_too_many_requests_in_minute_429_backoff_behavior(self, mocked_session, config = {} tap_stream_id = "contacts" - config_path = io.StringIO() + config_path = "" xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -421,7 +421,7 @@ def test_internalservererror_500_backoff_behaviour(self, mocked_session, mocked_ config = {} tap_stream_id = "contacts" - config_path = io.StringIO() + config_path = "" xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -447,8 +447,6 @@ def test_check_unauthorized_401_error_in_discovery_mode(self, mocked_unauthorize "tenant_id": "123" } config_path = "" - - config_path = io.StringIO() xero_client = client_.XeroClient(config, config_path) try: @@ -465,8 +463,6 @@ def test_check_forbidden_403_error_in_discovery_mode(self, mocked_refresh_creden mocked_refresh_credentials.return_value = "" config = {} config_path = "" - - config_path = io.StringIO() xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -485,8 +481,6 @@ def test_badrequest_400_error_in_discovery_mode(self, mocked_refresh_credentials mocked_refresh_credentials.return_value = "" config = {} config_path = "" - - config_path = io.StringIO() xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -505,8 +499,6 @@ def test_notfound_404_error_in_discovery_mode(self, mocked_refresh_credentials, mocked_refresh_credentials.return_value = "" config = {} config_path = "" - - config_path = io.StringIO() xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -525,8 +517,6 @@ def test_precondition_failed_412_error_in_discovery_mode(self, mocked_refresh_cr mocked_refresh_credentials.return_value = "" config = {} config_path = "" - - config_path = io.StringIO() xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -545,8 +535,6 @@ def test_internalservererror_500_error_in_discovery_mode(self, mocked_refresh_cr mocked_refresh_credentials.return_value = "" config = {} config_path = "" - - config_path = io.StringIO() xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -565,8 +553,6 @@ def test_notimplemented_501_error_in_discovery_mode(self, mocked_refresh_credent mocked_refresh_credentials.return_value = "" config = {} config_path = "" - - config_path = io.StringIO() xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -585,8 +571,6 @@ def test_not_available_503_error_in_discovery_mode(self, mocked_refresh_credenti mocked_refresh_credentials.return_value = "" config = {} config_path = "" - - config_path = io.StringIO() xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -605,8 +589,6 @@ def test_too_many_requests_in_day_429_error_in_discovery_mode(self, mocked_refre mocked_refresh_credentials.return_value = "" config = {} config_path = "" - - config_path = io.StringIO() xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -625,8 +607,6 @@ def test_too_many_requests_in_minute_429_error_in_discovery_mode(self, mocked_re mocked_refresh_credentials.return_value = "" config = {} config_path = "" - - config_path = io.StringIO() xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -645,8 +625,6 @@ def test_too_many_requests_in_day_429_not_backoff_behavior_discovery_mode(self, mocked_refresh_credentials.return_value = "" config = {} config_path = "" - - config_path = io.StringIO() xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -668,8 +646,6 @@ def test_too_many_requests_in_minute_429_backoff_behavior_discovery_mode(self, m mocked_refresh_credentials.return_value = "" config = {} config_path = "" - - config_path = io.StringIO() xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -690,8 +666,6 @@ def test_internalservererror_500_backoff_behaviour_discovery_mode(self, mocked_r mocked_refresh_credentials.return_value = "" config = {} config_path = "" - - config_path = io.StringIO() xero_client = client_.XeroClient(config, config_path) xero_client.access_token = "123" xero_client.tenant_id = "123" @@ -719,8 +693,6 @@ def test_check_success_200_in_discovery_mode(self, mock_successful_session_post, "tenant_id": "123" } config_path = "" - - config_path = io.StringIO() xero_client = client_.XeroClient(config, config_path) expected_access_token = "123" expected_refresh_token = "345" From 181e97d9e6a07150f3deb2b09ba18b7a463d3d9d Mon Sep 17 00:00:00 2001 From: Vi6hal <20889199+Vi6hal@users.noreply.github.com> Date: Thu, 27 Mar 2025 10:00:05 +0000 Subject: [PATCH 08/12] fixed pylint issue --- tap_xero/client.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index a231a46..7c25682 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -204,13 +204,13 @@ def refresh_credentials(self): if resp.status_code != 200: raise_for_error(resp) else: - resp = resp.json() + resp = resp.json() self.access_token = resp["access_token"] # Write to config file - self.config['refresh_token'] = resp["refresh_token"] + self.config['refresh_token'] = resp["refresh_token"] self.config['access_token'] = resp["access_token"] - update_config_file(config, config_path) + update_config_file(self.config, self.config_path) @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroInternalError), max_tries=3) From 50b3136bf8aa7c43d5d7d979eaca0caf987cbc2d Mon Sep 17 00:00:00 2001 From: Vi6hal <20889199+Vi6hal@users.noreply.github.com> Date: Thu, 27 Mar 2025 10:04:11 +0000 Subject: [PATCH 09/12] updated client impl. --- tap_xero/client.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index 7c25682..b2c6c9f 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -183,7 +183,6 @@ def __init__(self, config, config_path): self.user_agent = config.get("user_agent") self.config = config self.config_path = config_path - self.tenant_id = config['tenant_id'] self.access_token = None def refresh_credentials(self): @@ -205,12 +204,13 @@ def refresh_credentials(self): raise_for_error(resp) else: resp = resp.json() - self.access_token = resp["access_token"] # Write to config file self.config['refresh_token'] = resp["refresh_token"] self.config['access_token'] = resp["access_token"] update_config_file(self.config, self.config_path) + self.access_token = resp["access_token"] + self.tenant_id = config['tenant_id'] @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroInternalError), max_tries=3) From 26afeda80812770c727d6231976513bd4b3aeb32 Mon Sep 17 00:00:00 2001 From: Vi6hal <20889199+Vi6hal@users.noreply.github.com> Date: Thu, 27 Mar 2025 11:27:25 +0000 Subject: [PATCH 10/12] removed deprecated backoff logic for 401 --- tap_xero/client.py | 5 +++-- tap_xero/streams.py | 32 ++++---------------------------- 2 files changed, 7 insertions(+), 30 deletions(-) diff --git a/tap_xero/client.py b/tap_xero/client.py index b2c6c9f..4e0e312 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -181,6 +181,7 @@ class XeroClient(): def __init__(self, config, config_path): self.session = requests.Session() self.user_agent = config.get("user_agent") + self.tenant_id = None self.config = config self.config_path = config_path self.access_token = None @@ -210,7 +211,7 @@ def refresh_credentials(self): self.config['access_token'] = resp["access_token"] update_config_file(self.config, self.config_path) self.access_token = resp["access_token"] - self.tenant_id = config['tenant_id'] + self.tenant_id = self.config['tenant_id'] @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroInternalError), max_tries=3) @@ -234,7 +235,7 @@ def check_platform_access(self): if response.status_code != 200: raise_for_error(response) - + @backoff.on_exception(backoff.expo, XeroUnauthorizedError, max_tries=1) @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroInternalError, XeroUnauthorizedError), max_tries=3) @backoff.on_exception(retry_after_wait_gen, XeroTooManyInMinuteError, giveup=is_not_status_code_fn([429]), jitter=None, max_tries=3) def filter(self, tap_stream_id, since=None, **params): diff --git a/tap_xero/streams.py b/tap_xero/streams.py index 051402c..0140ab9 100644 --- a/tap_xero/streams.py +++ b/tap_xero/streams.py @@ -9,10 +9,12 @@ FULL_PAGE_SIZE = 100 -def _request_with_timer(tap_stream_id, xero, filter_options): + +def _make_request(ctx, tap_stream_id, filter_options=None, attempts=0): + filter_options = filter_options or {} with metrics.http_request_timer(tap_stream_id) as timer: try: - resp = xero.filter(tap_stream_id, **filter_options) + resp = ctx.client.filter(tap_stream_id, **filter_options) timer.tags[metrics.Tag.http_status_code] = 200 return resp except HTTPError as e: @@ -20,32 +22,6 @@ def _request_with_timer(tap_stream_id, xero, filter_options): raise -class RateLimitException(Exception): - pass - - -@backoff.on_exception(backoff.expo, - RateLimitException, - max_tries=10, - factor=2) -def _make_request(ctx, tap_stream_id, filter_options=None, attempts=0): - filter_options = filter_options or {} - try: - return _request_with_timer(tap_stream_id, ctx.client, filter_options) - except HTTPError as e: - if e.response.status_code == 401: - if attempts == 1: - raise Exception("Received Not Authorized response after credential refresh.") from e - ctx.refresh_credentials() - return _make_request(ctx, tap_stream_id, filter_options, attempts + 1) - - if e.response.status_code == 503: - raise RateLimitException() from e - - raise - assert False - - class Stream(): def __init__(self, tap_stream_id, pk_fields, bookmark_key="UpdatedDateUTC", format_fn=None): self.tap_stream_id = tap_stream_id From 79e7941cb83dccf81d0ee74b039a31ac56e35ee2 Mon Sep 17 00:00:00 2001 From: Vi6hal <20889199+Vi6hal@users.noreply.github.com> Date: Thu, 27 Mar 2025 11:33:25 +0000 Subject: [PATCH 11/12] fix pylint --- tap_xero/streams.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tap_xero/streams.py b/tap_xero/streams.py index 0140ab9..28d5776 100644 --- a/tap_xero/streams.py +++ b/tap_xero/streams.py @@ -2,7 +2,6 @@ import singer from singer import metadata, metrics, Transformer from singer.utils import strptime_with_tz -import backoff from . import transform LOGGER = singer.get_logger() @@ -10,7 +9,7 @@ -def _make_request(ctx, tap_stream_id, filter_options=None, attempts=0): +def _make_request(ctx, tap_stream_id, filter_options=None): filter_options = filter_options or {} with metrics.http_request_timer(tap_stream_id) as timer: try: From 450115dc337c09ddfbc36aaecaa29508814ec279 Mon Sep 17 00:00:00 2001 From: Vi6hal <20889199+Vi6hal@users.noreply.github.com> Date: Wed, 2 Apr 2025 05:11:08 +0000 Subject: [PATCH 12/12] fixed client and changelog --- CHANGELOG.md | 2 +- tap_xero/client.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ec2b024..f403b6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ # Changelog ## 2.4.0 - * Added Retry and exception handling for 401 errors + * Add backoff and error handling for 401 errors [#121](https://github.com/singer-io/tap-xero/pull/121) ## 2.3.1 * Add new fields into the schema of credit_notes stream [#117](https://github.com/singer-io/tap-xero/pull/117) diff --git a/tap_xero/client.py b/tap_xero/client.py index 4e0e312..47253f4 100644 --- a/tap_xero/client.py +++ b/tap_xero/client.py @@ -236,7 +236,7 @@ def check_platform_access(self): raise_for_error(response) @backoff.on_exception(backoff.expo, XeroUnauthorizedError, max_tries=1) - @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroInternalError, XeroUnauthorizedError), max_tries=3) + @backoff.on_exception(backoff.expo, (json.decoder.JSONDecodeError, XeroInternalError), max_tries=3) @backoff.on_exception(retry_after_wait_gen, XeroTooManyInMinuteError, giveup=is_not_status_code_fn([429]), jitter=None, max_tries=3) def filter(self, tap_stream_id, since=None, **params): xero_resource_name = tap_stream_id.title().replace("_", "")