Skip to content

Commit a1e3d16

Browse files
authored
SG-11528 Add optional sleep between retries (shotgunsoftware#196)
Add an optional sleep between retries on failed requests defaulting to 3 seconds. Allow the interval to be set by an environment variable or a property on sg.config.
1 parent 6094e33 commit a1e3d16

File tree

7 files changed

+115
-10
lines changed

7 files changed

+115
-10
lines changed

.travis.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@ language: python
22
python:
33
- "2.6"
44
- "2.7"
5+
6+
# use trusty dist, since xenial (default) does not support python 2.6
7+
dist: trusty
8+
59
# command to install dependencies
610
install:
711
- pip install -r tests/ci_requirements.txt

HISTORY.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ Shotgun Python API Changelog
44

55
Here you can see the full list of changes between each Python API release.
66

7+
v3.0.41 (2019 June 28)
8+
=====================
9+
- Adds an optional sleep between retries specified via the `SHOTGUN_API_RETRY_INTERVAL` environment variable, or by setting `sg.config.rpc_attempt_interval`.
10+
711
v3.0.40 (2019 March 13)
812
=====================
913
- Updates encoding method to use shutil when uploading, to avoid memory and overflow errors when reading large files. (contributed by @eestrada)

docs/reference.rst

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -869,4 +869,23 @@ are specifically displaying EventLogEntries in the web application, or API queri
869869
log that are run. We are always looking for ways to improve this in the future. If you have any
870870
immediate concerns, please `reach out to our support team <https://support.shotgunsoftware.com>`_
871871

872+
*********************
873+
Environment Variables
874+
*********************
875+
876+
SHOTGUN_API_CACERTS
877+
===================
878+
879+
Used to specify a path to an external SSL certificates file. This environment variable can be used in place of the ``ca_certs`` keyword argument to the :class:`~shotgun.Shotgun` constructor. In the case that both this environment variable is set and the keyword argument is provided, the value from the keyword argument will be used.
880+
881+
882+
SHOTGUN_API_RETRY_INTERVAL
883+
==========================
884+
885+
Stores the number of milliseconds to wait between request retries. By default, a value of 3000 milliseconds is used. You can override the default either by setting this environment variable, or by setting the ``rpc_attempt_interval`` property on the config like so: ::
886+
887+
sg = Shotgun(site_name, script_name, script_key)
888+
sg.config.rpc_attempt_interval = 1000 # adjusting default interval
889+
890+
In the case that both this environment variable and the config's ``rpc_attempt_interval`` property are set, the value in ``rpc_attempt_interal`` will be used.
872891

setup.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717

1818
setup(
1919
name='shotgun_api3',
20-
version='3.0.40',
20+
version='3.0.41',
2121
description='Shotgun Python API ',
2222
long_description=readme,
2323
author='Shotgun Software',

shotgun_api3/shotgun.py

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@
9292

9393
# ----------------------------------------------------------------------------
9494
# Version
95-
__version__ = "3.0.40"
95+
__version__ = "3.0.41"
9696

9797
# ----------------------------------------------------------------------------
9898
# Errors
@@ -345,6 +345,17 @@ def __init__(self, sg):
345345
"""
346346
self._sg = sg
347347
self.max_rpc_attempts = 3
348+
# rpc_attempt_interval stores the number of milliseconds to wait between
349+
# request retries. By default, this will be 3000 milliseconds. You can
350+
# override this by setting this property on the config like so:
351+
#
352+
# sg = Shotgun(site_name, script_name, script_key)
353+
# sg.config.rpc_attempt_interval = 1000 # adjusting default interval
354+
#
355+
# Or by setting the ``SHOTGUN_API_RETRY_INTERVAL`` environment variable.
356+
# In the case that the environment variable is already set, setting the
357+
# property on the config will override it.
358+
self.rpc_attempt_interval = 3000
348359
# From http://docs.python.org/2.6/library/httplib.html:
349360
# If the optional timeout parameter is given, blocking operations
350361
# (like connection attempts) will timeout after that many seconds
@@ -553,6 +564,17 @@ def __init__(self,
553564
self.config.convert_datetimes_to_utc = convert_datetimes_to_utc
554565
self.config.no_ssl_validation = NO_SSL_VALIDATION
555566
self.config.raw_http_proxy = http_proxy
567+
568+
try:
569+
self.config.rpc_attempt_interval = int(os.environ.get("SHOTGUN_API_RETRY_INTERVAL", 3000))
570+
except ValueError:
571+
retry_interval = os.environ.get("SHOTGUN_API_RETRY_INTERVAL", 3000)
572+
raise ValueError("Invalid value '%s' found in environment variable "
573+
"SHOTGUN_API_RETRY_INTERVAL, must be int." % retry_interval)
574+
if self.config.rpc_attempt_interval < 0:
575+
raise ValueError("Value of SHOTGUN_API_RETRY_INTERVAL must be positive, "
576+
"got '%s'." % self.config.rpc_attempt_interval)
577+
556578
self._connection = None
557579
if ca_certs is not None:
558580
self.__ca_certs = ca_certs
@@ -3308,6 +3330,7 @@ def _make_call(self, verb, path, body, headers):
33083330
body = body or None
33093331

33103332
max_rpc_attempts = self.config.max_rpc_attempts
3333+
rpc_attempt_interval = self.config.rpc_attempt_interval / 1000.0
33113334

33123335
while (attempt < max_rpc_attempts):
33133336
attempt += 1
@@ -3346,10 +3369,15 @@ def _make_call(self, verb, path, body, headers):
33463369
if attempt == max_rpc_attempts:
33473370
raise
33483371
except Exception:
3349-
#TODO: LOG ?
33503372
self._close_connection()
33513373
if attempt == max_rpc_attempts:
3374+
LOG.debug("Request failed. Giving up after %d attempts." % attempt)
33523375
raise
3376+
LOG.debug(
3377+
"Request failed, attempt %d of %d. Retrying in %.2f seconds..." %
3378+
(attempt, max_rpc_attempts, rpc_attempt_interval)
3379+
)
3380+
time.sleep(rpc_attempt_interval)
33533381

33543382
def _http_request(self, verb, path, body, headers):
33553383
"""

tests/test_api.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -801,6 +801,7 @@ def test_preferences_read(self):
801801

802802
expected = {
803803
'date_component_order': 'month_day',
804+
'duration_units': 'days',
804805
'format_currency_fields_decimal_options': '$1,000.99',
805806
'format_currency_fields_display_dollar_sign': False,
806807
'format_currency_fields_negative_options': '- $1,000',
@@ -810,8 +811,10 @@ def test_preferences_read(self):
810811
'format_footage_fields': '10-05',
811812
'format_number_fields': '1,000',
812813
'format_time_hour_fields': '12 hour',
814+
'hours_per_day': 8.0,
815+
'last_day_work_week': None,
813816
'support_local_storage': False,
814-
'view_master_settings': '{"status_groups":[{"name":"Upcoming","code":"upc_stgr","status_list":["wtg","rdy"]},{"name":"Active","code":"act_stgr","status_list":["ip","kickbk","rev","act","rsk","blk","late","opn","pndng","tkt","push","rrq","vwd","out"]},{"name":"Done","code":"done_stgr","status_list":["fin","cmpt","apr","cbb","clsd","cfrm","dlvr","recd","res"]}],"entity_fields":{"Task":["content","sg_description","sg_status_list","due_date","task_assignees","task_reviewers"],"Shot":["code","description","sg_status_list","created_at","sg_cut_in","sg_cut_out","sg_cut_duration","sg_cut_order"],"Asset":["code","description","sg_status_list","created_at"],"Scene":["code","sg_status_list","created_at"],"Element":["code","sg_status_list","created_at"],"Release":["code","sg_status_list","created_at"],"ShootDay":["code","sg_status_list","created_at"],"MocapTake":["code","sg_status_list","created_at"],"MocapSetup":["code","sg_status_list","created_at"],"Camera":["code","sg_status_list","created_at"],"MocapTakeRange":["code","sg_status_list","created_at"],"Sequence":["code","sg_status_list","created_at"],"Level":["code","sg_status_list","created_at"],"Episode":["code","sg_status_list","created_at"]},"entity_fields_fixed":{"Asset":["code","description","sg_status_list"],"Shot":["code","description","sg_status_list"],"Task":["content","sg_status_list","due_date","task_assignees","task_reviewers"],"Scene":["code","description","sg_status_list"],"Element":["code","description","sg_status_list"],"Release":["code","description","sg_status_list"],"ShootDay":["code","description","sg_status_list"],"MocapTake":["code","description","sg_status_list"],"MocapSetup":["code","description","sg_status_list"],"Camera":["code","description","sg_status_list"],"MocapTakeRange":["code","description","sg_status_list"],"Sequence":["code","description","sg_status_list"],"Level":["code","description","sg_status_list"],"Episode":["code","description","sg_status_list"]}}' # noqa
817+
'view_master_settings': '{"status_groups":[{"name":"Upcoming","code":"upc_stgr","status_list":["wtg","rdy"]},{"name":"Active","code":"act_stgr","status_list":["ip","kickbk","rev","act","rsk","blk","late","opn","pndng","tkt","push","rrq","vwd","out"]},{"name":"Done","code":"done_stgr","status_list":["fin","cmpt","apr","cbb","clsd","cfrm","dlvr","recd","res"]}],"entity_fields":{"Task":["content","sg_description","sg_status_list","due_date","task_assignees","task_reviewers","time_logs_sum"],"Shot":["code","description","sg_status_list","created_at","sg_cut_in","sg_cut_out","sg_cut_duration","sg_cut_order"],"Asset":["code","description","sg_status_list","created_at"],"Scene":["code","sg_status_list","created_at"],"Element":["code","sg_status_list","created_at"],"Release":["code","sg_status_list","created_at"],"ShootDay":["code","sg_status_list","created_at"],"MocapTake":["code","sg_status_list","created_at"],"MocapSetup":["code","sg_status_list","created_at"],"Camera":["code","sg_status_list","created_at"],"MocapTakeRange":["code","sg_status_list","created_at"],"Sequence":["code","sg_status_list","created_at"],"Level":["code","sg_status_list","created_at"],"Episode":["code","sg_status_list","created_at"],"Version":["code","description","sg_status_list"]},"entity_fields_fixed":{"Asset":["code","description","sg_status_list"],"Shot":["code","description","sg_status_list"],"Task":["content","sg_status_list","due_date","task_assignees","task_reviewers","time_logs_sum"],"Scene":["code","description","sg_status_list"],"Element":["code","description","sg_status_list"],"Release":["code","description","sg_status_list"],"ShootDay":["code","description","sg_status_list"],"MocapTake":["code","description","sg_status_list"],"MocapSetup":["code","description","sg_status_list"],"Camera":["code","description","sg_status_list"],"MocapTakeRange":["code","description","sg_status_list"],"Sequence":["code","description","sg_status_list"],"Level":["code","description","sg_status_list"],"Episode":["code","description","sg_status_list"],"Version":["code","description","sg_status_list"]},"board_sorting":{"Upcoming":{"Task":[{"direction":"desc","field_name":"due_date"},{"direction":"asc","field_name":"content"}]},"Done":{"Task":[{"direction":"desc","field_name":"due_date"},{"direction":"asc","field_name":"content"}]},"Active":{"Task":[{"direction":"desc","field_name":"due_date"},{"direction":"asc","field_name":"content"}]}},"status_default":{"Version":{"pending_review_status":["rev"],"viewed_review_status":["vwd"]},"Task":{"final_review_status":["fin"]}},"entity_forms":{"TimeLog":["date","description","duration"]},"entity_forms_fixed":{"TimeLog":["date","description","duration"]},"enable_timelog_at_version_creation":false}' # noqa
815818
}
816819
self.assertEqual(expected, resp)
817820

tests/test_client.py

Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import base64
66
import datetime
77
import urllib
8+
import os
89
import re
910
try:
1011
import simplejson as json
@@ -225,15 +226,61 @@ def test_connect_close(self):
225226
self.sg.close()
226227
self.assertEqual(None, self.sg._connection)
227228

228-
229229
def test_network_retry(self):
230-
"""Network failure is retried"""
230+
"""Network failure is retried, with a sleep call between retries."""
231231
self.sg._http_request.side_effect = httplib2.HttpLib2Error
232232

233-
self.assertRaises(httplib2.HttpLib2Error, self.sg.info)
234-
self.assertTrue(
235-
self.sg.config.max_rpc_attempts ==self.sg._http_request.call_count,
236-
"Call is repeated")
233+
with mock.patch("time.sleep") as mock_sleep:
234+
self.assertRaises(httplib2.HttpLib2Error, self.sg.info)
235+
self.assertTrue(
236+
self.sg.config.max_rpc_attempts == self.sg._http_request.call_count,
237+
"Call is repeated")
238+
# Ensure that sleep was called with the retry interval between each attempt
239+
attempt_interval = self.sg.config.rpc_attempt_interval / 1000.0
240+
calls = [mock.callargs(((attempt_interval,), {}))]
241+
calls *= (self.sg.config.max_rpc_attempts - 1)
242+
self.assertTrue(
243+
mock_sleep.call_args_list == calls,
244+
"Call is repeated at correct interval."
245+
)
246+
247+
def test_set_retry_interval(self):
248+
"""Setting the retry interval through parameter and environment variable works."""
249+
original_env_val = os.environ.pop("SHOTGUN_API_RETRY_INTERVAL", None)
250+
251+
try:
252+
def run_interval_test(expected_interval, interval_property=None):
253+
self.sg = api.Shotgun(self.config.server_url,
254+
self.config.script_name,
255+
self.config.api_key,
256+
http_proxy=self.config.http_proxy,
257+
connect=self.connect)
258+
self._setup_mock()
259+
if interval_property:
260+
# if a value was provided for interval_property, set the
261+
# config's property to that value.
262+
self.sg.config.rpc_attempt_interval = interval_property
263+
self.sg._http_request.side_effect = httplib2.HttpLib2Error
264+
self.assertEqual(self.sg.config.rpc_attempt_interval, expected_interval)
265+
self.test_network_retry()
266+
267+
# Try passing parameter and ensure the correct interval is used.
268+
run_interval_test(expected_interval=2500, interval_property=2500)
269+
270+
# Try setting ENV VAR and ensure the correct interval is used.
271+
os.environ["SHOTGUN_API_RETRY_INTERVAL"] = "2000"
272+
run_interval_test(expected_interval=2000)
273+
274+
# Try both parameter and environment variable, to ensure parameter wins.
275+
run_interval_test(expected_interval=4000, interval_property=4000)
276+
277+
finally:
278+
# Restore environment variable.
279+
if original_env_val is not None:
280+
os.environ["SHOTGUN_API_RETRY_INTERVAL"] = original_env_val
281+
elif "SHOTGUN_API_RETRY_INTERVAL" in os.environ:
282+
os.environ.pop("SHOTGUN_API_RETRY_INTERVAL")
283+
237284

238285
def test_http_error(self):
239286
"""HTTP error raised and not retried."""

0 commit comments

Comments
 (0)