Skip to content
Merged
Show file tree
Hide file tree
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
61 changes: 36 additions & 25 deletions shotgun_api3/shotgun.py
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,8 @@ class Shotgun(object):
r"(\D?([01]\d|2[0-3])\D?([0-5]\d)\D?([0-5]\d)?\D?(\d{3})?)?$")

_MULTIPART_UPLOAD_CHUNK_SIZE = 20000000
MAX_ATTEMPTS = 3 # Retries on failure
BACKOFF = 0.75 # Seconds to wait before retry, times the attempt number

def __init__(self,
base_url,
Expand Down Expand Up @@ -3431,10 +3433,7 @@ def _call_rpc(self, method, params, include_auth_params=True, first=False):
req_headers["locale"] = "auto"

attempt = 1
max_attempts = 4 # Three retries on failure
backoff = 0.75 # Seconds to wait before retry, times the attempt number

while attempt <= max_attempts:
while attempt <= self.MAX_ATTEMPTS:
http_status, resp_headers, body = self._make_call(
"POST",
self.config.api_path,
Expand All @@ -3452,9 +3451,9 @@ def _call_rpc(self, method, params, include_auth_params=True, first=False):
# We've seen some rare instances of PTR returning 502 for issues that
# appear to be caused by something internal to PTR. We're going to
# allow for limited retries for those specifically.
if attempt != max_attempts and e.errcode in [502, 504]:
if attempt != self.MAX_ATTEMPTS and e.errcode in [502, 504]:
LOG.debug("Got a 502 or 504 response. Waiting and retrying...")
time.sleep(float(attempt) * backoff)
time.sleep(float(attempt) * self.BACKOFF)
attempt += 1
continue
elif e.errcode == 403:
Expand Down Expand Up @@ -4143,28 +4142,31 @@ def _upload_data_to_storage(self, data, content_type, size, storage_url):
request.get_method = lambda: "PUT"

attempt = 1
max_attempts = 4 # Three retries on failure
backoff = 0.75 # Seconds to wait before retry, times the attempt number

while attempt <= max_attempts:
while attempt <= self.MAX_ATTEMPTS:
try:
result = self._make_upload_request(request, opener)

LOG.debug("Completed request to %s" % request.get_method())

except urllib.error.HTTPError as e:
if attempt != max_attempts and e.code in [500, 503]:
if attempt != self.MAX_ATTEMPTS and e.code in [500, 503]:
LOG.debug("Got a %s response. Waiting and retrying..." % e.code)
time.sleep(float(attempt) * backoff)
time.sleep(float(attempt) * self.BACKOFF)
attempt += 1
continue
elif e.code in [500, 503]:
raise ShotgunError("Got a %s response when uploading to %s: %s" % (e.code, storage_url, e))
else:
raise ShotgunError("Unanticipated error occurred uploading to %s: %s" % (storage_url, e))

except urllib.error.URLError as e:
LOG.debug("Got a '%s' response. Waiting and retrying..." % e)
time.sleep(float(attempt) * self.BACKOFF)
attempt += 1
continue
else:
break
else:
raise ShotgunError("Max attemps limit reached.")

etag = result.info()["Etag"]
LOG.debug("Part upload completed successfully.")
Expand Down Expand Up @@ -4250,19 +4252,28 @@ def _send_form(self, url, params):

opener = self._build_opener(FormPostHandler)

# Perform the request
try:
resp = opener.open(url, params)
result = resp.read()
# response headers are in str(resp.info()).splitlines()
except urllib.error.HTTPError as e:
if e.code == 500:
raise ShotgunError("Server encountered an internal error. "
"\n%s\n(%s)\n%s\n\n" % (url, self._sanitize_auth_params(params), e))
else:
raise ShotgunError("Unanticipated error occurred %s" % (e))
attempt = 1
while attempt <= self.MAX_ATTEMPTS:
# Perform the request
try:
resp = opener.open(url, params)
result = resp.read()
# response headers are in str(resp.info()).splitlines()
except urllib.error.URLError as e:
LOG.debug("Got a %s response. Waiting and retrying..." % e)
time.sleep(float(attempt) * self.BACKOFF)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The problem with this loop is that, in the worst case scenario, where every attempt failed, you end up sleeping before leaving the loop.

attempt += 1
continue
except urllib.error.HTTPError as e:
if e.code == 500:
raise ShotgunError("Server encountered an internal error. "
"\n%s\n(%s)\n%s\n\n" % (url, self._sanitize_auth_params(params), e))
else:
raise ShotgunError("Unanticipated error occurred %s" % (e))

return six.ensure_text(result)
return six.ensure_text(result)
else:
raise ShotgunError("Max attemps limit reached.")


class CACertsHTTPSConnection(http_client.HTTPConnection):
Expand Down
2 changes: 2 additions & 0 deletions tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,8 @@ def _setup_mock(self, s3_status_code_error=503):
# create the server caps directly to say we have the correct version
self.sg._server_caps = ServerCapabilities(self.sg.config.server,
{"version": [2, 4, 0]})
# prevent waiting for backoff
self.sg.BACKOFF = 0

def _mock_http(self, data, headers=None, status=None):
"""Setup a mock response from the PTR server.
Expand Down
80 changes: 70 additions & 10 deletions tests/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -438,9 +438,9 @@ def test_call_rpc(self):
self._mock_http(d, status=(502, "bad gateway"))
self.assertRaises(api.ProtocolError, self.sg._call_rpc, "list", a)
self.assertEqual(
4,
self.sg.MAX_ATTEMPTS,
self.sg._http_request.call_count,
"Call is repeated up to 3 times",
f"Call is repeated up to {self.sg.MAX_ATTEMPTS} times",
)

# 504
Expand All @@ -449,9 +449,9 @@ def test_call_rpc(self):
self._mock_http(d, status=(504, "gateway timeout"))
self.assertRaises(api.ProtocolError, self.sg._call_rpc, "list", a)
self.assertEqual(
4,
self.sg.MAX_ATTEMPTS,
self.sg._http_request.call_count,
"Call is repeated up to 3 times",
f"Call is repeated up to {self.sg.MAX_ATTEMPTS} times",
)

def test_upload_s3_503(self):
Expand All @@ -462,7 +462,6 @@ def test_upload_s3_503(self):
storage_url = "http://foo.com/"
path = os.path.abspath(os.path.expanduser(
os.path.join(this_dir, "sg_logo.jpg")))
max_attempts = 4 # Max retries to S3 server attempts
# Expected HTTPError exception error message
expected = "The server is currently down or to busy to reply." \
"Please try again later."
Expand All @@ -474,8 +473,8 @@ def test_upload_s3_503(self):
self.sg._upload_file_to_storage(path, storage_url)
# Test the max retries attempt
self.assertTrue(
max_attempts == self.sg._make_upload_request.call_count,
"Call is repeated up to 3 times")
self.sg.MAX_ATTEMPTS == self.sg._make_upload_request.call_count,
f"Call is repeated up to {self.sg.MAX_ATTEMPTS} times")

def test_upload_s3_500(self):
"""
Expand All @@ -486,7 +485,6 @@ def test_upload_s3_500(self):
storage_url = "http://foo.com/"
path = os.path.abspath(os.path.expanduser(
os.path.join(this_dir, "sg_logo.jpg")))
max_attempts = 4 # Max retries to S3 server attempts
# Expected HTTPError exception error message
expected = "The server is currently down or to busy to reply." \
"Please try again later."
Expand All @@ -498,8 +496,70 @@ def test_upload_s3_500(self):
self.sg._upload_file_to_storage(path, storage_url)
# Test the max retries attempt
self.assertTrue(
max_attempts == self.sg._make_upload_request.call_count,
"Call is repeated up to 3 times")
self.sg.MAX_ATTEMPTS == self.sg._make_upload_request.call_count,
f"Call is repeated up to {self.sg.MAX_ATTEMPTS} times")

def test_upload_s3_urlerror__get_attachment_upload_info(self):
"""
Test URLError response is retried when invoking _send_form
"""
mock_opener = mock.Mock()
mock_opener.return_value.open.side_effect = urllib.error.URLError(
"[WinError 10054] An existing connection was forcibly closed by the remote host"
)
self.sg._build_opener = mock_opener
this_dir, _ = os.path.split(__file__)
path = os.path.abspath(
os.path.expanduser(os.path.join(this_dir, "sg_logo.jpg"))
)

with self.assertRaises(api.ShotgunError) as cm:
self.sg._get_attachment_upload_info(False, path, False)

# Test the max retries attempt
self.assertEqual(
self.sg.MAX_ATTEMPTS,
mock_opener.return_value.open.call_count,
f"Call is repeated up to {self.sg.MAX_ATTEMPTS} times"
)

# Test the exception message
the_exception = cm.exception
self.assertEqual(str(the_exception), "Max attemps limit reached.")

def test_upload_s3_urlerror__upload_to_storage(self):
"""
Test URLError response is retried when uploading to S3.
"""
self.sg._make_upload_request = mock.Mock(
spec=api.Shotgun._make_upload_request,
side_effect=urllib.error.URLError(
"[Errno 104] Connection reset by peer",
),
)

this_dir, _ = os.path.split(__file__)
storage_url = "http://foo.com/"
path = os.path.abspath(
os.path.expanduser(os.path.join(this_dir, "sg_logo.jpg"))
)

# Test the Internal function that is used to upload each
# data part in the context of multi-part uploads to S3, we
# simulate the HTTPError exception raised with 503 status errors
with self.assertRaises(api.ShotgunError) as cm:
self.sg._upload_file_to_storage(path, storage_url)

# Test the max retries attempt
self.assertEqual(
self.sg.MAX_ATTEMPTS,
self.sg._make_upload_request.call_count,
f"Call is repeated up to {self.sg.MAX_ATTEMPTS} times"
)

# Test the exception message
the_exception = cm.exception
self.assertEqual(str(the_exception), "Max attemps limit reached.")

def test_transform_data(self):
"""Outbound data is transformed"""
Expand Down