From a4d4474e0824acffde7f5bb1e452166cc19e4de3 Mon Sep 17 00:00:00 2001 From: prijendev Date: Tue, 23 Aug 2022 10:23:25 +0530 Subject: [PATCH 1/9] Updated exception handling and added unit test. --- tap_freshdesk/client.py | 111 ++++++++++++++- tests/unittests/test_exception_handling.py | 151 +++++++++++++++++++++ 2 files changed, 258 insertions(+), 4 deletions(-) create mode 100644 tests/unittests/test_exception_handling.py diff --git a/tap_freshdesk/client.py b/tap_freshdesk/client.py index 03e7208..2800295 100644 --- a/tap_freshdesk/client.py +++ b/tap_freshdesk/client.py @@ -7,7 +7,110 @@ LOGGER = singer.get_logger() BASE_URL = "https://{}.freshdesk.com" +DEFAULT_TIMEOUT = 6 +class FreshdeskException(Exception): + pass + +class FresdeskValidationError(FreshdeskException): + pass + +class FresdeskAuthenticationError(FreshdeskException): + pass + +class FresdeskAccessDeniedError(FreshdeskException): + pass + +class FresdeskNotFoundError(FreshdeskException): + pass + +class FresdeskMethodNotAllowedError(FreshdeskException): + pass + +class FresdeskUnsupportedAcceptHeaderError(FreshdeskException): + pass + +class FresdeskConflictingStateError(FreshdeskException): + pass + +class FresdeskUnsupportedContentError(FreshdeskException): + pass + +class FresdeskRateLimitError(FreshdeskException): + pass + +class Server5xxError(FreshdeskException): + pass + +class FresdeskServerError(Server5xxError): + pass + + +ERROR_CODE_EXCEPTION_MAPPING = { + 400: { + "raise_exception": FresdeskValidationError, + "message": "The request body/query string is not in the correct format." + }, + 401: { + "raise_exception": FresdeskAuthenticationError, + "message": "The Authorization header is either missing or incorrect." + }, + 403: { + "raise_exception": FresdeskAccessDeniedError, + "message": "The agent whose credentials were used in making this request was not authorized to perform this API call." + }, + 404: { + "raise_exception": FresdeskNotFoundError, + "message": "The request contains invalid ID/Freshdesk domain in the URL or an invalid URL itself." + }, + 405: { + "raise_exception": FresdeskMethodNotAllowedError, + "message": "This API request used the wrong HTTP verb/method." + }, + 406: { + "raise_exception": FresdeskUnsupportedAcceptHeaderError, + "message": "Only application/json and */* are supported in 'Accepted' header." + }, + 409: { + "raise_exception": FresdeskConflictingStateError, + "message": "The resource that is being created/updated is in an inconsistent or conflicting state." + }, + 415: { + "raise_exception": FresdeskUnsupportedContentError, + "message": "Content type application/xml is not supported. Only application/json is supported." + }, + 429: { + "raise_exception": FresdeskRateLimitError, + "message": "The API rate limit allotted for your Freshdesk domain has been exhausted." + }, + 500: { + "raise_exception": FresdeskServerError, + "message": "Unexpected Server Error." + }, +} + +def raise_for_error(response): + """ + Retrieve the error code and the error message from the response and return custom exceptions accordingly. + """ + try: + response.raise_for_status() + except (requests.HTTPError) as error: + error_code = response.status_code + # Forming a response message for raising a custom exception + try: + response_json = response.json() + except Exception: + response_json = {} + + if error_code not in ERROR_CODE_EXCEPTION_MAPPING and error_code > 500: + # Raise `Server5xxError` for all 5xx unknown error + exc = Server5xxError + else: + exc = ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("raise_exception", FreshdeskException) + message = response_json.get("description", ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("message", "Unknown Error")) + formatted_message = "HTTP-error-code: {}, Error: {}".format(error_code, message) + raise exc(formatted_message) from None class FreshdeskClient: """ @@ -34,9 +137,8 @@ def check_access_token(self): self.request(self.base_url+"/api/v2/roles", {"per_page": 1, "page": 1}) @backoff.on_exception(backoff.expo, - (requests.exceptions.RequestException), + (TimeoutError, ConnectionError, Server5xxError), max_tries=5, - giveup=lambda e: e.response is not None and 400 <= e.response.status_code < 500, factor=2) @utils.ratelimit(1, 2) def request(self, url, params={}): @@ -49,7 +151,7 @@ def request(self, url, params={}): req = requests.Request('GET', url, params=params, auth=(self.config['api_key'], ""), headers=headers).prepare() LOGGER.info("GET {}".format(req.url)) - response = self.session.send(req) + response = self.session.send(req, timeout=DEFAULT_TIMEOUT) # Call the function again if the rate limit is exceeded if 'Retry-After' in response.headers: @@ -58,6 +160,7 @@ def request(self, url, params={}): time.sleep(retry_after) return self.request(url, params) - response.raise_for_status() + if response.status_code != 200: + raise_for_error(response) return response.json() diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py new file mode 100644 index 0000000..62af5b0 --- /dev/null +++ b/tests/unittests/test_exception_handling.py @@ -0,0 +1,151 @@ +import unittest +from unittest import mock +import json +import requests +from parameterized import parameterized +from tap_freshdesk import client +from tap_freshdesk.client import raise_for_error, ERROR_CODE_EXCEPTION_MAPPING + +def get_response(status_code, json_resp={}, headers = None): + """ + Returns mock response + """ + response = requests.Response() + response.status_code = status_code + response._content = json.dumps(json_resp).encode() + if headers: + response.headers = headers + return response + + +class TestExceptionHanfling(unittest.TestCase): + """ + Test Error is thrown with the expected error message. + """ + + @parameterized.expand([ + (400, client.FresdeskValidationError), + (401, client.FresdeskAuthenticationError), + (403, client.FresdeskAccessDeniedError), + (404, client.FresdeskNotFoundError), + (405, client.FresdeskMethodNotAllowedError), + (406, client.FresdeskUnsupportedAcceptHeaderError), + (409, client.FresdeskConflictingStateError), + (415, client.FresdeskUnsupportedContentError), + (429, client.FresdeskRateLimitError), + (500, client.FresdeskServerError), + (503, client.Server5xxError), # Unknown 5xx error + ]) + def test_custom_error_message(self, error_code, error): + """ + Test that error is thrown with the custom error message + if no description is provided in response. + """ + expected_message = "HTTP-error-code: {}, Error: {}".format(error_code, ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("message", "Unknown Error")) + with self.assertRaises(error) as e: + raise_for_error(get_response(error_code)) + + # Verify that an error message is expected + self.assertEqual(str(e.exception), expected_message) + + @parameterized.expand([ + (400, "Client or Validation Error", client.FresdeskValidationError), + (401, "Authentication Failure", client.FresdeskAuthenticationError), + (403, "Access Denied", client.FresdeskAccessDeniedError), + (404, "Requested Resource not Found", client.FresdeskNotFoundError), + (405, "Method not allowed", client.FresdeskMethodNotAllowedError), + (406, "Unsupported Accept Header", client.FresdeskUnsupportedAcceptHeaderError), + (409, "AInconsistent/Conflicting State", client.FresdeskConflictingStateError), + (415, "Unsupported Content-type", client.FresdeskUnsupportedContentError), + (429, "Rate Limit Exceeded", client.FresdeskRateLimitError), + (500, "Unexpected Server Error", client.FresdeskServerError), + ]) + def test_error_response_message(self, error_code, message, error): + """ + Test that error is thrown with description in the response. + """ + expected_message = "HTTP-error-code: {}, Error: {}".format(error_code, message) + with self.assertRaises(error) as e: + raise_for_error(get_response(error_code, {"description": message})) + + # Verify that an error message is expected + self.assertEqual(str(e.exception), expected_message) + + def json_decoder_error(self): + """Test for invalid json response, tap does not throw JSON decoder error.""" + mock_response = get_response(400, {"description": "Client or Validation Error"}) + mock_response._content = "ABC".encode() + expected_message = "HTTP-error-code: {}, Error: {}".format(400, "Client or Validation Error") + with self.assertRaises(client.FresdeskValidationError) as e: + raise_for_error(mock_response) + + # Verify that an error message is expected + self.assertEqual(str(e.exception), expected_message) + + +@mock.patch("requests.Session.send") +@mock.patch("time.sleep") +class TestBackoffHandling(unittest.TestCase): + """ + Test backoff handling for all 5xx, timeout and connection error. + """ + + @parameterized.expand([ + (lambda *x,**y:get_response(500), client.FresdeskServerError), + (lambda *x,**y:get_response(503), client.Server5xxError), + (ConnectionError, ConnectionError), + (TimeoutError, TimeoutError), + ]) + def test_backoff(self, mock_sleep, mock_request, mock_response, error): + """ + Test that for 500, timeout and connection error `request` method will backoff 5 times. + """ + mock_request.side_effect = mock_response + config = {"user_agent": "SAMPLE_AGENT", "api_key": "TEST_API_KEY"} + _client = client.FreshdeskClient(config) + with self.assertRaises(error) as e: + _client.request("https://TEST_URL.com") + + # Verify that `request` method was called 5 times. + self.assertEqual(mock_request.call_count, 5) + + +@mock.patch("requests.Session.send") +@mock.patch("tap_freshdesk.client.time.sleep") +class TestRateLimitHandling(unittest.TestCase): + """ + Test rate-limit exception handling. + """ + + @parameterized.expand([ + ("30",), + ("5",), + ("50",), + ]) + def test_rate_limit_exceeded(self, mock_sleep, mock_request, retry_seconds): + """ + Test that when the rate limit is exceeded, the function is called again after `Retry-After` seconds. + """ + mock_request.side_effect = [get_response(429, headers={"Retry-After": retry_seconds}), get_response(200)] + config = {"user_agent": "SAMPLE_AGENT", "api_key": "TEST_API_KEY"} + _client = client.FreshdeskClient(config) + _client.request("https://TEST_URL.com") + + # Verify that `requests` method is called twice. + self.assertEqual(mock_request.call_count, 2) + + # Verify that `time.sleep` was called for 'Retry-After' seconds from header. + mock_sleep.assert_any_call(int(retry_seconds)) + + def test_rate_limite_not_exceeded(self, mock_sleep, mock_request): + """ + Test that the function will not retry for the success response. + """ + mock_request.side_effect = [get_response(200)] + config = {"user_agent": "SAMPLE_AGENT", "api_key": "TEST_API_KEY"} + _client = client.FreshdeskClient(config) + _client.request("https://TEST_URL.com") + + # Verify that `requests` method is called twice. + self.assertEqual(mock_request.call_count, 1) + mock_request.assert_called_with(mock.ANY, timeout=client.DEFAULT_TIMEOUT) From 5e496e62085499fd37daeeceeff8478d04fdf2bc Mon Sep 17 00:00:00 2001 From: prijendev Date: Tue, 23 Aug 2022 14:05:46 +0530 Subject: [PATCH 2/9] Added pylint in the tap. --- setup.py | 5 +++++ tests/unittests/test_exception_handling.py | 6 +++--- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/setup.py b/setup.py index 67d9ead..cbbc079 100644 --- a/setup.py +++ b/setup.py @@ -14,6 +14,11 @@ 'requests==2.20.0', 'backoff==1.8.0' ], + extras_require={ + 'dev': [ + 'pylint', + ] + }, entry_points=''' [console_scripts] tap-freshdesk=tap_freshdesk:main diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py index 62af5b0..26d3e97 100644 --- a/tests/unittests/test_exception_handling.py +++ b/tests/unittests/test_exception_handling.py @@ -92,7 +92,7 @@ class TestBackoffHandling(unittest.TestCase): @parameterized.expand([ (lambda *x,**y:get_response(500), client.FresdeskServerError), - (lambda *x,**y:get_response(503), client.Server5xxError), + (lambda *x,**y:get_response(503), client.Server5xxError), # Unknown 5xx error (ConnectionError, ConnectionError), (TimeoutError, TimeoutError), ]) @@ -134,7 +134,7 @@ def test_rate_limit_exceeded(self, mock_sleep, mock_request, retry_seconds): # Verify that `requests` method is called twice. self.assertEqual(mock_request.call_count, 2) - # Verify that `time.sleep` was called for 'Retry-After' seconds from header. + # Verify that `time.sleep` was called for 'Retry-After' seconds from the header. mock_sleep.assert_any_call(int(retry_seconds)) def test_rate_limite_not_exceeded(self, mock_sleep, mock_request): @@ -146,6 +146,6 @@ def test_rate_limite_not_exceeded(self, mock_sleep, mock_request): _client = client.FreshdeskClient(config) _client.request("https://TEST_URL.com") - # Verify that `requests` method is called twice. + # Verify that `requests` method is called once. self.assertEqual(mock_request.call_count, 1) mock_request.assert_called_with(mock.ANY, timeout=client.DEFAULT_TIMEOUT) From 8b7a262c40c8aab44f443303aa969894236a510d Mon Sep 17 00:00:00 2001 From: prijendev Date: Thu, 25 Aug 2022 10:31:00 +0530 Subject: [PATCH 3/9] Added request timeout and skip 404 error. --- tap_freshdesk/client.py | 56 +++++++++++------- tap_freshdesk/streams.py | 12 +++- tests/unittests/test_exception_handling.py | 68 +++++++++++++++------- tests/unittests/test_timeout.py | 56 ++++++++++++++++++ 4 files changed, 148 insertions(+), 44 deletions(-) create mode 100644 tests/unittests/test_timeout.py diff --git a/tap_freshdesk/client.py b/tap_freshdesk/client.py index 2800295..1ad832a 100644 --- a/tap_freshdesk/client.py +++ b/tap_freshdesk/client.py @@ -7,84 +7,84 @@ LOGGER = singer.get_logger() BASE_URL = "https://{}.freshdesk.com" -DEFAULT_TIMEOUT = 6 +DEFAULT_TIMEOUT = 300 class FreshdeskException(Exception): pass -class FresdeskValidationError(FreshdeskException): +class FreshdeskValidationError(FreshdeskException): pass -class FresdeskAuthenticationError(FreshdeskException): +class FreshdeskAuthenticationError(FreshdeskException): pass -class FresdeskAccessDeniedError(FreshdeskException): +class FreshdeskAccessDeniedError(FreshdeskException): pass -class FresdeskNotFoundError(FreshdeskException): +class FreshdeskNotFoundError(FreshdeskException): pass -class FresdeskMethodNotAllowedError(FreshdeskException): +class FreshdeskMethodNotAllowedError(FreshdeskException): pass -class FresdeskUnsupportedAcceptHeaderError(FreshdeskException): +class FreshdeskUnsupportedAcceptHeaderError(FreshdeskException): pass -class FresdeskConflictingStateError(FreshdeskException): +class FreshdeskConflictingStateError(FreshdeskException): pass -class FresdeskUnsupportedContentError(FreshdeskException): +class FreshdeskUnsupportedContentError(FreshdeskException): pass -class FresdeskRateLimitError(FreshdeskException): +class FreshdeskRateLimitError(FreshdeskException): pass class Server5xxError(FreshdeskException): pass -class FresdeskServerError(Server5xxError): +class FreshdeskServerError(Server5xxError): pass ERROR_CODE_EXCEPTION_MAPPING = { 400: { - "raise_exception": FresdeskValidationError, + "raise_exception": FreshdeskValidationError, "message": "The request body/query string is not in the correct format." }, 401: { - "raise_exception": FresdeskAuthenticationError, + "raise_exception": FreshdeskAuthenticationError, "message": "The Authorization header is either missing or incorrect." }, 403: { - "raise_exception": FresdeskAccessDeniedError, + "raise_exception": FreshdeskAccessDeniedError, "message": "The agent whose credentials were used in making this request was not authorized to perform this API call." }, 404: { - "raise_exception": FresdeskNotFoundError, + "raise_exception": FreshdeskNotFoundError, "message": "The request contains invalid ID/Freshdesk domain in the URL or an invalid URL itself." }, 405: { - "raise_exception": FresdeskMethodNotAllowedError, + "raise_exception": FreshdeskMethodNotAllowedError, "message": "This API request used the wrong HTTP verb/method." }, 406: { - "raise_exception": FresdeskUnsupportedAcceptHeaderError, + "raise_exception": FreshdeskUnsupportedAcceptHeaderError, "message": "Only application/json and */* are supported in 'Accepted' header." }, 409: { - "raise_exception": FresdeskConflictingStateError, + "raise_exception": FreshdeskConflictingStateError, "message": "The resource that is being created/updated is in an inconsistent or conflicting state." }, 415: { - "raise_exception": FresdeskUnsupportedContentError, + "raise_exception": FreshdeskUnsupportedContentError, "message": "Content type application/xml is not supported. Only application/json is supported." }, 429: { - "raise_exception": FresdeskRateLimitError, + "raise_exception": FreshdeskRateLimitError, "message": "The API rate limit allotted for your Freshdesk domain has been exhausted." }, 500: { - "raise_exception": FresdeskServerError, + "raise_exception": FreshdeskServerError, "message": "Unexpected Server Error." }, } @@ -121,6 +121,8 @@ def __init__(self, config): self.config = config self.session = requests.Session() self.base_url = BASE_URL.format(config.get("domain")) + self.timeout = DEFAULT_TIMEOUT + self.set_timeout() def __enter__(self): self.check_access_token() @@ -130,6 +132,18 @@ def __exit__(self, exception_type, exception_value, traceback): # Kill the session instance. self.session.close() + def set_timeout(self): + """ + Set timeout value from config, if the value is passed. + Else raise an exception. + """ + timeout = self.config.get("timeout", DEFAULT_TIMEOUT) + if ((type(timeout) in [int, float]) or + (type(timeout)==str and timeout.replace('.', '', 1).isdigit())) and float(timeout): + self.timeout = int(float(timeout)) + else: + raise Exception("The entered timeout is invalid, it should be a valid none-zero integer.") + def check_access_token(self): """ Check if the access token is valid. diff --git a/tap_freshdesk/streams.py b/tap_freshdesk/streams.py index 2a3382d..68df5ae 100644 --- a/tap_freshdesk/streams.py +++ b/tap_freshdesk/streams.py @@ -1,7 +1,7 @@ from datetime import datetime import singer from singer import bookmarks - +from tap_freshdesk.client import FreshdeskNotFoundError LOGGER = singer.get_logger() PAGE_SIZE = 100 @@ -204,6 +204,16 @@ class TimeEntries(Stream): path = 'tickets/{}/time_entries' parent = 'tickets' + def sync_obj(self, state, start_date, client, catalog, selected_streams, streams_to_sync, predefined_filter=None): + try: + return super().sync_obj(state, start_date, client, catalog, selected_streams, streams_to_sync, predefined_filter) + except FreshdeskNotFoundError: + # Skipping 404 error as it is returned for deleted tickets and spam + LOGGER.warning("Could not retrieve time entries for ticket id {}. This may be caused by tickets " + "marked as spam or deleted.".format(self.parent_id)) + pass + + STREAMS = { "agents": Agents, "companies": Companies, diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py index 26d3e97..a1f01b4 100644 --- a/tests/unittests/test_exception_handling.py +++ b/tests/unittests/test_exception_handling.py @@ -5,6 +5,7 @@ from parameterized import parameterized from tap_freshdesk import client from tap_freshdesk.client import raise_for_error, ERROR_CODE_EXCEPTION_MAPPING +from tap_freshdesk.streams import Tickets, TimeEntries def get_response(status_code, json_resp={}, headers = None): """ @@ -24,16 +25,16 @@ class TestExceptionHanfling(unittest.TestCase): """ @parameterized.expand([ - (400, client.FresdeskValidationError), - (401, client.FresdeskAuthenticationError), - (403, client.FresdeskAccessDeniedError), - (404, client.FresdeskNotFoundError), - (405, client.FresdeskMethodNotAllowedError), - (406, client.FresdeskUnsupportedAcceptHeaderError), - (409, client.FresdeskConflictingStateError), - (415, client.FresdeskUnsupportedContentError), - (429, client.FresdeskRateLimitError), - (500, client.FresdeskServerError), + (400, client.FreshdeskValidationError), + (401, client.FreshdeskAuthenticationError), + (403, client.FreshdeskAccessDeniedError), + (404, client.FreshdeskNotFoundError), + (405, client.FreshdeskMethodNotAllowedError), + (406, client.FreshdeskUnsupportedAcceptHeaderError), + (409, client.FreshdeskConflictingStateError), + (415, client.FreshdeskUnsupportedContentError), + (429, client.FreshdeskRateLimitError), + (500, client.FreshdeskServerError), (503, client.Server5xxError), # Unknown 5xx error ]) def test_custom_error_message(self, error_code, error): @@ -49,16 +50,16 @@ def test_custom_error_message(self, error_code, error): self.assertEqual(str(e.exception), expected_message) @parameterized.expand([ - (400, "Client or Validation Error", client.FresdeskValidationError), - (401, "Authentication Failure", client.FresdeskAuthenticationError), - (403, "Access Denied", client.FresdeskAccessDeniedError), - (404, "Requested Resource not Found", client.FresdeskNotFoundError), - (405, "Method not allowed", client.FresdeskMethodNotAllowedError), - (406, "Unsupported Accept Header", client.FresdeskUnsupportedAcceptHeaderError), - (409, "AInconsistent/Conflicting State", client.FresdeskConflictingStateError), - (415, "Unsupported Content-type", client.FresdeskUnsupportedContentError), - (429, "Rate Limit Exceeded", client.FresdeskRateLimitError), - (500, "Unexpected Server Error", client.FresdeskServerError), + (400, "Client or Validation Error", client.FreshdeskValidationError), + (401, "Authentication Failure", client.FreshdeskAuthenticationError), + (403, "Access Denied", client.FreshdeskAccessDeniedError), + (404, "Requested Resource not Found", client.FreshdeskNotFoundError), + (405, "Method not allowed", client.FreshdeskMethodNotAllowedError), + (406, "Unsupported Accept Header", client.FreshdeskUnsupportedAcceptHeaderError), + (409, "AInconsistent/Conflicting State", client.FreshdeskConflictingStateError), + (415, "Unsupported Content-type", client.FreshdeskUnsupportedContentError), + (429, "Rate Limit Exceeded", client.FreshdeskRateLimitError), + (500, "Unexpected Server Error", client.FreshdeskServerError), ]) def test_error_response_message(self, error_code, message, error): """ @@ -76,7 +77,7 @@ def json_decoder_error(self): mock_response = get_response(400, {"description": "Client or Validation Error"}) mock_response._content = "ABC".encode() expected_message = "HTTP-error-code: {}, Error: {}".format(400, "Client or Validation Error") - with self.assertRaises(client.FresdeskValidationError) as e: + with self.assertRaises(client.FreshdeskValidationError) as e: raise_for_error(mock_response) # Verify that an error message is expected @@ -91,7 +92,7 @@ class TestBackoffHandling(unittest.TestCase): """ @parameterized.expand([ - (lambda *x,**y:get_response(500), client.FresdeskServerError), + (lambda *x,**y:get_response(500), client.FreshdeskServerError), (lambda *x,**y:get_response(503), client.Server5xxError), # Unknown 5xx error (ConnectionError, ConnectionError), (TimeoutError, TimeoutError), @@ -149,3 +150,26 @@ def test_rate_limite_not_exceeded(self, mock_sleep, mock_request): # Verify that `requests` method is called once. self.assertEqual(mock_request.call_count, 1) mock_request.assert_called_with(mock.ANY, timeout=client.DEFAULT_TIMEOUT) + + +class TestSkip404(unittest.TestCase): + """ + Test handling of 404 for a specific child. + """ + + @mock.patch("tap_freshdesk.streams.LOGGER.warning") + @mock.patch("tap_freshdesk.client.FreshdeskClient.request") + def test_child_stream_skips(self, mock_request, mock_logger): + """ + Test that on 404 error is skipped for `TimeEntries`. + """ + stream = TimeEntries() + _client = mock.Mock() + _client.base_url = "" + _client.request.side_effect = client.FreshdeskNotFoundError + + stream.parent_id = 10 + stream.sync_obj({}, "START_DATE", _client, {}, [], []) + + # Verify that error is not raised and the warning logger is called. + mock_logger.assert_called_with("Could not retrieve time entries for ticket id 10. This may be caused by tickets marked as spam or deleted.") diff --git a/tests/unittests/test_timeout.py b/tests/unittests/test_timeout.py new file mode 100644 index 0000000..21501b2 --- /dev/null +++ b/tests/unittests/test_timeout.py @@ -0,0 +1,56 @@ +import unittest +from unittest import mock +from parameterized import parameterized +from tap_freshdesk.client import FreshdeskClient, DEFAULT_TIMEOUT + + +class TestTimeOut(unittest.TestCase): + """ + Test `set_timeout` method of the client. + """ + + @parameterized.expand([ + ["integer_value", 10, 10], + ["float_value", 100.5, 100], + ["string_integer", "10", 10], + ["string_float", "100.5", 100], + ]) + def test_timeout_values(self, name, timeout_value, expected_value): + """ + Test that for the valid value of timeout, + No exception is raised and the expected value is set. + """ + config = {"timeout": timeout_value} + _client = FreshdeskClient(config) + + # Verify timeout value is expected + self.assertEqual(_client.timeout, expected_value) + + @parameterized.expand([ + ["integer_zero", 0], + ["float_zero", 0.0], + ["string_zero", "0"], + ["string_float_zero", "0.0"], + ["string_alphabate", "abc"], + ]) + def test_invalid_value(self, name, timeout_value): + """ + Test that for invalid value exception is raised. + """ + config = {"timeout": timeout_value} + with self.assertRaises(Exception) as e: + _client = FreshdeskClient(config) + + # Verify that the exception message is expected. + self.assertEqual(str(e.exception), "The entered timeout is invalid, it should be a valid none-zero integer.") + + + def test_none_value(self): + """ + Test if no timeout is not passed in the config, then set it to the default value. + """ + config = {} + _client = FreshdeskClient(config) + + # Verify that the default timeout value is set. + self.assertEqual(_client.timeout, DEFAULT_TIMEOUT) From 6a22d24fa3d78f7fb892e4cc62f39269080a9456 Mon Sep 17 00:00:00 2001 From: prijendev Date: Thu, 25 Aug 2022 17:11:14 +0530 Subject: [PATCH 4/9] Added import in streams.py --- tap_freshdesk/streams.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tap_freshdesk/streams.py b/tap_freshdesk/streams.py index 90f41dc..bb4ad8a 100644 --- a/tap_freshdesk/streams.py +++ b/tap_freshdesk/streams.py @@ -2,6 +2,7 @@ from datetime import datetime as dt import singer from singer.bookmarks import get_bookmark +from tap_freshdesk.client import FreshdeskNotFoundError LOGGER = singer.get_logger() From 6045881ebc71cc2522c9fb2b0c6f74c00868ce95 Mon Sep 17 00:00:00 2001 From: prijendev Date: Thu, 25 Aug 2022 18:56:00 +0530 Subject: [PATCH 5/9] Updated exception handling. --- README.md | 2 + tap_freshdesk/client.py | 17 +++-- tap_freshdesk/streams.py | 3 +- tests/unittests/test_exception_handling.py | 80 ++++++++++++---------- tests/unittests/test_timeout.py | 16 ++--- 5 files changed, 64 insertions(+), 54 deletions(-) diff --git a/README.md b/README.md index 1f5849a..64578e5 100644 --- a/README.md +++ b/README.md @@ -36,11 +36,13 @@ This tap: Create a JSON file called `config.json` containing the api token you just found and the subdomain to your Freshdesk account. The subdomain will take the format `subdomain.freshdesk.com`. + The `request_timeout` is an optional parameter to set the timeout for requests. Default: 300 seconds ```json { "api_key": "your-api-token", "domain": "subdomain", + "request_timeout": 100, "start_date": "2017-01-17T20:32:05Z" } ``` diff --git a/tap_freshdesk/client.py b/tap_freshdesk/client.py index 1ad832a..a369235 100644 --- a/tap_freshdesk/client.py +++ b/tap_freshdesk/client.py @@ -7,7 +7,7 @@ LOGGER = singer.get_logger() BASE_URL = "https://{}.freshdesk.com" -DEFAULT_TIMEOUT = 300 +REQUEST_TIMEOUT = 300 class FreshdeskException(Exception): pass @@ -57,7 +57,7 @@ class FreshdeskServerError(Server5xxError): }, 403: { "raise_exception": FreshdeskAccessDeniedError, - "message": "The agent whose credentials were used in making this request was not authorized to perform this API call." + "message": "The agent whose credentials were used to make this request was not authorized to perform this API call." }, 404: { "raise_exception": FreshdeskNotFoundError, @@ -109,6 +109,9 @@ def raise_for_error(response): else: exc = ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("raise_exception", FreshdeskException) message = response_json.get("description", ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("message", "Unknown Error")) + code = response_json.get("code", "") + if code: + error_code = f"{str(error_code)} {code}" formatted_message = "HTTP-error-code: {}, Error: {}".format(error_code, message) raise exc(formatted_message) from None @@ -121,7 +124,7 @@ def __init__(self, config): self.config = config self.session = requests.Session() self.base_url = BASE_URL.format(config.get("domain")) - self.timeout = DEFAULT_TIMEOUT + self.timeout = REQUEST_TIMEOUT self.set_timeout() def __enter__(self): @@ -137,10 +140,10 @@ def set_timeout(self): Set timeout value from config, if the value is passed. Else raise an exception. """ - timeout = self.config.get("timeout", DEFAULT_TIMEOUT) + timeout = self.config.get("request_timeout", REQUEST_TIMEOUT) if ((type(timeout) in [int, float]) or (type(timeout)==str and timeout.replace('.', '', 1).isdigit())) and float(timeout): - self.timeout = int(float(timeout)) + self.timeout = float(timeout) else: raise Exception("The entered timeout is invalid, it should be a valid none-zero integer.") @@ -151,7 +154,7 @@ def check_access_token(self): self.request(self.base_url+"/api/v2/roles", {"per_page": 1, "page": 1}) @backoff.on_exception(backoff.expo, - (TimeoutError, ConnectionError, Server5xxError), + (requests.Timeout, requests.ConnectionError, Server5xxError), max_tries=5, factor=2) @utils.ratelimit(1, 2) @@ -165,7 +168,7 @@ def request(self, url, params={}): req = requests.Request('GET', url, params=params, auth=(self.config['api_key'], ""), headers=headers).prepare() LOGGER.info("GET {}".format(req.url)) - response = self.session.send(req, timeout=DEFAULT_TIMEOUT) + response = self.session.send(req, timeout=self.timeout) # Call the function again if the rate limit is exceeded if 'Retry-After' in response.headers: diff --git a/tap_freshdesk/streams.py b/tap_freshdesk/streams.py index bb4ad8a..2242bc4 100644 --- a/tap_freshdesk/streams.py +++ b/tap_freshdesk/streams.py @@ -117,7 +117,7 @@ def write_records(self, catalog, state, selected_streams, start_date, data, max_ child_max_bookmark = get_bookmark(state, child_obj.tap_stream_id, child_obj.replication_keys[0], start_date) if child in selected_streams: child_obj.parent_id = row['id'] - child_max_bookmark = max(child_max_bookmark, child_obj.sync_obj(state, start_date, client, catalog, selected_streams, streams_to_sync)) + child_max_bookmark = max(child_max_bookmark, child_obj.sync_obj(state, start_date, client, catalog, selected_streams, streams_to_sync) or "") child_max_bookmarks[child] = child_max_bookmark return max_bookmark, child_max_bookmarks @@ -261,7 +261,6 @@ def sync_obj(self, state, start_date, client, catalog, selected_streams, streams # Skipping 404 error as it is returned for deleted tickets and spam LOGGER.warning("Could not retrieve time entries for ticket id {}. This may be caused by tickets " "marked as spam or deleted.".format(self.parent_id)) - pass STREAMS = { diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py index a1f01b4..8bcc222 100644 --- a/tests/unittests/test_exception_handling.py +++ b/tests/unittests/test_exception_handling.py @@ -25,17 +25,17 @@ class TestExceptionHanfling(unittest.TestCase): """ @parameterized.expand([ - (400, client.FreshdeskValidationError), - (401, client.FreshdeskAuthenticationError), - (403, client.FreshdeskAccessDeniedError), - (404, client.FreshdeskNotFoundError), - (405, client.FreshdeskMethodNotAllowedError), - (406, client.FreshdeskUnsupportedAcceptHeaderError), - (409, client.FreshdeskConflictingStateError), - (415, client.FreshdeskUnsupportedContentError), - (429, client.FreshdeskRateLimitError), - (500, client.FreshdeskServerError), - (503, client.Server5xxError), # Unknown 5xx error + [400, client.FreshdeskValidationError], + [401, client.FreshdeskAuthenticationError], + [403, client.FreshdeskAccessDeniedError], + [404, client.FreshdeskNotFoundError], + [405, client.FreshdeskMethodNotAllowedError], + [406, client.FreshdeskUnsupportedAcceptHeaderError], + [409, client.FreshdeskConflictingStateError], + [415, client.FreshdeskUnsupportedContentError], + [429, client.FreshdeskRateLimitError], + [500, client.FreshdeskServerError], + [503, client.Server5xxError], # Unknown 5xx error ]) def test_custom_error_message(self, error_code, error): """ @@ -50,31 +50,36 @@ def test_custom_error_message(self, error_code, error): self.assertEqual(str(e.exception), expected_message) @parameterized.expand([ - (400, "Client or Validation Error", client.FreshdeskValidationError), - (401, "Authentication Failure", client.FreshdeskAuthenticationError), - (403, "Access Denied", client.FreshdeskAccessDeniedError), - (404, "Requested Resource not Found", client.FreshdeskNotFoundError), - (405, "Method not allowed", client.FreshdeskMethodNotAllowedError), - (406, "Unsupported Accept Header", client.FreshdeskUnsupportedAcceptHeaderError), - (409, "AInconsistent/Conflicting State", client.FreshdeskConflictingStateError), - (415, "Unsupported Content-type", client.FreshdeskUnsupportedContentError), - (429, "Rate Limit Exceeded", client.FreshdeskRateLimitError), - (500, "Unexpected Server Error", client.FreshdeskServerError), + [400, "Client or Validation Error", None, client.FreshdeskValidationError], + [401, "Authentication Failure", "invalid_credentials", client.FreshdeskAuthenticationError], + [403, "Access Denied", "access_denied", client.FreshdeskAccessDeniedError], + [404, "Requested Resource not Found", None, client.FreshdeskNotFoundError], + [405, "Method not allowed", None, client.FreshdeskMethodNotAllowedError], + [406, "Unsupported Accept Header", None, client.FreshdeskUnsupportedAcceptHeaderError], + [409, "AInconsistent/Conflicting State", "inconsistent_state", client.FreshdeskConflictingStateError], + [415, "Unsupported Content-type", "invalid_json", client.FreshdeskUnsupportedContentError], + [429, "Rate Limit Exceeded", None, client.FreshdeskRateLimitError], + [500, "Unexpected Server Error", None, client.FreshdeskServerError], + [503, "Service Unavailable", None, client.Server5xxError], # Unknown 5xx error ]) - def test_error_response_message(self, error_code, message, error): + def test_error_response_message(self, status_code, message, code, error): """ Test that error is thrown with description in the response. """ + + error_code = status_code + if code: + error_code = f"{str(status_code)} {code}" expected_message = "HTTP-error-code: {}, Error: {}".format(error_code, message) with self.assertRaises(error) as e: - raise_for_error(get_response(error_code, {"description": message})) + raise_for_error(get_response(status_code, {"description": message, "code": code})) # Verify that an error message is expected self.assertEqual(str(e.exception), expected_message) def json_decoder_error(self): """Test for invalid json response, tap does not throw JSON decoder error.""" - mock_response = get_response(400, {"description": "Client or Validation Error"}) + mock_response = get_response(400, {"description": "Client or Validation Error", "code": None}) mock_response._content = "ABC".encode() expected_message = "HTTP-error-code: {}, Error: {}".format(400, "Client or Validation Error") with self.assertRaises(client.FreshdeskValidationError) as e: @@ -84,22 +89,23 @@ def json_decoder_error(self): self.assertEqual(str(e.exception), expected_message) -@mock.patch("requests.Session.send") -@mock.patch("time.sleep") + class TestBackoffHandling(unittest.TestCase): """ Test backoff handling for all 5xx, timeout and connection error. """ @parameterized.expand([ - (lambda *x,**y:get_response(500), client.FreshdeskServerError), - (lambda *x,**y:get_response(503), client.Server5xxError), # Unknown 5xx error - (ConnectionError, ConnectionError), - (TimeoutError, TimeoutError), + ["For error 500", lambda *x,**y: get_response(500), client.FreshdeskServerError], + ["For 503 (unknown 5xx error)", lambda *x,**y:get_response(503), client.Server5xxError], # Unknown 5xx error + ["For Connection Error", requests.ConnectionError, requests.ConnectionError], + ["For timeour Error", requests.Timeout, requests.Timeout], ]) - def test_backoff(self, mock_sleep, mock_request, mock_response, error): + @mock.patch("requests.Session.send") + @mock.patch("time.sleep") + def test_backoff(self, name, mock_response, error, mock_sleep, mock_request): """ - Test that for 500, timeout and connection error `request` method will backoff 5 times. + Test that for 500, timeout and connection error `request` method will back off 5 times. """ mock_request.side_effect = mock_response config = {"user_agent": "SAMPLE_AGENT", "api_key": "TEST_API_KEY"} @@ -119,9 +125,9 @@ class TestRateLimitHandling(unittest.TestCase): """ @parameterized.expand([ - ("30",), - ("5",), - ("50",), + ["30"], + ["5"], + ["50"], ]) def test_rate_limit_exceeded(self, mock_sleep, mock_request, retry_seconds): """ @@ -138,7 +144,7 @@ def test_rate_limit_exceeded(self, mock_sleep, mock_request, retry_seconds): # Verify that `time.sleep` was called for 'Retry-After' seconds from the header. mock_sleep.assert_any_call(int(retry_seconds)) - def test_rate_limite_not_exceeded(self, mock_sleep, mock_request): + def test_rate_limit_not_exceeded(self, mock_sleep, mock_request): """ Test that the function will not retry for the success response. """ @@ -149,7 +155,7 @@ def test_rate_limite_not_exceeded(self, mock_sleep, mock_request): # Verify that `requests` method is called once. self.assertEqual(mock_request.call_count, 1) - mock_request.assert_called_with(mock.ANY, timeout=client.DEFAULT_TIMEOUT) + mock_request.assert_called_with(mock.ANY, timeout=client.REQUEST_TIMEOUT) class TestSkip404(unittest.TestCase): diff --git a/tests/unittests/test_timeout.py b/tests/unittests/test_timeout.py index 21501b2..6885d93 100644 --- a/tests/unittests/test_timeout.py +++ b/tests/unittests/test_timeout.py @@ -1,7 +1,7 @@ import unittest from unittest import mock from parameterized import parameterized -from tap_freshdesk.client import FreshdeskClient, DEFAULT_TIMEOUT +from tap_freshdesk.client import FreshdeskClient, REQUEST_TIMEOUT class TestTimeOut(unittest.TestCase): @@ -10,17 +10,17 @@ class TestTimeOut(unittest.TestCase): """ @parameterized.expand([ - ["integer_value", 10, 10], - ["float_value", 100.5, 100], - ["string_integer", "10", 10], - ["string_float", "100.5", 100], + ["integer_value", 10, 10.0], + ["float_value", 100.5, 100.5], + ["string_integer", "10", 10.0], + ["string_float", "100.5", 100.5], ]) def test_timeout_values(self, name, timeout_value, expected_value): """ Test that for the valid value of timeout, No exception is raised and the expected value is set. """ - config = {"timeout": timeout_value} + config = {"request_timeout": timeout_value} _client = FreshdeskClient(config) # Verify timeout value is expected @@ -37,7 +37,7 @@ def test_invalid_value(self, name, timeout_value): """ Test that for invalid value exception is raised. """ - config = {"timeout": timeout_value} + config = {"request_timeout": timeout_value} with self.assertRaises(Exception) as e: _client = FreshdeskClient(config) @@ -53,4 +53,4 @@ def test_none_value(self): _client = FreshdeskClient(config) # Verify that the default timeout value is set. - self.assertEqual(_client.timeout, DEFAULT_TIMEOUT) + self.assertEqual(_client.timeout, REQUEST_TIMEOUT) From 744d5dbd53a871c1048a8a12e29b73740e426b4d Mon Sep 17 00:00:00 2001 From: prijendev Date: Fri, 26 Aug 2022 12:30:09 +0530 Subject: [PATCH 6/9] Updated timeout value in README. --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 64578e5..cc91a9b 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ This tap: { "api_key": "your-api-token", "domain": "subdomain", - "request_timeout": 100, + "request_timeout": 300, "start_date": "2017-01-17T20:32:05Z" } ``` From 0489943b8bb439954a7794b035cf8fecc90da277 Mon Sep 17 00:00:00 2001 From: prijendev Date: Fri, 2 Sep 2022 10:37:36 +0530 Subject: [PATCH 7/9] Updated unit test cases. --- tests/unittests/test_exception_handling.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py index 8bcc222..3c72ee5 100644 --- a/tests/unittests/test_exception_handling.py +++ b/tests/unittests/test_exception_handling.py @@ -77,11 +77,11 @@ def test_error_response_message(self, status_code, message, code, error): # Verify that an error message is expected self.assertEqual(str(e.exception), expected_message) - def json_decoder_error(self): + def test_json_decoder_error(self): """Test for invalid json response, tap does not throw JSON decoder error.""" - mock_response = get_response(400, {"description": "Client or Validation Error", "code": None}) + mock_response = get_response(400) mock_response._content = "ABC".encode() - expected_message = "HTTP-error-code: {}, Error: {}".format(400, "Client or Validation Error") + expected_message = "HTTP-error-code: {}, Error: {}".format(400, "The request body/query string is not in the correct format.") with self.assertRaises(client.FreshdeskValidationError) as e: raise_for_error(mock_response) @@ -99,7 +99,7 @@ class TestBackoffHandling(unittest.TestCase): ["For error 500", lambda *x,**y: get_response(500), client.FreshdeskServerError], ["For 503 (unknown 5xx error)", lambda *x,**y:get_response(503), client.Server5xxError], # Unknown 5xx error ["For Connection Error", requests.ConnectionError, requests.ConnectionError], - ["For timeour Error", requests.Timeout, requests.Timeout], + ["For timeout Error", requests.Timeout, requests.Timeout], ]) @mock.patch("requests.Session.send") @mock.patch("time.sleep") From 3d7dc861cb8df6b94a865262e7ed8f8d6477e9ae Mon Sep 17 00:00:00 2001 From: prijendev Date: Fri, 2 Sep 2022 17:23:19 +0530 Subject: [PATCH 8/9] Resolved your comments. --- tap_freshdesk/client.py | 46 +++++++++++----------- tap_freshdesk/streams.py | 7 ++-- tests/unittests/test_exception_handling.py | 10 ++--- 3 files changed, 31 insertions(+), 32 deletions(-) diff --git a/tap_freshdesk/client.py b/tap_freshdesk/client.py index 1efb84f..12671ad 100644 --- a/tap_freshdesk/client.py +++ b/tap_freshdesk/client.py @@ -3,6 +3,7 @@ import backoff import requests import singer +from simplejson import JSONDecodeError from tap_freshdesk import utils LOGGER = singer.get_logger() @@ -86,34 +87,31 @@ class FreshdeskServerError(Server5xxError): 500: { "raise_exception": FreshdeskServerError, "message": "Unexpected Server Error." - }, + } } def raise_for_error(response): """ Retrieve the error code and the error message from the response and return custom exceptions accordingly. """ + error_code = response.status_code + # Forming a response message for raising a custom exception try: - response.raise_for_status() - except (requests.HTTPError) as error: - error_code = response.status_code - # Forming a response message for raising a custom exception - try: - response_json = response.json() - except Exception: - response_json = {} - - if error_code not in ERROR_CODE_EXCEPTION_MAPPING and error_code > 500: - # Raise `Server5xxError` for all 5xx unknown error - exc = Server5xxError - else: - exc = ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("raise_exception", FreshdeskException) - message = response_json.get("description", ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("message", "Unknown Error")) - code = response_json.get("code", "") - if code: - error_code = f"{str(error_code)} {code}" - formatted_message = "HTTP-error-code: {}, Error: {}".format(error_code, message) - raise exc(formatted_message) from None + response_json = response.json() + except JSONDecodeError: + response_json = {} + + if error_code not in ERROR_CODE_EXCEPTION_MAPPING and error_code > 500: + # Raise `Server5xxError` for all 5xx unknown error + exc = Server5xxError + else: + exc = ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("raise_exception", FreshdeskException) + message = response_json.get("description", ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("message", "Unknown Error")) + code = response_json.get("code", "") + if code: + error_code = f"{str(error_code)} {code}" + formatted_message = "HTTP-error-code: {}, Error: {}".format(error_code, message) + raise exc(formatted_message) from None class FreshdeskClient: """ @@ -137,12 +135,12 @@ def __exit__(self, exception_type, exception_value, traceback): def set_timeout(self): """ - Set timeout value from config, if the value is passed. + Set timeout value from config, if the value is passed. Else raise an exception. """ timeout = self.config.get("request_timeout", REQUEST_TIMEOUT) - if ((type(timeout) in [int, float]) or - (type(timeout)==str and timeout.replace('.', '', 1).isdigit())) and float(timeout): + if ((type(timeout) in [int, float]) or + (isinstance(timeout, str) and timeout.replace('.', '', 1).isdigit())) and float(timeout): self.timeout = float(timeout) else: raise Exception("The entered timeout is invalid, it should be a valid none-zero integer.") diff --git a/tap_freshdesk/streams.py b/tap_freshdesk/streams.py index 876aaaa..2c6cd75 100644 --- a/tap_freshdesk/streams.py +++ b/tap_freshdesk/streams.py @@ -110,7 +110,7 @@ def write_records(self, catalog, state, selected_streams, start_date, data, max_ stream_metadata = singer.metadata.to_map(stream_catalog['metadata']) for row in data: if self.tap_stream_id in selected_streams and row[self.replication_keys[0]] >= bookmark: - # Custom fields are expected to be strings, but sometimes the API sends + # Custom fields are expected to be strings, but sometimes the API sends # booleans. We cast those to strings to match the schema. if 'custom_fields' in row: row['custom_fields'] = self.transform_dict(row['custom_fields'], force_str=self.force_str) @@ -307,8 +307,9 @@ def sync_obj(self, state, start_date, client, catalog, selected_streams, streams return super().sync_obj(state, start_date, client, catalog, selected_streams, streams_to_sync, predefined_filter) except FreshdeskNotFoundError: # Skipping 404 error as it is returned for deleted tickets and spam - LOGGER.warning("Could not retrieve time entries for ticket id {}. This may be caused by tickets " - "marked as spam or deleted.".format(self.parent_id)) + LOGGER.warning("Could not retrieve time entries for ticket id %s. This may be caused by tickets " + "marked as spam or deleted.", self.parent_id) + return None STREAMS = { diff --git a/tests/unittests/test_exception_handling.py b/tests/unittests/test_exception_handling.py index 3c72ee5..4384ba6 100644 --- a/tests/unittests/test_exception_handling.py +++ b/tests/unittests/test_exception_handling.py @@ -77,11 +77,11 @@ def test_error_response_message(self, status_code, message, code, error): # Verify that an error message is expected self.assertEqual(str(e.exception), expected_message) - def test_json_decoder_error(self): + def json_decoder_error(self): """Test for invalid json response, tap does not throw JSON decoder error.""" - mock_response = get_response(400) + mock_response = get_response(400, {"description": "Client or Validation Error", "code": None}) mock_response._content = "ABC".encode() - expected_message = "HTTP-error-code: {}, Error: {}".format(400, "The request body/query string is not in the correct format.") + expected_message = "HTTP-error-code: {}, Error: {}".format(400, "Client or Validation Error") with self.assertRaises(client.FreshdeskValidationError) as e: raise_for_error(mock_response) @@ -99,7 +99,7 @@ class TestBackoffHandling(unittest.TestCase): ["For error 500", lambda *x,**y: get_response(500), client.FreshdeskServerError], ["For 503 (unknown 5xx error)", lambda *x,**y:get_response(503), client.Server5xxError], # Unknown 5xx error ["For Connection Error", requests.ConnectionError, requests.ConnectionError], - ["For timeout Error", requests.Timeout, requests.Timeout], + ["For timeour Error", requests.Timeout, requests.Timeout], ]) @mock.patch("requests.Session.send") @mock.patch("time.sleep") @@ -178,4 +178,4 @@ def test_child_stream_skips(self, mock_request, mock_logger): stream.sync_obj({}, "START_DATE", _client, {}, [], []) # Verify that error is not raised and the warning logger is called. - mock_logger.assert_called_with("Could not retrieve time entries for ticket id 10. This may be caused by tickets marked as spam or deleted.") + mock_logger.assert_called_with("Could not retrieve time entries for ticket id %s. This may be caused by tickets marked as spam or deleted.", 10) From a0c34a5cdc990fdf038b9bf0ee3b67cb69533bc9 Mon Sep 17 00:00:00 2001 From: prijendev Date: Wed, 21 Sep 2022 12:00:50 +0530 Subject: [PATCH 9/9] Resovled missing-class-string pylint. --- tap_freshdesk/client.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tap_freshdesk/client.py b/tap_freshdesk/client.py index 04a7a0f..79335a9 100644 --- a/tap_freshdesk/client.py +++ b/tap_freshdesk/client.py @@ -11,40 +11,40 @@ DEFAULT_PAGE_SIZE = 100 class FreshdeskException(Exception): - pass + """Custom error class for all the freshdesk errors.""" class FreshdeskValidationError(FreshdeskException): - pass + """Custom error class for validation error.""" class FreshdeskAuthenticationError(FreshdeskException): - pass + """Custom error class for authentication error.""" class FreshdeskAccessDeniedError(FreshdeskException): - pass + """Custom error class for access denied error.""" class FreshdeskNotFoundError(FreshdeskException): - pass + """Custom error class for not found error.""" class FreshdeskMethodNotAllowedError(FreshdeskException): - pass + """Custom error class for method not allowed.""" class FreshdeskUnsupportedAcceptHeaderError(FreshdeskException): - pass + """Custom error class for unsupported accept error.""" class FreshdeskConflictingStateError(FreshdeskException): - pass + """Custom error class for conflicting state.""" class FreshdeskUnsupportedContentError(FreshdeskException): - pass + """Custom error class for unsupported content.""" class FreshdeskRateLimitError(FreshdeskException): - pass + """Custom error class for rate limit error.""" class Server5xxError(FreshdeskException): - pass + """Custom error class for all the 5xx errors.""" class FreshdeskServerError(Server5xxError): - pass + """Custom error class for 500 server error""" ERROR_CODE_EXCEPTION_MAPPING = {