Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TDL-19844: Add custom exception handling #54

Merged
merged 13 commits into from
Sep 26, 2022

Conversation

savan-chovatiya
Copy link
Contributor

Description of change

TDL-19844: Add custom exception handling

  • Added customer exception codes map to handle the exception and custom message if message not found in API response.
  • Kept retrying mechanism as it is for 5xx and other codes.

Manual QA steps

  • Verified that exception is raised with a proper error message if a message is present in API response for error.
  • Verified that exception is raised with a custom message if API response does not have a message.

Risks

Rollback steps

  • revert this branch


exception = ERROR_CODE_EXCEPTION_MAPPING.get(error_code, {}).get("raise_exception")
if not exception:
if error_code >= 500:

Choose a reason for hiding this comment

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

Suggested change
if error_code >= 500:
exception = Server5xxError if error_code >= 500 else IntercomError

self.assertEqual(str(e.exception), expected_message)
self.assertEqual(mocked_402_successful.call_count, 1)

def test_403_custom_response_message(self, mocked_api_token, mocked_403_successful, mocked_sleep):

Choose a reason for hiding this comment

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

Instead of writing tests for each error use @parameterized.expand, ref: https://github.com/singer-io/tap-recharge/pull/30/files

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated unit test cases with @parameterized.expand,

else:
err_msg = errors[0].get('message')
intercom_error_code = errors[0].get('code')
message = "HTTP-error-code: {}, Error:{}".format(status_code,err_msg)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we also include intercom_error_code in the error message?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Included intercom_error_code in the error message.

@hpatel41 hpatel41 merged commit 6d0c818 into crest-master Sep 26, 2022
@hpatel41 hpatel41 mentioned this pull request Sep 26, 2022
sgandhi1311 added a commit that referenced this pull request Dec 12, 2022
* TDL-15031: Add addressable list fields (Copy of PR #34) (#58)

* added addressable list fields in for contacts stream

* resolve pylint error

* resolve pylint error

* resolved multiple record bug in transform_times

* resolved pylint error

* updated the code to do API call if we have some record for addresable fields and updated bookmark test case

* updated bookmark test case

* updated the code to do API calls if has_more is true and store only ids for addressable list fields

* resolved pylint error

* reverted notes schema change

* merged skipping condiitons

* added unittest

* updated variable in unittest

* TDL-19860: Need to update Primary keys for Company_Attributes & Contacts streams (#56)

* updated PK for company_attributes and used hash of records as the PK

* updated setup.py to install parameterized for unittests

* added activate version for company and contact attributes stream

* updated test case and code

* updated test cases

* fix CCi build failure

* updated code and comments

* updated README file

* TDL-19844: Add custom exception handling (#54)

* Added custom exception handling

* Make pylint happy

* Added unit tests

* Added backoff for 5xx error

* Added/updated unit tests

* Added/updated unit tests

* Updated unit tests

* Updated cconfig.yml

* Added unit test case for json decode

* Added unit test case for backoff

* Added error code in logger message

* make pylint happy

Co-authored-by: harshpatel4crest <harsh.patel4@crestdatasys.com>

* TDL-19859: Upgrade API version and related new fields (#53)

* upgraded intercom to V2.5 and added missing fields

* added all fields test case

* resolved integration test failure

* updated api version in readme file

* TDl-19845: Add missing tap tester & unit tests (#57)

* Added missing integration & unit tests

* Fixed unit test failure

* Fixed unit test failure

* Updated unit test

* Added unit test for init and transform

* Updated expected PKs

* Fixed unit test

* Updated inturrpted_sync test

* Updated transform unit test

* Resolved review comment

* Resolved review comment and added workaround for live data

* Removed get_logger

* Removed get_logger

* updated pagination and interrupted sync test cases

* updated tap-tester tests

* updated tap-tester test code

* resolved unittest failure

Co-authored-by: harshpatel4crest <harsh.patel4@crestdatasys.com>

* TDL-19892: Use common API calls for parent's data for parent/child streams (#55)

* added code change to use common API call for parent-child streams

* resolved pylint error

* resolved unittest error

* updated the code

* added unittests

* fix unittest failure

* resolve review comments

* updated the code

* updated the code

* updated comment

* updated the location of get_parent_data method for admins and admin_list

* added comment for admin and admin_list

* updated comment and unittest

* resolve unittest failure

* updated comment and removed redundant code of returning state when parent and child are not selected

* added tap-tester test for parent-child syncing

* updated bookmark test case

* updated test case name

* updated setup.py file

* resolve pylint error

* update setup.py and changelog.md

Co-authored-by: savan-chovatiya <80703490+savan-chovatiya@users.noreply.github.com>
Co-authored-by: Sourabh Gandhi <sgandhi@talend.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants