From 4296539a1409150e46a02e1799a7790396a49d30 Mon Sep 17 00:00:00 2001 From: rashidsp Date: Mon, 23 Sep 2019 21:29:16 +0500 Subject: [PATCH 1/5] Added blocking timeout in polling manager --- optimizely/config_manager.py | 36 +++++++++++++++++++++++++++++++++++- optimizely/helpers/enums.py | 2 ++ 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 091bdca9..52f0b807 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -131,8 +131,9 @@ def _set_config(self, datafile): if previous_revision == config.get_revision(): return - + self._condition.acquire() self._config = config + self._condition.notify() # Notifies the consumer about the availability. self.notification_center.send_notifications(enums.NotificationTypes.OPTIMIZELY_CONFIG_UPDATE) self.logger.debug( 'Received new datafile and updated config. ' @@ -145,6 +146,9 @@ def get_config(self): Returns: ProjectConfig. None if not set. """ + self._condition.acquire() + self._condition.wait(self.blocking_timeout) + self._condition.release() return self._config @@ -155,6 +159,7 @@ def __init__(self, sdk_key=None, datafile=None, update_interval=None, + blocking_timeout=None, url=None, url_template=None, logger=None, @@ -168,6 +173,8 @@ def __init__(self, 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. + blocking_timeout: Optional Time in seconds to block the config call until config object + has been initialized. 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. @@ -187,10 +194,12 @@ def __init__(self, self.datafile_url = self.get_datafile_url(sdk_key, url, url_template or enums.ConfigManager.DATAFILE_URL_TEMPLATE) self.set_update_interval(update_interval) + self.set_blocking_timeout(blocking_timeout) self.last_modified = None self._polling_thread = threading.Thread(target=self._run) self._polling_thread.setDaemon(True) self._polling_thread.start() + self._condition = threading.Condition() @staticmethod def get_datafile_url(sdk_key, url, url_template): @@ -249,6 +258,31 @@ def set_update_interval(self, update_interval): self.update_interval = update_interval + def set_blocking_timeout(self, blocking_timeout): + """ Helper method to set time in seconds to block the config call until config has been initialized. + + Args: + blocking_timeout: Time in seconds to block the config call. + """ + if not blocking_timeout: + blocking_timeout = enums.ConfigManager.DEFAULT_BLOCKING_TIMEOUT + self.logger.debug('Set config blocking timeout to default value {}.'.format(blocking_timeout)) + + if not isinstance(blocking_timeout, (int, float)): + raise optimizely_exceptions.InvalidInputException( + 'Invalid blocking timeout "{}" provided.'.format(blocking_timeout) + ) + + # If blocking timeout is less than or equal to 0 then set it to default blocking timeout. + if blocking_timeout <= 0: + self.logger.debug('blocking timeout value {} too small. Defaulting to {}'.format( + blocking_timeout, + enums.ConfigManager.DEFAULT_BLOCKING_TIMEOUT) + ) + blocking_timeout = enums.ConfigManager.DEFAULT_BLOCKING_TIMEOUT + + self.blocking_timeout = blocking_timeout + def set_last_modified(self, response_headers): """ Looks up and sets last modified time based on Last-Modified header in the response. diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 73ecfe54..e0ae031f 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -38,6 +38,8 @@ class AudienceEvaluationLogs(object): class ConfigManager(object): DATAFILE_URL_TEMPLATE = 'https://cdn.optimizely.com/datafiles/{sdk_key}.json' + # Default time in seconds to block the 'config' method call until 'config' instance has been initialized. + DEFAULT_BLOCKING_TIMEOUT = 15 # Default config update interval of 5 minutes DEFAULT_UPDATE_INTERVAL = 5 * 60 # Time in seconds before which request for datafile times out From 4bb2cfbf9977f95a40688949a0b208e50d360d2a Mon Sep 17 00:00:00 2001 From: Owais Date: Thu, 26 Sep 2019 15:48:01 +0500 Subject: [PATCH 2/5] check tests --- optimizely/config_manager.py | 18 +++++++++--------- tests/test_config_manager.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 52f0b807..a5bbe6cf 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -12,6 +12,7 @@ # limitations under the License. import abc +import numbers import requests import threading import time @@ -95,6 +96,8 @@ def __init__(self, notification_center=notification_center) self._config = None self.validate_schema = not skip_json_validation + self.blocking_timeout = None + self._event = threading.Event() self._set_config(datafile) def _set_config(self, datafile): @@ -131,9 +134,8 @@ def _set_config(self, datafile): if previous_revision == config.get_revision(): return - self._condition.acquire() self._config = config - self._condition.notify() # Notifies the consumer about the availability. + self._event.set() self.notification_center.send_notifications(enums.NotificationTypes.OPTIMIZELY_CONFIG_UPDATE) self.logger.debug( 'Received new datafile and updated config. ' @@ -146,9 +148,8 @@ def get_config(self): Returns: ProjectConfig. None if not set. """ - self._condition.acquire() - self._condition.wait(self.blocking_timeout) - self._condition.release() + if self.blocking_timeout is not None: + self._event.wait(self.blocking_timeout) return self._config @@ -199,7 +200,6 @@ def __init__(self, self._polling_thread = threading.Thread(target=self._run) self._polling_thread.setDaemon(True) self._polling_thread.start() - self._condition = threading.Condition() @staticmethod def get_datafile_url(sdk_key, url, url_template): @@ -239,7 +239,7 @@ def set_update_interval(self, update_interval): Args: update_interval: Time in seconds after which to update datafile. """ - if not update_interval: + if update_interval is None: update_interval = enums.ConfigManager.DEFAULT_UPDATE_INTERVAL self.logger.debug('Set config update interval to default value {}.'.format(update_interval)) @@ -264,11 +264,11 @@ def set_blocking_timeout(self, blocking_timeout): Args: blocking_timeout: Time in seconds to block the config call. """ - if not blocking_timeout: + if blocking_timeout is None: blocking_timeout = enums.ConfigManager.DEFAULT_BLOCKING_TIMEOUT self.logger.debug('Set config blocking timeout to default value {}.'.format(blocking_timeout)) - if not isinstance(blocking_timeout, (int, float)): + if not isinstance(blocking_timeout, (numbers.Integral, int)): raise optimizely_exceptions.InvalidInputException( 'Invalid blocking timeout "{}" provided.'.format(blocking_timeout) ) diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index 040ba7b3..cba143f1 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -14,6 +14,7 @@ import json import mock import requests +import time from optimizely import config_manager from optimizely import exceptions as optimizely_exceptions @@ -235,6 +236,33 @@ def test_set_update_interval(self, _): project_config_manager.set_update_interval(42) self.assertEqual(42, project_config_manager.update_interval) + def test_set_blocking_timeout(self, _): + """ Test set_blocking_timeout with different inputs. """ + project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') + + # Assert that if invalid blocking_timeout is set, then exception is raised. + with self.assertRaisesRegexp(optimizely_exceptions.InvalidInputException, + 'Invalid blocking timeout "invalid timeout" provided.'): + project_config_manager.set_blocking_timeout('invalid timeout') + + # Assert that blocking_timeout cannot be set to less than allowed minimum and instead is set to default value. + project_config_manager.set_blocking_timeout(-4) + self.assertEqual(enums.ConfigManager.DEFAULT_BLOCKING_TIMEOUT, project_config_manager.blocking_timeout) + + # Assert that if no blocking_timeout is provided, it is set to default value. + project_config_manager.set_blocking_timeout(None) + self.assertEqual(enums.ConfigManager.DEFAULT_BLOCKING_TIMEOUT, project_config_manager.blocking_timeout) + + # Assert that if valid blocking_timeout is provided, it is set to that value. + project_config_manager.set_blocking_timeout(5) + self.assertEqual(5, project_config_manager.blocking_timeout) + + # Assert get_config should block until blocking timeout. + start_time = time.time() + project_config_manager.get_config() + end_time = time.time() + self.assertEqual(5, round(end_time - start_time)) + def test_set_last_modified(self, _): """ Test that set_last_modified sets last_modified field based on header. """ project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') From c9e5ef4a57f621fca23268f606df7764b2c6e083 Mon Sep 17 00:00:00 2001 From: Owais Date: Thu, 26 Sep 2019 17:05:57 +0500 Subject: [PATCH 3/5] revise --- optimizely/config_manager.py | 24 +++++++++++++++++------- optimizely/helpers/enums.py | 2 +- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index a5bbe6cf..9021757b 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -96,8 +96,7 @@ def __init__(self, notification_center=notification_center) self._config = None self.validate_schema = not skip_json_validation - self.blocking_timeout = None - self._event = threading.Event() + self._configReadyEvent = threading.Event() self._set_config(datafile) def _set_config(self, datafile): @@ -134,8 +133,9 @@ def _set_config(self, datafile): if previous_revision == config.get_revision(): return + self._config = config - self._event.set() + self._configReadyEvent.set() self.notification_center.send_notifications(enums.NotificationTypes.OPTIMIZELY_CONFIG_UPDATE) self.logger.debug( 'Received new datafile and updated config. ' @@ -148,8 +148,7 @@ def get_config(self): Returns: ProjectConfig. None if not set. """ - if self.blocking_timeout is not None: - self._event.wait(self.blocking_timeout) + return self._config @@ -174,7 +173,7 @@ def __init__(self, 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. - blocking_timeout: Optional Time in seconds to block the config call until config object + blocking_timeout: Optional Time in seconds to block the get_config call until config object has been initialized. 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 @@ -233,6 +232,17 @@ def get_datafile_url(sdk_key, url, url_template): return url + def get_config(self): + """ Returns instance of ProjectConfig. Returns immediately if project config is ready otherwise + blocks maximum for value of blocking_timeout in seconds. + + Returns: + ProjectConfig. None if not set. + """ + + self._configReadyEvent.wait(self.blocking_timeout) + return self._config + def set_update_interval(self, update_interval): """ Helper method to set frequency at which datafile has to be polled and ProjectConfig updated. @@ -268,7 +278,7 @@ def set_blocking_timeout(self, blocking_timeout): blocking_timeout = enums.ConfigManager.DEFAULT_BLOCKING_TIMEOUT self.logger.debug('Set config blocking timeout to default value {}.'.format(blocking_timeout)) - if not isinstance(blocking_timeout, (numbers.Integral, int)): + if not isinstance(blocking_timeout, (numbers.Integral, float)): raise optimizely_exceptions.InvalidInputException( 'Invalid blocking timeout "{}" provided.'.format(blocking_timeout) ) diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index e0ae031f..9b875bdd 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -38,7 +38,7 @@ class AudienceEvaluationLogs(object): class ConfigManager(object): DATAFILE_URL_TEMPLATE = 'https://cdn.optimizely.com/datafiles/{sdk_key}.json' - # Default time in seconds to block the 'config' method call until 'config' instance has been initialized. + # Default time in seconds to block the 'get_config' method call until 'config' instance has been initialized. DEFAULT_BLOCKING_TIMEOUT = 15 # Default config update interval of 5 minutes DEFAULT_UPDATE_INTERVAL = 5 * 60 From a1925bfae7289a2f3848f6a9f8a3ce122d12f1a6 Mon Sep 17 00:00:00 2001 From: Owais Date: Fri, 27 Sep 2019 10:04:36 +0500 Subject: [PATCH 4/5] address comments --- optimizely/config_manager.py | 14 +++++++------- optimizely/helpers/enums.py | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 9021757b..11eb1959 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -96,7 +96,7 @@ def __init__(self, notification_center=notification_center) self._config = None self.validate_schema = not skip_json_validation - self._configReadyEvent = threading.Event() + self._config_ready_event = threading.Event() self._set_config(datafile) def _set_config(self, datafile): @@ -135,7 +135,7 @@ def _set_config(self, datafile): return self._config = config - self._configReadyEvent.set() + self._config_ready_event.set() self.notification_center.send_notifications(enums.NotificationTypes.OPTIMIZELY_CONFIG_UPDATE) self.logger.debug( 'Received new datafile and updated config. ' @@ -240,7 +240,7 @@ def get_config(self): ProjectConfig. None if not set. """ - self._configReadyEvent.wait(self.blocking_timeout) + self._config_ready_event.wait(self.blocking_timeout) return self._config def set_update_interval(self, update_interval): @@ -251,7 +251,7 @@ def set_update_interval(self, update_interval): """ if update_interval is None: update_interval = enums.ConfigManager.DEFAULT_UPDATE_INTERVAL - self.logger.debug('Set config update interval to default value {}.'.format(update_interval)) + self.logger.debug('Setting config update interval to default value {}.'.format(update_interval)) if not isinstance(update_interval, (int, float)): raise optimizely_exceptions.InvalidInputException( @@ -276,15 +276,15 @@ def set_blocking_timeout(self, blocking_timeout): """ if blocking_timeout is None: blocking_timeout = enums.ConfigManager.DEFAULT_BLOCKING_TIMEOUT - self.logger.debug('Set config blocking timeout to default value {}.'.format(blocking_timeout)) + self.logger.debug('Setting config blocking timeout to default value {}.'.format(blocking_timeout)) if not isinstance(blocking_timeout, (numbers.Integral, float)): raise optimizely_exceptions.InvalidInputException( 'Invalid blocking timeout "{}" provided.'.format(blocking_timeout) ) - # If blocking timeout is less than or equal to 0 then set it to default blocking timeout. - if blocking_timeout <= 0: + # If blocking timeout is less than 0 then set it to default blocking timeout. + if blocking_timeout < 0: self.logger.debug('blocking timeout value {} too small. Defaulting to {}'.format( blocking_timeout, enums.ConfigManager.DEFAULT_BLOCKING_TIMEOUT) diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 9b875bdd..506d4981 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -39,7 +39,7 @@ class AudienceEvaluationLogs(object): class ConfigManager(object): DATAFILE_URL_TEMPLATE = 'https://cdn.optimizely.com/datafiles/{sdk_key}.json' # Default time in seconds to block the 'get_config' method call until 'config' instance has been initialized. - DEFAULT_BLOCKING_TIMEOUT = 15 + DEFAULT_BLOCKING_TIMEOUT = 10 # Default config update interval of 5 minutes DEFAULT_UPDATE_INTERVAL = 5 * 60 # Time in seconds before which request for datafile times out From 1e4ace45ca8e9e55b3b3d7464b9be9e574d2bee3 Mon Sep 17 00:00:00 2001 From: Owais Date: Fri, 27 Sep 2019 10:09:15 +0500 Subject: [PATCH 5/5] add unit test --- tests/test_config_manager.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index cba143f1..905b7a65 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -249,6 +249,10 @@ def test_set_blocking_timeout(self, _): project_config_manager.set_blocking_timeout(-4) self.assertEqual(enums.ConfigManager.DEFAULT_BLOCKING_TIMEOUT, project_config_manager.blocking_timeout) + # Assert that blocking_timeout can be set to 0. + project_config_manager.set_blocking_timeout(0) + self.assertIs(0, project_config_manager.blocking_timeout) + # Assert that if no blocking_timeout is provided, it is set to default value. project_config_manager.set_blocking_timeout(None) self.assertEqual(enums.ConfigManager.DEFAULT_BLOCKING_TIMEOUT, project_config_manager.blocking_timeout)