From 9de6c554315e33e768b1ea959016808e0275a127 Mon Sep 17 00:00:00 2001 From: Will Cavanagh Date: Wed, 1 May 2019 16:00:46 -0700 Subject: [PATCH 01/15] Add rpc_attempt_interval to config sleep between request attempts for the configured time, default to 3 seconds, and allow override through optional environment variable. --- shotgun_api3/shotgun.py | 12 +++++++++++- tests/test_client.py | 19 +++++++++++++------ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 81627ca19..406fe80eb 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -345,6 +345,10 @@ def __init__(self, sg): """ self._sg = sg self.max_rpc_attempts = 3 + # The interval between _call_rpc retries. Since the Config can't be + # modified until the Shotgun instance is created, use an environment + # variable to override the default value. + self.rpc_attempt_interval = os.environ.get("SHOTGUN_API_RETRY_INTERVAL", 3) # From http://docs.python.org/2.6/library/httplib.html: # If the optional timeout parameter is given, blocking operations # (like connection attempts) will timeout after that many seconds @@ -3308,6 +3312,7 @@ def _make_call(self, verb, path, body, headers): body = body or None max_rpc_attempts = self.config.max_rpc_attempts + rpc_attempt_interval = self.config.rpc_attempt_interval while (attempt < max_rpc_attempts): attempt += 1 @@ -3346,10 +3351,15 @@ def _make_call(self, verb, path, body, headers): if attempt == max_rpc_attempts: raise except Exception: - #TODO: LOG ? self._close_connection() if attempt == max_rpc_attempts: + LOG.debug("Request failed. Giving up after %d attempts." % attempt) raise + LOG.debug( + "Request failed, attempt %d of %d. Retrying in %.2f seconds..." % + (attempt, max_rpc_attempts, rpc_attempt_interval) + ) + time.sleep(rpc_attempt_interval) def _http_request(self, verb, path, body, headers): """ diff --git a/tests/test_client.py b/tests/test_client.py index 72d59a445..5d679d24f 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -225,15 +225,22 @@ def test_connect_close(self): self.sg.close() self.assertEqual(None, self.sg._connection) - def test_network_retry(self): - """Network failure is retried""" + """Network failure is retried, with a sleep call between retries.""" self.sg._http_request.side_effect = httplib2.HttpLib2Error - self.assertRaises(httplib2.HttpLib2Error, self.sg.info) - self.assertTrue( - self.sg.config.max_rpc_attempts ==self.sg._http_request.call_count, - "Call is repeated") + with mock.patch("time.sleep") as mock_sleep: + self.assertRaises(httplib2.HttpLib2Error, self.sg.info) + self.assertTrue( + self.sg.config.max_rpc_attempts == self.sg._http_request.call_count, + "Call is repeated") + # Ensure that sleep was called with the retry interval between each attempt + calls = [mock.callargs(((self.sg.config.rpc_attempt_interval,), {}))] + calls *= (self.sg.config.max_rpc_attempts - 1) + self.assertTrue( + mock_sleep.call_args_list == calls, + "Call is repeated at correct interval." + ) def test_http_error(self): """HTTP error raised and not retried.""" From 9d217a0caca3c9c9540ad69f2e2e58550ee673ca Mon Sep 17 00:00:00 2001 From: Will Cavanagh Date: Wed, 1 May 2019 16:37:41 -0700 Subject: [PATCH 02/15] Update expected preferences to include duration/day and view_master_settings changes. --- tests/test_api.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_api.py b/tests/test_api.py index 453f6c4e3..af87c449b 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -801,6 +801,7 @@ def test_preferences_read(self): expected = { 'date_component_order': 'month_day', + 'duration_units' : 'days', 'format_currency_fields_decimal_options': '$1,000.99', 'format_currency_fields_display_dollar_sign': False, 'format_currency_fields_negative_options': '- $1,000', @@ -810,8 +811,10 @@ def test_preferences_read(self): 'format_footage_fields': '10-05', 'format_number_fields': '1,000', 'format_time_hour_fields': '12 hour', + 'hours_per_day' : 8.0, + 'last_day_work_week' : None, 'support_local_storage': False, - '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 + '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"}]}}}' # noqa } self.assertEqual(expected, resp) From d8d790a82a605344fc4f5ba2815d08597578b4a4 Mon Sep 17 00:00:00 2001 From: Will Cavanagh Date: Wed, 1 May 2019 16:39:45 -0700 Subject: [PATCH 03/15] Fix whitespace. --- tests/test_api.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index af87c449b..3431c983f 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -801,7 +801,7 @@ def test_preferences_read(self): expected = { 'date_component_order': 'month_day', - 'duration_units' : 'days', + 'duration_units': 'days', 'format_currency_fields_decimal_options': '$1,000.99', 'format_currency_fields_display_dollar_sign': False, 'format_currency_fields_negative_options': '- $1,000', @@ -811,8 +811,8 @@ def test_preferences_read(self): 'format_footage_fields': '10-05', 'format_number_fields': '1,000', 'format_time_hour_fields': '12 hour', - 'hours_per_day' : 8.0, - 'last_day_work_week' : None, + 'hours_per_day': 8.0, + 'last_day_work_week': None, 'support_local_storage': False, '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"}]}}}' # noqa } From 6d8a3c3b0d8a908646916337246d50554e7abaae Mon Sep 17 00:00:00 2001 From: Will Cavanagh Date: Fri, 17 May 2019 13:24:47 -0700 Subject: [PATCH 04/15] Allow setting retry interval by passing parameter store interval in milliseconds, add tests for both environment variable and parameter. --- shotgun_api3/shotgun.py | 19 +++++++++++++------ tests/test_client.py | 37 ++++++++++++++++++++++++++++++++++++- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 406fe80eb..43aa947c9 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -345,10 +345,8 @@ def __init__(self, sg): """ self._sg = sg self.max_rpc_attempts = 3 - # The interval between _call_rpc retries. Since the Config can't be - # modified until the Shotgun instance is created, use an environment - # variable to override the default value. - self.rpc_attempt_interval = os.environ.get("SHOTGUN_API_RETRY_INTERVAL", 3) + # The interval between _call_rpc retries in ms. Default to 3 seconds. + self.rpc_attempt_interval = 3000 # From http://docs.python.org/2.6/library/httplib.html: # If the optional timeout parameter is given, blocking operations # (like connection attempts) will timeout after that many seconds @@ -428,7 +426,8 @@ def __init__(self, password=None, sudo_as_login=None, session_token=None, - auth_token=None): + auth_token=None, + rpc_attempt_interval=None): """ Initializes a new instance of the Shotgun client. @@ -496,6 +495,10 @@ def __init__(self, :class:`~shotgun_api3.MissingTwoFactorAuthenticationFault` will be raised if the ``auth_token`` is invalid. .. todo: Add this info to the Authentication section of the docs + :param int rpc_attempt_interval: (optional) milliseconds to wait between request retries. + By default, this will be 3000 milliseconds. You can override this by passing a value + to this parameter, or by setting the ``SHOTGUN_API_RETRY_INTERVAL`` environment variable. + In the case both are set, this parameter will take precedence. .. note:: A note about proxy connections: If you are using Python <= v2.6.2, HTTPS connections through a proxy server will not work due to a bug in the :mod:`urllib2` @@ -562,6 +565,10 @@ def __init__(self, self.__ca_certs = ca_certs else: self.__ca_certs = os.environ.get('SHOTGUN_API_CACERTS') + if rpc_attempt_interval is not None: + self.config.rpc_attempt_interval = rpc_attempt_interval + else: + self.config.rpc_attempt_interval = int(os.environ.get("SHOTGUN_API_RETRY_INTERVAL", 3000)) self.base_url = (base_url or "").lower() self.config.scheme, self.config.server, api_base, _, _ = \ @@ -3312,7 +3319,7 @@ def _make_call(self, verb, path, body, headers): body = body or None max_rpc_attempts = self.config.max_rpc_attempts - rpc_attempt_interval = self.config.rpc_attempt_interval + rpc_attempt_interval = float(self.config.rpc_attempt_interval) / 1000 while (attempt < max_rpc_attempts): attempt += 1 diff --git a/tests/test_client.py b/tests/test_client.py index 5d679d24f..61ee93a62 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -5,6 +5,7 @@ import base64 import datetime import urllib +import os import re try: import simplejson as json @@ -235,13 +236,47 @@ def test_network_retry(self): self.sg.config.max_rpc_attempts == self.sg._http_request.call_count, "Call is repeated") # Ensure that sleep was called with the retry interval between each attempt - calls = [mock.callargs(((self.sg.config.rpc_attempt_interval,), {}))] + attempt_interval = float(self.sg.config.rpc_attempt_interval) / 1000 + calls = [mock.callargs(((attempt_interval,), {}))] calls *= (self.sg.config.max_rpc_attempts - 1) self.assertTrue( mock_sleep.call_args_list == calls, "Call is repeated at correct interval." ) + def test_set_retry_interval(self): + """Setting the retry interval through parameter and environment variable works.""" + original_env_val = os.environ.pop("SHOTGUN_API_RETRY_INTERVAL", None) + + def do_test(expected_interval, interval_parameter=None): + self.sg = api.Shotgun(self.config.server_url, + self.config.script_name, + self.config.api_key, + http_proxy=self.config.http_proxy, + connect=self.connect, + rpc_attempt_interval=interval_parameter) + self._setup_mock() + self.sg._http_request.side_effect = httplib2.HttpLib2Error + self.assertEqual(self.sg.config.rpc_attempt_interval, expected_interval) + self.test_network_retry() + + # Try passing parameter and ensure the correct interval is used. + do_test(2500, interval_parameter=2500) + + # Try setting ENV VAR and ensure the correct interval is used. + os.environ["SHOTGUN_API_RETRY_INTERVAL"] = "2000" + do_test(2000) + + # Try both parameter and Environment variable, to ensure parameter wins. + do_test(4000, interval_parameter=4000) + + # restore environment variable + if original_env_val is not None: + os.environ["SHOTGUN_API_RETRY_INTERVAL"] = original_env_val + else: + os.environ.pop("SHOTGUN_API_RETRY_INTERVAL") + + def test_http_error(self): """HTTP error raised and not retried.""" From 2f2192aeae304d842a3231b7c2292a35c009121d Mon Sep 17 00:00:00 2001 From: Will Cavanagh Date: Fri, 17 May 2019 13:42:28 -0700 Subject: [PATCH 05/15] Minor cleanup for clarity. --- shotgun_api3/shotgun.py | 4 ++-- tests/test_client.py | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 43aa947c9..349780bf4 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -345,7 +345,7 @@ def __init__(self, sg): """ self._sg = sg self.max_rpc_attempts = 3 - # The interval between _call_rpc retries in ms. Default to 3 seconds. + # The interval between _call_rpc retries in ms. Default to 3 seconds. self.rpc_attempt_interval = 3000 # From http://docs.python.org/2.6/library/httplib.html: # If the optional timeout parameter is given, blocking operations @@ -496,7 +496,7 @@ def __init__(self, ``auth_token`` is invalid. .. todo: Add this info to the Authentication section of the docs :param int rpc_attempt_interval: (optional) milliseconds to wait between request retries. - By default, this will be 3000 milliseconds. You can override this by passing a value + By default, this will be 3000 milliseconds. You can override this by passing a value to this parameter, or by setting the ``SHOTGUN_API_RETRY_INTERVAL`` environment variable. In the case both are set, this parameter will take precedence. diff --git a/tests/test_client.py b/tests/test_client.py index 61ee93a62..e5bcafd5c 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -248,7 +248,7 @@ def test_set_retry_interval(self): """Setting the retry interval through parameter and environment variable works.""" original_env_val = os.environ.pop("SHOTGUN_API_RETRY_INTERVAL", None) - def do_test(expected_interval, interval_parameter=None): + def run_interval_test(expected_interval, interval_parameter=None): self.sg = api.Shotgun(self.config.server_url, self.config.script_name, self.config.api_key, @@ -261,14 +261,14 @@ def do_test(expected_interval, interval_parameter=None): self.test_network_retry() # Try passing parameter and ensure the correct interval is used. - do_test(2500, interval_parameter=2500) + run_interval_test(expected_interval=2500, interval_parameter=2500) # Try setting ENV VAR and ensure the correct interval is used. os.environ["SHOTGUN_API_RETRY_INTERVAL"] = "2000" - do_test(2000) + run_interval_test(expected_interval=2000) - # Try both parameter and Environment variable, to ensure parameter wins. - do_test(4000, interval_parameter=4000) + # Try both parameter and environment variable, to ensure parameter wins. + run_interval_test(expected_interval=4000, interval_parameter=4000) # restore environment variable if original_env_val is not None: From 1103ba08c77e2b9312f5068a076740bff10eda2a Mon Sep 17 00:00:00 2001 From: Will Cavanagh Date: Fri, 17 May 2019 13:51:42 -0700 Subject: [PATCH 06/15] Make float division easier to read. --- shotgun_api3/shotgun.py | 2 +- tests/test_client.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 349780bf4..1d570e28a 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -3319,7 +3319,7 @@ def _make_call(self, verb, path, body, headers): body = body or None max_rpc_attempts = self.config.max_rpc_attempts - rpc_attempt_interval = float(self.config.rpc_attempt_interval) / 1000 + rpc_attempt_interval = self.config.rpc_attempt_interval / 1000.0 while (attempt < max_rpc_attempts): attempt += 1 diff --git a/tests/test_client.py b/tests/test_client.py index e5bcafd5c..131f691bc 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -236,7 +236,7 @@ def test_network_retry(self): self.sg.config.max_rpc_attempts == self.sg._http_request.call_count, "Call is repeated") # Ensure that sleep was called with the retry interval between each attempt - attempt_interval = float(self.sg.config.rpc_attempt_interval) / 1000 + attempt_interval = self.sg.config.rpc_attempt_interval / 1000.0 calls = [mock.callargs(((attempt_interval,), {}))] calls *= (self.sg.config.max_rpc_attempts - 1) self.assertTrue( From c6106558442e1d6021c14e19686f77923dc762ae Mon Sep 17 00:00:00 2001 From: Will Cavanagh Date: Thu, 6 Jun 2019 13:52:32 -0700 Subject: [PATCH 07/15] Wrap retry_interval tests in try-finally to prevent leaking SHOTGUN_API_RETRY_INTERVAL env var in the event that tests fail. --- tests/test_client.py | 56 +++++++++++++++++++++++--------------------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/tests/test_client.py b/tests/test_client.py index 131f691bc..c863b9610 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -248,33 +248,35 @@ def test_set_retry_interval(self): """Setting the retry interval through parameter and environment variable works.""" original_env_val = os.environ.pop("SHOTGUN_API_RETRY_INTERVAL", None) - def run_interval_test(expected_interval, interval_parameter=None): - self.sg = api.Shotgun(self.config.server_url, - self.config.script_name, - self.config.api_key, - http_proxy=self.config.http_proxy, - connect=self.connect, - rpc_attempt_interval=interval_parameter) - self._setup_mock() - self.sg._http_request.side_effect = httplib2.HttpLib2Error - self.assertEqual(self.sg.config.rpc_attempt_interval, expected_interval) - self.test_network_retry() - - # Try passing parameter and ensure the correct interval is used. - run_interval_test(expected_interval=2500, interval_parameter=2500) - - # Try setting ENV VAR and ensure the correct interval is used. - os.environ["SHOTGUN_API_RETRY_INTERVAL"] = "2000" - run_interval_test(expected_interval=2000) - - # Try both parameter and environment variable, to ensure parameter wins. - run_interval_test(expected_interval=4000, interval_parameter=4000) - - # restore environment variable - if original_env_val is not None: - os.environ["SHOTGUN_API_RETRY_INTERVAL"] = original_env_val - else: - os.environ.pop("SHOTGUN_API_RETRY_INTERVAL") + try: + def run_interval_test(expected_interval, interval_parameter=None): + self.sg = api.Shotgun(self.config.server_url, + self.config.script_name, + self.config.api_key, + http_proxy=self.config.http_proxy, + connect=self.connect, + rpc_attempt_interval=interval_parameter) + self._setup_mock() + self.sg._http_request.side_effect = httplib2.HttpLib2Error + self.assertEqual(self.sg.config.rpc_attempt_interval, expected_interval) + self.test_network_retry() + + # Try passing parameter and ensure the correct interval is used. + run_interval_test(expected_interval=2500, interval_parameter=2500) + + # Try setting ENV VAR and ensure the correct interval is used. + os.environ["SHOTGUN_API_RETRY_INTERVAL"] = "2000" + run_interval_test(expected_interval=2000) + + # Try both parameter and environment variable, to ensure parameter wins. + run_interval_test(expected_interval=4000, interval_parameter=4000) + + finally: + # Restore environment variable. + if original_env_val is not None: + os.environ["SHOTGUN_API_RETRY_INTERVAL"] = original_env_val + elif "SHOTGUN_API_RETRY_INTERVAL" in os.environ: + os.environ.pop("SHOTGUN_API_RETRY_INTERVAL") def test_http_error(self): From f2140b87c05f4488981555aee91baa88e767e8fb Mon Sep 17 00:00:00 2001 From: Will Cavanagh Date: Thu, 6 Jun 2019 14:31:30 -0700 Subject: [PATCH 08/15] Remove parameter, and add better comment explaining config.rpc_attempt_interval property. --- shotgun_api3/shotgun.py | 23 ++++++++++++----------- tests/test_client.py | 13 ++++++++----- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 1d570e28a..96d39aef9 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -345,7 +345,16 @@ def __init__(self, sg): """ self._sg = sg self.max_rpc_attempts = 3 - # The interval between _call_rpc retries in ms. Default to 3 seconds. + # rpc_attempt_interval stores the number of milliseconds to wait between + # request retries. By default, this will be 3000 milliseconds. You can + # override this by setting this property on the config like so: + # + # sg = Shotgun(site_name, script_name, script_key) + # sg.config.rpc_attempt_interval = 1000 # adjusting default interval + # + # Or by setting the ``SHOTGUN_API_RETRY_INTERVAL`` environment variable. + # In the case that the environment variable is already set, setting the + # property on the config will override it. self.rpc_attempt_interval = 3000 # From http://docs.python.org/2.6/library/httplib.html: # If the optional timeout parameter is given, blocking operations @@ -426,8 +435,7 @@ def __init__(self, password=None, sudo_as_login=None, session_token=None, - auth_token=None, - rpc_attempt_interval=None): + auth_token=None): """ Initializes a new instance of the Shotgun client. @@ -495,10 +503,6 @@ def __init__(self, :class:`~shotgun_api3.MissingTwoFactorAuthenticationFault` will be raised if the ``auth_token`` is invalid. .. todo: Add this info to the Authentication section of the docs - :param int rpc_attempt_interval: (optional) milliseconds to wait between request retries. - By default, this will be 3000 milliseconds. You can override this by passing a value - to this parameter, or by setting the ``SHOTGUN_API_RETRY_INTERVAL`` environment variable. - In the case both are set, this parameter will take precedence. .. note:: A note about proxy connections: If you are using Python <= v2.6.2, HTTPS connections through a proxy server will not work due to a bug in the :mod:`urllib2` @@ -560,15 +564,12 @@ def __init__(self, self.config.convert_datetimes_to_utc = convert_datetimes_to_utc self.config.no_ssl_validation = NO_SSL_VALIDATION self.config.raw_http_proxy = http_proxy + self.config.rpc_attempt_interval = int(os.environ.get("SHOTGUN_API_RETRY_INTERVAL", 3000)) self._connection = None if ca_certs is not None: self.__ca_certs = ca_certs else: self.__ca_certs = os.environ.get('SHOTGUN_API_CACERTS') - if rpc_attempt_interval is not None: - self.config.rpc_attempt_interval = rpc_attempt_interval - else: - self.config.rpc_attempt_interval = int(os.environ.get("SHOTGUN_API_RETRY_INTERVAL", 3000)) self.base_url = (base_url or "").lower() self.config.scheme, self.config.server, api_base, _, _ = \ diff --git a/tests/test_client.py b/tests/test_client.py index c863b9610..c2fecb021 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -249,27 +249,30 @@ def test_set_retry_interval(self): original_env_val = os.environ.pop("SHOTGUN_API_RETRY_INTERVAL", None) try: - def run_interval_test(expected_interval, interval_parameter=None): + def run_interval_test(expected_interval, interval_property=None): self.sg = api.Shotgun(self.config.server_url, self.config.script_name, self.config.api_key, http_proxy=self.config.http_proxy, - connect=self.connect, - rpc_attempt_interval=interval_parameter) + connect=self.connect) self._setup_mock() + if interval_property: + # if a value was provided for interval_property, set the + # config's property to that value. + self.sg.config.rpc_attempt_interval = interval_property self.sg._http_request.side_effect = httplib2.HttpLib2Error self.assertEqual(self.sg.config.rpc_attempt_interval, expected_interval) self.test_network_retry() # Try passing parameter and ensure the correct interval is used. - run_interval_test(expected_interval=2500, interval_parameter=2500) + run_interval_test(expected_interval=2500, interval_property=2500) # Try setting ENV VAR and ensure the correct interval is used. os.environ["SHOTGUN_API_RETRY_INTERVAL"] = "2000" run_interval_test(expected_interval=2000) # Try both parameter and environment variable, to ensure parameter wins. - run_interval_test(expected_interval=4000, interval_parameter=4000) + run_interval_test(expected_interval=4000, interval_property=4000) finally: # Restore environment variable. From 66456089ce74d560a9aa10685e5a56f869460714 Mon Sep 17 00:00:00 2001 From: Will Cavanagh Date: Thu, 27 Jun 2019 11:02:28 -0400 Subject: [PATCH 09/15] Give a more useful warning when an invalid value is set in the SHOTGUN_API_RETRY_INTERVAL environment variable. --- shotgun_api3/shotgun.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 96d39aef9..5975d8df8 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -564,7 +564,14 @@ def __init__(self, self.config.convert_datetimes_to_utc = convert_datetimes_to_utc self.config.no_ssl_validation = NO_SSL_VALIDATION self.config.raw_http_proxy = http_proxy - self.config.rpc_attempt_interval = int(os.environ.get("SHOTGUN_API_RETRY_INTERVAL", 3000)) + + try: + self.config.rpc_attempt_interval = int(os.environ.get("SHOTGUN_API_RETRY_INTERVAL", 3000)) + except ValueError: + retry_interval = os.environ.get("SHOTGUN_API_RETRY_INTERVAL", 3000) + raise ValueError("Invalid value '%s' found in environment variable " + "SHOTGUN_API_RETRY_INTERVAL, must be int." % retry_interval) + self._connection = None if ca_certs is not None: self.__ca_certs = ca_certs From 5c7a9f1cb259f4d8512ce2093dd0de12ffe8033b Mon Sep 17 00:00:00 2001 From: Will Cavanagh Date: Thu, 27 Jun 2019 11:16:01 -0400 Subject: [PATCH 10/15] Update view_master_settings field in expected preferences. --- tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_api.py b/tests/test_api.py index 3431c983f..21be3555f 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -814,7 +814,7 @@ def test_preferences_read(self): 'hours_per_day': 8.0, 'last_day_work_week': None, 'support_local_storage': False, - '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"}]}}}' # noqa + '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 } self.assertEqual(expected, resp) From b763f2606ce007a300919b0ee8381b193cde4897 Mon Sep 17 00:00:00 2001 From: Will Cavanagh Date: Thu, 27 Jun 2019 11:16:58 -0400 Subject: [PATCH 11/15] Ensure rpc_attempt_interval is positive. --- shotgun_api3/shotgun.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index 5975d8df8..e7adf47f2 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -571,6 +571,9 @@ def __init__(self, retry_interval = os.environ.get("SHOTGUN_API_RETRY_INTERVAL", 3000) raise ValueError("Invalid value '%s' found in environment variable " "SHOTGUN_API_RETRY_INTERVAL, must be int." % retry_interval) + if self.config.rpc_attempt_interval < 0: + raise ValueError("Value of SHOTGUN_API_RETRY_INTERVAL must be positive, " + "got '%s'." % self.config.rpc_attempt_interval) self._connection = None if ca_certs is not None: From bf585ceca6b0e4b28d54f22340093e3031eb0208 Mon Sep 17 00:00:00 2001 From: Will Cavanagh Date: Thu, 27 Jun 2019 14:59:45 -0400 Subject: [PATCH 12/15] Use trusty dist for travis, since xenial doesn't support python 2.6 --- .travis.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.travis.yml b/.travis.yml index d8bfa73f0..89a7ee6c6 100644 --- a/.travis.yml +++ b/.travis.yml @@ -2,6 +2,10 @@ language: python python: - "2.6" - "2.7" + +# use trusty dist, since xenial (default) does not support python 2.6 +dist: trusty + # command to install dependencies install: - pip install -r tests/ci_requirements.txt From 769cb3cae49efa342c011f617a36aed7330d7295 Mon Sep 17 00:00:00 2001 From: Will Cavanagh Date: Fri, 28 Jun 2019 10:07:35 -0400 Subject: [PATCH 13/15] Packaging for the v3.0.41 release. --- HISTORY.rst | 4 ++++ setup.py | 2 +- shotgun_api3/shotgun.py | 2 +- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 39c5fecd3..45950bbc7 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -4,6 +4,10 @@ Shotgun Python API Changelog Here you can see the full list of changes between each Python API release. +v3.0.41 (2019 June 28) +===================== +- Adds an optional sleep between retries specified via the `SHOTGUN_API_RETRY_INTERVAL` environment variable, or by setting `sg.config.rpc_attempt_interval`. + v3.0.40 (2019 March 13) ===================== - Updates encoding method to use shutil when uploading, to avoid memory and overflow errors when reading large files. (contributed by @eestrada) diff --git a/setup.py b/setup.py index 0f3b065c0..638625434 100644 --- a/setup.py +++ b/setup.py @@ -17,7 +17,7 @@ setup( name='shotgun_api3', - version='3.0.40', + version='3.0.41', description='Shotgun Python API ', long_description=readme, author='Shotgun Software', diff --git a/shotgun_api3/shotgun.py b/shotgun_api3/shotgun.py index e7adf47f2..65a6ebc62 100755 --- a/shotgun_api3/shotgun.py +++ b/shotgun_api3/shotgun.py @@ -92,7 +92,7 @@ # ---------------------------------------------------------------------------- # Version -__version__ = "3.0.40" +__version__ = "3.0.41" # ---------------------------------------------------------------------------- # Errors From 9c6afd79bdbcae74207e91be5636c23d7dc8938c Mon Sep 17 00:00:00 2001 From: Will Cavanagh Date: Fri, 28 Jun 2019 10:34:06 -0400 Subject: [PATCH 14/15] Add documentation for environment variables. --- docs/reference.rst | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/reference.rst b/docs/reference.rst index 50eb45b3e..42c179a56 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -868,4 +868,23 @@ are specifically displaying EventLogEntries in the web application, or API queri log that are run. We are always looking for ways to improve this in the future. If you have any immediate concerns, please `reach out to our support team `_ +********************* +Environment Variables +********************* + +SHOTGUN_API_CACERTS +=================== + +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 keywork argument will be used. + + +SHOTGUN_API_RETRY_INTERVAL +========================== + +Stores the number of milliseconds to wait between request retries. By default, a value of 3000 milliseconds is used. You can override this default either by setting this environment variable, or by setting the ``rpc_attempt_interval`` property on the config like so: :: + + sg = Shotgun(site_name, script_name, script_key) + sg.config.rpc_attempt_interval = 1000 # adjusting default interval + +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. From 5810c2199c62b39ecc009b4411daa533e2b63339 Mon Sep 17 00:00:00 2001 From: Will Cavanagh Date: Fri, 28 Jun 2019 10:51:56 -0400 Subject: [PATCH 15/15] Fix spelling in documentation. --- docs/reference.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/reference.rst b/docs/reference.rst index 42c179a56..00b7b954a 100644 --- a/docs/reference.rst +++ b/docs/reference.rst @@ -875,13 +875,13 @@ Environment Variables SHOTGUN_API_CACERTS =================== -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 keywork argument will be used. +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. SHOTGUN_API_RETRY_INTERVAL ========================== -Stores the number of milliseconds to wait between request retries. By default, a value of 3000 milliseconds is used. You can override this default either by setting this environment variable, or by setting the ``rpc_attempt_interval`` property on the config like so: :: +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: :: sg = Shotgun(site_name, script_name, script_key) sg.config.rpc_attempt_interval = 1000 # adjusting default interval