Add backoff mechanism for 429, throw friendly message on REST API error and validate authorization during discovery mode #85

karanpanchal-crest · 2021-04-04T18:39:29Z

Description of change

Since below JIRAs have interdependent code, I have added code for all these JIRAs in this single PR

TDL-593: Added backoff mechanism to retry API execution on encountering 429 status code
TDL-594: Added support of checking the authentication and authorization of the credentials provided during discovery mode
TDL-648: Added custom exceptions to throw friendly message on permission errors and other failure status codes

Manual QA steps

Raised 429 status code in the local environment and verified if the function is retrying for this scenario
Verified the execution of normal scenario (having status code 200) if it is getting affected because of adding backoff behavior

Risks

If Day Limit is reached, retrying might not be able to collect data until API resumes providing the response.
Link: https://developer.xero.com/documentation/oauth2/limits

Rollback steps

revert this branch

…n two executions

…orization of the credentials passed in the config json file

karanpanchal-crest · 2021-04-06T18:21:05Z

tap_xero/__init__.py

@@ -68,7 +68,7 @@ def ensure_credentials_are_valid(config):
    XeroClient(config).filter("currencies")

 def discover(ctx):
-    ctx.refresh_credentials()
+    ctx.check_platform_access()


Code added w.r.to TDL-594 (Check authentication and authorization during discovery mode)

karanpanchal-crest · 2021-04-06T18:21:21Z

tap_xero/client.py

+            self.tenant_id = config['tenant_id']
+
+
+    def check_platform_access(self, config, config_path, check_authentication=True):


Code added w.r.to TDL-594 (Check authentication and authorization during discovery mode)

karanpanchal-crest · 2021-04-06T18:21:27Z

tap_xero/client.py

-        update_config_file(config, config_path)
-        self.access_token = resp["access_token"]
-        self.tenant_id = config['tenant_id']
+        if resp.status_code != 200:


Code added w.r.to TDL-594 (Check authentication and authorization during discovery mode)

karanpanchal-crest · 2021-04-06T18:22:41Z

tap_xero/client.py


 BASE_URL = "https://api.xero.com/api.xro/2.0"


+class XeroError(Exception):


Code added w.r.to TDL-648 (Permission Denied error should throw a user friendly message)

karanpanchal-crest · 2021-04-06T18:23:03Z

tap_xero/client.py

+                api_rate_limit_message = ERROR_CODE_EXCEPTION_MAPPING[429]["message"]
+                message = "HTTP-error-code: 429, Error: {}. Please retry after {} seconds".format(api_rate_limit_message, resp_headers.get("Retry-After"))
+            # Handling status code 403 specially since response of API does not contain enough information
+            elif error_code in (403, 401):


Code added w.r.to TDL-648 (Permission Denied error should throw a user friendly message)

…ssfully

karanpanchal-crest · 2021-04-06T19:33:18Z

tests/unittests/test_exception_handling.py

