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.

Sign up for GitHub

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

Jump to bottom

TDL-6829 added backoff for incompleteread error #144

Merged
merged 3 commits into from May 23, 2023
Merged

TDL-6829 added backoff for incompleteread error #144

merged 3 commits into from May 23, 2023

Conversation

namrata270998
Copy link
Contributor

@namrata270998 namrata270998 commented Mar 30, 2022 •
edited

Description of change

  • Added backoff for 5 times in case of http.client.IncompleteRead error
  • Added unittests to ensure the same

QA steps

  • Verify that the tap backoffs 5 times for http.client.IncompleteRead error

Risks

Rollback steps

  • revert this branch

@namrata270998 namrata270998 changed the base branch from master to crest-work March 30, 2022 13:54
dbshah1212
dbshah1212 reviewed Mar 31, 2022
View reviewed changes
tap_shopify/streams/base.py
@@ -77,6 +78,10 @@ def is_timeout_error(error_raised):
return True

def shopify_error_handling(fnc):
@backoff.on_exception(backoff.expo, # IncompleteRead error raised
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 merge this inside below backoff?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we cannot merge it as it has giveup function here

@RushiT0122 RushiT0122 changed the base branch from crest-work to master May 22, 2023 13:18
@RushiT0122 RushiT0122 force-pushed the TDL-6829-retry-IncompleteReaderror branch from 3a30901 to 79101ce Compare May 22, 2023 13:26
@RushiT0122 RushiT0122 force-pushed the TDL-6829-retry-IncompleteReaderror branch from 79101ce to b0f4c3c Compare May 22, 2023 13:39
Vi6hal
Vi6hal approved these changes May 23, 2023
View reviewed changes
Copy link
Member

@Vi6hal Vi6hal left a comment

Choose a reason for hiding this comment

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

👍🏻 LGTM

@RushiT0122 RushiT0122 merged commit 1b26fbc into master May 23, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Vi6hal Vi6hal Vi6hal approved these changes

@dbshah1212 dbshah1212 dbshah1212 approved these changes

@leslievandemark leslievandemark leslievandemark approved these changes

@kspeer825 kspeer825 kspeer825 approved these changes

@savan-chovatiya savan-chovatiya savan-chovatiya approved these changes

@RushiT0122 RushiT0122 RushiT0122 approved these changes

@KrisPersonal KrisPersonal Awaiting requested review from KrisPersonal

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@namrata270998 @Vi6hal @dbshah1212 @leslievandemark @kspeer825 @savan-chovatiya @RushiT0122