From 031e31ee2d9fd9e7e5522fb51616a630cee73dd8 Mon Sep 17 00:00:00 2001 From: Gordon So Date: Fri, 7 May 2021 17:29:10 +0100 Subject: [PATCH 1/3] ISSUE-326 Expose request timeout configuration for retrieving datafile --- optimizely/config_manager.py | 32 +++++++++++++++++++++++++-- optimizely/helpers/enums.py | 2 +- tests/test_config_manager.py | 43 +++++++++++++++++++++++++++++++----- 3 files changed, 68 insertions(+), 9 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index b0f959bf..ee30a1aa 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -164,6 +164,7 @@ def __init__( error_handler=None, notification_center=None, skip_json_validation=False, + request_timeout=None, ): """ Initialize config manager. One of sdk_key or url has to be set to be able to use. @@ -198,6 +199,7 @@ def __init__( ) self.set_update_interval(update_interval) self.set_blocking_timeout(blocking_timeout) + self.set_request_timeout(request_timeout) self.last_modified = None self._polling_thread = threading.Thread(target=self._run) self._polling_thread.setDaemon(True) @@ -309,6 +311,32 @@ def set_blocking_timeout(self, blocking_timeout): self.blocking_timeout = blocking_timeout + def set_request_timeout(self, request_timeout): + """ Helper method to stop waiting for a response after a given number of seconds with the timeout parameter + + Args: + request_timeout: Time in seconds to block the config call. + """ + if request_timeout is None: + request_timeout = enums.ConfigManager.DEFAULT_REQUEST_TIMEOUT + self.logger.debug('Setting config request timeout to default value {}.'.format(request_timeout)) + + if not isinstance(request_timeout, (numbers.Integral, float)): + raise optimizely_exceptions.InvalidInputException( + 'Invalid request timeout "{}" provided.'.format(request_timeout) + ) + + # If request timeout is less than 0 then set it to default request timeout. + if request_timeout < 0: + self.logger.debug( + 'request timeout value {} too small. Defaulting to {}'.format( + request_timeout, enums.ConfigManager.DEFAULT_REQUEST_TIMEOUT + ) + ) + request_timeout = enums.ConfigManager.DEFAULT_REQUEST_TIMEOUT + + self.request_timeout = request_timeout + def set_last_modified(self, response_headers): """ Looks up and sets last modified time based on Last-Modified header in the response. @@ -346,7 +374,7 @@ def fetch_datafile(self): try: response = requests.get( - self.datafile_url, headers=request_headers, timeout=enums.ConfigManager.REQUEST_TIMEOUT, + self.datafile_url, headers=request_headers, timeout=self.request_timeout, ) except requests_exceptions.RequestException as err: self.logger.error('Fetching datafile from {} failed. Error: {}'.format(self.datafile_url, str(err))) @@ -418,7 +446,7 @@ def fetch_datafile(self): try: response = requests.get( - self.datafile_url, headers=request_headers, timeout=enums.ConfigManager.REQUEST_TIMEOUT, + self.datafile_url, headers=request_headers, timeout=self.request_timeout, ) except requests_exceptions.RequestException as err: self.logger.error('Fetching datafile from {} failed. Error: {}'.format(self.datafile_url, str(err))) diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 8339eee6..04bb055b 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -65,7 +65,7 @@ class ConfigManager(object): # Default config update interval of 5 minutes DEFAULT_UPDATE_INTERVAL = 5 * 60 # Time in seconds before which request for datafile times out - REQUEST_TIMEOUT = 10 + DEFAULT_REQUEST_TIMEOUT = 10 class ControlAttributes(object): diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index 15c93245..3685823d 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -350,6 +350,37 @@ def test_set_blocking_timeout(self, _): project_config_manager.set_blocking_timeout(5) self.assertEqual(5, project_config_manager.blocking_timeout) + def test_set_request_timeout(self, _): + """ Test set_request_timeout with different inputs. """ + with mock.patch('optimizely.config_manager.PollingConfigManager.fetch_datafile'): + project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') + + # Assert that if invalid request_timeout is set, then exception is raised. + with self.assertRaisesRegexp( + optimizely_exceptions.InvalidInputException, 'Invalid request timeout "invalid timeout" provided.', + ): + project_config_manager.set_request_timeout('invalid timeout') + + # Assert that request_timeout cannot be set to less than allowed minimum and instead is set to default value. + project_config_manager.set_request_timeout(-4) + self.assertEqual( + enums.ConfigManager.DEFAULT_REQUEST_TIMEOUT, project_config_manager.request_timeout, + ) + + # Assert that request_timeout can be set to 0. + project_config_manager.set_request_timeout(0) + self.assertIs(0, project_config_manager.request_timeout) + + # Assert that if no request_timeout is provided, it is set to default value. + project_config_manager.set_request_timeout(None) + self.assertEqual( + enums.ConfigManager.DEFAULT_REQUEST_TIMEOUT, project_config_manager.request_timeout, + ) + + # Assert that if valid request_timeout is provided, it is set to that value. + project_config_manager.set_request_timeout(5) + self.assertEqual(5, project_config_manager.request_timeout) + def test_set_last_modified(self, _): """ Test that set_last_modified sets last_modified field based on header. """ with mock.patch('optimizely.config_manager.PollingConfigManager.fetch_datafile'): @@ -388,7 +419,7 @@ def test_fetch_datafile(self, _): mock_requests.assert_called_once_with( expected_datafile_url, headers={'If-Modified-Since': test_headers['Last-Modified']}, - timeout=enums.ConfigManager.REQUEST_TIMEOUT, + timeout=enums.ConfigManager.DEFAULT_REQUEST_TIMEOUT, ) self.assertEqual(test_headers['Last-Modified'], project_config_manager.last_modified) self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig) @@ -424,7 +455,7 @@ def raise_for_status(self): mock_requests.assert_called_once_with( expected_datafile_url, headers={'If-Modified-Since': test_headers['Last-Modified']}, - timeout=enums.ConfigManager.REQUEST_TIMEOUT, + timeout=enums.ConfigManager.DEFAULT_REQUEST_TIMEOUT, ) mock_logger.error.assert_called_once_with('Fetching datafile from {} failed. Error: Error Error !!'.format( expected_datafile_url @@ -463,7 +494,7 @@ def test_fetch_datafile__request_exception_raised(self, _): mock_requests.assert_called_once_with( expected_datafile_url, headers={'If-Modified-Since': test_headers['Last-Modified']}, - timeout=enums.ConfigManager.REQUEST_TIMEOUT, + timeout=enums.ConfigManager.DEFAULT_REQUEST_TIMEOUT, ) mock_logger.error.assert_called_once_with('Fetching datafile from {} failed. Error: Error Error !!'.format( expected_datafile_url @@ -527,7 +558,7 @@ def test_fetch_datafile(self, _): expected_datafile_url, headers={'Authorization': 'Bearer {datafile_access_token}'.format( datafile_access_token=datafile_access_token)}, - timeout=enums.ConfigManager.REQUEST_TIMEOUT, + timeout=enums.ConfigManager.DEFAULT_REQUEST_TIMEOUT, ) self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig) @@ -558,7 +589,7 @@ def test_fetch_datafile__request_exception_raised(self, _): expected_datafile_url, headers={'Authorization': 'Bearer {datafile_access_token}'.format( datafile_access_token=datafile_access_token)}, - timeout=enums.ConfigManager.REQUEST_TIMEOUT, + timeout=enums.ConfigManager.DEFAULT_REQUEST_TIMEOUT, ) self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig) @@ -577,7 +608,7 @@ def test_fetch_datafile__request_exception_raised(self, _): 'Authorization': 'Bearer {datafile_access_token}'.format( datafile_access_token=datafile_access_token), }, - timeout=enums.ConfigManager.REQUEST_TIMEOUT, + timeout=enums.ConfigManager.DEFAULT_REQUEST_TIMEOUT, ) mock_logger.error.assert_called_once_with('Fetching datafile from {} failed. Error: Error Error !!'.format( expected_datafile_url From a6198545ea5e32e0ac592b7ddbd7363a5f7d533a Mon Sep 17 00:00:00 2001 From: Gordon So Date: Sat, 8 May 2021 05:45:27 +0100 Subject: [PATCH 2/3] ISSUE-326 - added docstring --- optimizely/config_manager.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index ee30a1aa..862a8b0a 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -184,6 +184,8 @@ def __init__( skip_json_validation: Optional boolean param which allows skipping JSON schema validation upon object invocation. By default JSON schema validation will be performed. + request_timeout: Optional Time in seconds to block the fetch_datafile call + until datafile object retrieved. """ self._config_ready_event = threading.Event() From 67efddfc0d97e469606e596b3f1d4a7aa66c0041 Mon Sep 17 00:00:00 2001 From: Gordon So Date: Sat, 8 May 2021 06:01:24 +0100 Subject: [PATCH 3/3] provided default settings in docstring --- optimizely/config_manager.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 862a8b0a..40b37572 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -172,9 +172,9 @@ def __init__( sdk_key: Optional string uniquely identifying the datafile. datafile: Optional JSON string representing the project. update_interval: Optional floating point number representing time interval in seconds - at which to request datafile and set ProjectConfig. + at which to request datafile and set ProjectConfig. (Default is 300 secs) blocking_timeout: Optional Time in seconds to block the get_config call until config object - has been initialized. + has been initialized. (Default is 10 secs) url: Optional string representing URL from where to fetch the datafile. If set it supersedes the sdk_key. url_template: Optional string template which in conjunction with sdk_key determines URL from where to fetch the datafile. @@ -185,7 +185,7 @@ def __init__( validation upon object invocation. By default JSON schema validation will be performed. request_timeout: Optional Time in seconds to block the fetch_datafile call - until datafile object retrieved. + until datafile object retrieved. (Default is 10 secs) """ self._config_ready_event = threading.Event()