+    """
+
+    @mock.patch('requests.Request', side_effect=mocked_badrequest_400_error)
+    def test_badrequest_400_error(self, mocked_session, mocked_badrequest_400_error):


Test cases to verify TDL-648 (Verifying that different error codes being handled gracefully)

karanpanchal-crest · 2021-04-06T19:33:59Z

tests/unittests/test_exception_handling.py

+
+
+    @mock.patch('requests.Request', side_effect=mocked_failed_429_request)
+    def test_too_many_requests_429_error(self, mocked_session, mocked_failed_429_request):


Test cases to verify TDL-593 (Verifying the backoff behavior with Error code 429)

karanpanchal-crest · 2021-04-06T19:34:47Z

tests/unittests/test_exception_handling.py

+
+@mock.patch('requests.Session.send', side_effect=mocked_session)
+class TestCheckPlatformAccessBehavior(unittest.TestCase):
+


Test cases to verify TDL-594 (Verifying the authentication and authorization behavior during discovery mode)

KAllan357

One adjustment, then this looks good.

KAllan357 · 2021-04-08T18:25:15Z

tap_xero/client.py

-                          json.decoder.JSONDecodeError,
-                          max_tries=3)
+    @backoff.on_exception(backoff.expo,json.decoder.JSONDecodeError,max_tries=3)
+    @backoff.on_exception(backoff.expo, XeroTooManyError, max_tries=3, factor=2)


The exceptions can be combined to form a single backoff statement as a tuple (https://github.com/litl/backoff#backoffon_exception)

ex:

@backoff.on_exception(backoff.expo, (XeroTooManyError, json.decoder.JSONDecodeError), max_tries=3, factor=2)

@KAllan357 Incorporated the suggestion

TDL-593: Adding backoff behavior of 429 errors

fe68ca4

karanpanchal-crest requested review from KAllan357 and cosimon April 4, 2021 18:39

Karan Panchal added 3 commits April 5, 2021 20:24

TDL-593: Adding the factor parameter to increase the wait time betwee…

c6d742a

…n two executions

TDL-648: Added custom exceptions to handle xero REST API error codes

d1e74ba

TDL-594: Adding the support of validating the authentication and auth…

ff9c22a

…orization of the credentials passed in the config json file

karanpanchal-crest commented Apr 6, 2021

View reviewed changes

karanpanchal-crest changed the title ~~TDL-593: Adding backoff behavior of 429 errors~~ Add backoff mechanism for 429, throw friendly message on REST API error and validate authorization during discovery mode Apr 6, 2021

TDL-594: Adding a positive scenario where discovery mode passes succe…

587956f

…ssfully

karanpanchal-crest commented Apr 6, 2021

View reviewed changes

TDL-593: Resolving merge conflicts

6419134

KAllan357 suggested changes Apr 8, 2021

View reviewed changes

TDL-593: Resolving code review comment

94b7e41

KAllan357 approved these changes Apr 9, 2021

View reviewed changes

KAllan357 merged commit 4cc8f3c into master Apr 9, 2021

KAllan357 deleted the TDL-593-add-backoff-support-for-429-errors branch April 9, 2021 16:06

erinkcochran87 mentioned this pull request May 17, 2021

Changelog - 4/13 to 4/26/2021 stitchdata/docs#633

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add backoff mechanism for 429, throw friendly message on REST API error and validate authorization during discovery mode #85

Add backoff mechanism for 429, throw friendly message on REST API error and validate authorization during discovery mode #85

karanpanchal-crest commented Apr 4, 2021 •

edited

Loading

karanpanchal-crest Apr 6, 2021

karanpanchal-crest Apr 6, 2021

karanpanchal-crest Apr 6, 2021

karanpanchal-crest Apr 6, 2021

karanpanchal-crest Apr 6, 2021

karanpanchal-crest Apr 6, 2021 •

edited

Loading

karanpanchal-crest Apr 6, 2021 •

edited

Loading

karanpanchal-crest Apr 6, 2021 •

edited

Loading

KAllan357 left a comment

KAllan357 Apr 8, 2021

karanpanchal-crest Apr 9, 2021

		self.tenant_id = config['tenant_id']


		def check_platform_access(self, config, config_path, check_authentication=True):


		BASE_URL = "https://api.xero.com/api.xro/2.0"


		class XeroError(Exception):



		@mock.patch('requests.Request', side_effect=mocked_failed_429_request)
		def test_too_many_requests_429_error(self, mocked_session, mocked_failed_429_request):


		@mock.patch('requests.Session.send', side_effect=mocked_session)
		class TestCheckPlatformAccessBehavior(unittest.TestCase):

Add backoff mechanism for 429, throw friendly message on REST API error and validate authorization during discovery mode #85

Add backoff mechanism for 429, throw friendly message on REST API error and validate authorization during discovery mode #85

Conversation

karanpanchal-crest commented Apr 4, 2021 • edited Loading

Description of change

Manual QA steps

Risks

Rollback steps

karanpanchal-crest Apr 6, 2021

Choose a reason for hiding this comment

karanpanchal-crest Apr 6, 2021

Choose a reason for hiding this comment

karanpanchal-crest Apr 6, 2021

Choose a reason for hiding this comment

karanpanchal-crest Apr 6, 2021

Choose a reason for hiding this comment

karanpanchal-crest Apr 6, 2021

Choose a reason for hiding this comment

karanpanchal-crest Apr 6, 2021 • edited Loading

Choose a reason for hiding this comment

karanpanchal-crest Apr 6, 2021 • edited Loading

Choose a reason for hiding this comment

karanpanchal-crest Apr 6, 2021 • edited Loading

Choose a reason for hiding this comment

KAllan357 left a comment

Choose a reason for hiding this comment

KAllan357 Apr 8, 2021

Choose a reason for hiding this comment

karanpanchal-crest Apr 9, 2021

Choose a reason for hiding this comment

karanpanchal-crest commented Apr 4, 2021 •

edited

Loading

karanpanchal-crest Apr 6, 2021 •

edited

Loading

karanpanchal-crest Apr 6, 2021 •

edited

Loading

karanpanchal-crest Apr 6, 2021 •

edited

Loading