TDL-19745 unittests and retrylogic for 500 exceptions #11
Conversation
…nger-io/tap-dixa into TDL-19674-bookmarking_strategy_bugfix
…inger-io/tap-dixa into TDL-19668-overlapping-timeslots_bugfix
Co-authored-by: Collin Simon <cosimon@users.noreply.github.com>
Co-authored-by: Collin Simon <cosimon@users.noreply.github.com>
|
@rdeshmukh15 can u add Unit Tests stage in circleCI config.yml. |
|
@kethan1122 : Added the changes suggested in config.yaml |
| response = IncrementalStream.get_interval(self) | ||
| except InvalidInterval as e : | ||
| expected_error_message = "invalid interval provided" | ||
| assert str(e) == expected_error_message |
There was a problem hiding this comment.
Use unittest assert methods
There was a problem hiding this comment.
Done the changes as suggested.
tests/unittests/test_backoff.py
Outdated
| def mocked_failed_429_request(*args, **kwargs): | ||
| return Mockresponse('', 429, headers={}, raise_error=True) | ||
|
|
||
| @patch("time.sleep") | ||
| @patch("requests.Session.request", side_effect=mocked_failed_429_request) | ||
| class Test_Client(unittest.TestCase): | ||
|
|
||
| def test_too_many_requests_429_error(self, mocked_send, mocked_sleep): | ||
| client = Client(api_token="test") | ||
|
|
||
| try: | ||
| # Verifying if the custom exception 'DixaClient429Error' is raised on receiving status code 429 | ||
| _ = client.get("https://test.com", "/test") | ||
| except DixaClient429Error: #as api_error: | ||
| pass | ||
|
|
||
| # Verifying the retry is happening thrice for the 429 exception | ||
| self.assertEquals(mocked_send.call_count, 3) |
There was a problem hiding this comment.
Observed that we have not implemented backoff and retry for 5XX in the tap, implement that and add respective unittests as well.
- verify exceptions for applicable http error codes
cngpowered
changed the title
|
HTTPTimeout is not implemented, please implement it. |
| try: | ||
| self.client_obj.get("https://test.com", "/test") | ||
| except exceptions.DixaClientError as _: | ||
| self.assertEqual(str(_), "Server Error") |
There was a problem hiding this comment.
Can we have descriptive error message for 5XX than "Server Error"?
There was a problem hiding this comment.
Made the changes as per suggestion.
tap_dixa/exceptions.py
Outdated
| class DixaClient5xxError(DixaClientError): | ||
| pass |
There was a problem hiding this comment.
Move DixaClient5xxError class to below DixaClient4xxError.
Description of change
Covered below scenarios in Unit Test Cases :
Tap Changes:
Manual QA steps
Risks
Rollback steps