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-6829 added backoff for incompleteread error #144

Merged
merged 3 commits into from
May 23, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
# Changelog

## 1.7.4
* Add backoff for IncompleteRead [#144](https://github.com/singer-io/tap-shopify/pull/144)

## 1.7.3
* Update interrupted sync bookmark strategy [#166](https://github.com/singer-io/tap-shopify/pull/166)

2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
@@ -3,7 +3,7 @@

setup(
name="tap-shopify",
version="1.7.3",
version="1.7.4",
description="Singer.io tap for extracting Shopify data",
author="Stitch",
url="http://github.com/singer-io/tap-shopify",
5 changes: 5 additions & 0 deletions tap_shopify/streams/base.py
Original file line number Diff line number Diff line change
@@ -4,6 +4,7 @@
import sys
import socket
from urllib.error import URLError
import http
import backoff
import pyactiveresource
import pyactiveresource.formats
@@ -120,6 +121,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

http.client.IncompleteRead,
max_tries=MAX_RETRIES,
factor=2)
@backoff.on_exception(backoff.expo, # timeout error raise by Shopify
(pyactiveresource.connection.Error, socket.timeout),
giveup=is_timeout_error,
24 changes: 24 additions & 0 deletions tests/unittests/test_backoff_on_incompleteread_error.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
from unittest import mock
from tap_shopify.streams.base import http
from tap_shopify.streams.transactions import Transactions
import unittest

class TestIncompleteReadBackoff(unittest.TestCase):
@mock.patch("time.sleep")
@mock.patch("pyactiveresource.activeresource.ActiveResource.find")
def test_Transactions_pyactiveresource_error_incomplete_read_error_backoff(self, mocked_find, mocked_sleep):
"""
Test case to verify that we backoff for 5 times when 'http.client.IncompleteRead' error occurs
"""
# mock 'find' and raise IncompleteRead error
mocked_find.side_effect = http.client.IncompleteRead(b'')
# initialize class
locations = Transactions()
try:
# function call
locations.replication_object.find()
except http.client.IncompleteRead:
pass

# verify we backoff 5 times
self.assertEquals(mocked_find.call_count, 5)