From 26ecf1b1ccdb7ef48a146a54482a41472fd60532 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 7 May 2019 17:56:29 -0700 Subject: [PATCH 01/14] Introducing datafile manager package. WIP. --- optimizely/datafile_manager/__init__.py | 12 +++ optimizely/datafile_manager/base.py | 16 ++++ .../polling_datafile_manager.py | 80 +++++++++++++++++++ .../static_datafile_manager.py | 23 ++++++ 4 files changed, 131 insertions(+) create mode 100644 optimizely/datafile_manager/__init__.py create mode 100644 optimizely/datafile_manager/base.py create mode 100644 optimizely/datafile_manager/polling_datafile_manager.py create mode 100644 optimizely/datafile_manager/static_datafile_manager.py diff --git a/optimizely/datafile_manager/__init__.py b/optimizely/datafile_manager/__init__.py new file mode 100644 index 000000000..2d7c2f05b --- /dev/null +++ b/optimizely/datafile_manager/__init__.py @@ -0,0 +1,12 @@ +# Copyright 2019, Optimizely +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. diff --git a/optimizely/datafile_manager/base.py b/optimizely/datafile_manager/base.py new file mode 100644 index 000000000..b2bffea83 --- /dev/null +++ b/optimizely/datafile_manager/base.py @@ -0,0 +1,16 @@ +# Copyright 2019, Optimizely +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +class BaseDatafileManager(object): + def get_datafile(self): + raise NotImplementedError diff --git a/optimizely/datafile_manager/polling_datafile_manager.py b/optimizely/datafile_manager/polling_datafile_manager.py new file mode 100644 index 000000000..7d4d0a5e9 --- /dev/null +++ b/optimizely/datafile_manager/polling_datafile_manager.py @@ -0,0 +1,80 @@ +# Copyright 2019, Optimizely +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + + +import asyncio +import http +import requests + +from .base import BaseDatafileManager + + +DATAFILE_URL_TEMPLATE = 'https://cdn.optimizely.com/datafiles/{sdk_key}.json' + + +class PollingDatafileManager(BaseDatafileManager): + + def __init__(self, + sdk_key=None, + url=None, + url_template=None): + self.is_started = False + assert sdk_key is not None or url is not None, 'Must provide at least one of sdk_key or url.' + datafile_url_template = url_template or DATAFILE_URL_TEMPLATE + if url is None: + self.datafile_url = datafile_url_template.format(sdk_key=sdk_key) + else: + self.datafile_url = url + self.datafile = None + self.last_modified = None + + def __del__(self): + self.stop() + + def set_last_modified(self, response): + self.last_modified = response.headers['Last-Modified'] + + def set_datafile(self, response): + if response.status_code == http.HTTPStatus.NOT_MODIFIED: + return + self.datafile = response.json() + + def fetch_datafile(self): + request_headers = { + 'If-Modified-Since': self.last_modified + } + response = requests.get(self.datafile_url, headers=request_headers) + + if response.status_code == http.HTTPStatus.OK: + self.set_datafile(response) + self.set_last_modified(response) + + async def _run(self): + while True: + self.fetch_datafile() + await asyncio.sleep(5) + + def start(self): + if not self.is_started: + self.is_started = True + event_loop = asyncio.get_event_loop() + event_loop.run_until_complete(self._run()) + + def stop(self): + if self.is_started: + self.is_started = False + event_loop = asyncio.get_event_loop() + event_loop.close() + + def get_datafile(self): + return self.datafile diff --git a/optimizely/datafile_manager/static_datafile_manager.py b/optimizely/datafile_manager/static_datafile_manager.py new file mode 100644 index 000000000..e2f7b2e3b --- /dev/null +++ b/optimizely/datafile_manager/static_datafile_manager.py @@ -0,0 +1,23 @@ +# Copyright 2019, Optimizely +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from .base import BaseDatafileManager + + +class StaticDatafileManager(BaseDatafileManager): + + def __init__(self, datafile): + self.datafile = datafile + + def get_datafile(self): + return self.datafile From 608b3352334ae7424c04714c80852c203bf14f36 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Fri, 10 May 2019 16:45:04 -0700 Subject: [PATCH 02/14] Moving to use threading --- ..._datafile_manager.py => config_manager.py} | 40 ++++++++++++------- optimizely/datafile_manager/__init__.py | 12 ------ optimizely/datafile_manager/base.py | 16 -------- .../static_datafile_manager.py | 23 ----------- optimizely/optimizely.py | 3 +- optimizely/project_config.py | 10 ++--- tests/test_optimizely.py | 4 +- 7 files changed, 34 insertions(+), 74 deletions(-) rename optimizely/{datafile_manager/polling_datafile_manager.py => config_manager.py} (73%) delete mode 100644 optimizely/datafile_manager/__init__.py delete mode 100644 optimizely/datafile_manager/base.py delete mode 100644 optimizely/datafile_manager/static_datafile_manager.py diff --git a/optimizely/datafile_manager/polling_datafile_manager.py b/optimizely/config_manager.py similarity index 73% rename from optimizely/datafile_manager/polling_datafile_manager.py rename to optimizely/config_manager.py index 7d4d0a5e9..2546c4e75 100644 --- a/optimizely/datafile_manager/polling_datafile_manager.py +++ b/optimizely/config_manager.py @@ -12,23 +12,37 @@ # limitations under the License. -import asyncio import http import requests +import threading -from .base import BaseDatafileManager +from . import project_config DATAFILE_URL_TEMPLATE = 'https://cdn.optimizely.com/datafiles/{sdk_key}.json' -class PollingDatafileManager(BaseDatafileManager): +class BaseConfigManager(object): + def get_config(self, *args, **kwargs): + raise NotImplementedError - def __init__(self, - sdk_key=None, - url=None, - url_template=None): + +class StaticDatafileManager(BaseConfigManager): + + def __init__(self, datafile): + self.datafile = datafile + + def get_config(self, *args, **kwargs): + return project_config.ProjectConfig(self.datafile, *args, **kwargs) + + +class PollingConfigManager(BaseConfigManager): + + def __init__(self, **kwargs): self.is_started = False + sdk_key = kwargs.get('sdk_key') + url = kwargs.get('url') + url_template = kwargs.get('url_template') assert sdk_key is not None or url is not None, 'Must provide at least one of sdk_key or url.' datafile_url_template = url_template or DATAFILE_URL_TEMPLATE if url is None: @@ -37,9 +51,8 @@ def __init__(self, self.datafile_url = url self.datafile = None self.last_modified = None - - def __del__(self): - self.stop() + self.polling_thread = threading.Thread(target=self.fetch_datafile) + self._is_running = False def set_last_modified(self, response): self.last_modified = response.headers['Last-Modified'] @@ -65,8 +78,8 @@ async def _run(self): await asyncio.sleep(5) def start(self): - if not self.is_started: - self.is_started = True + if not self._is_running: + self._is_running= True event_loop = asyncio.get_event_loop() event_loop.run_until_complete(self._run()) @@ -75,6 +88,3 @@ def stop(self): self.is_started = False event_loop = asyncio.get_event_loop() event_loop.close() - - def get_datafile(self): - return self.datafile diff --git a/optimizely/datafile_manager/__init__.py b/optimizely/datafile_manager/__init__.py deleted file mode 100644 index 2d7c2f05b..000000000 --- a/optimizely/datafile_manager/__init__.py +++ /dev/null @@ -1,12 +0,0 @@ -# Copyright 2019, Optimizely -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 - -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. diff --git a/optimizely/datafile_manager/base.py b/optimizely/datafile_manager/base.py deleted file mode 100644 index b2bffea83..000000000 --- a/optimizely/datafile_manager/base.py +++ /dev/null @@ -1,16 +0,0 @@ -# Copyright 2019, Optimizely -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 - -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -class BaseDatafileManager(object): - def get_datafile(self): - raise NotImplementedError diff --git a/optimizely/datafile_manager/static_datafile_manager.py b/optimizely/datafile_manager/static_datafile_manager.py deleted file mode 100644 index e2f7b2e3b..000000000 --- a/optimizely/datafile_manager/static_datafile_manager.py +++ /dev/null @@ -1,23 +0,0 @@ -# Copyright 2019, Optimizely -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 - -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from .base import BaseDatafileManager - - -class StaticDatafileManager(BaseDatafileManager): - - def __init__(self, datafile): - self.datafile = datafile - - def get_datafile(self): - return self.datafile diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index 0ca27fb6b..bc42e10df 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -12,12 +12,13 @@ # limitations under the License. from six import string_types +from . import config_manager from . import decision_service from . import entities from . import event_builder from . import exceptions -from . import logger as _logging from . import project_config +from . import logger as _logging from .error_handler import NoOpErrorHandler as noop_error_handler from .event_dispatcher import EventDispatcher as default_event_dispatcher from .helpers import enums diff --git a/optimizely/project_config.py b/optimizely/project_config.py index dea4ac9d8..85956fd1c 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -13,11 +13,11 @@ import json -from .helpers import condition as condition_helper -from .helpers import enums -from .helpers import validator -from . import entities -from . import exceptions +from optimizely.helpers import condition as condition_helper +from optimizely.helpers import enums +from optimizely.helpers import validator +from optimizely import entities +from optimizely import exceptions SUPPORTED_VERSIONS = [enums.DatafileVersions.V2, enums.DatafileVersions.V3, enums.DatafileVersions.V4] diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index b9d02c45c..c40259bf3 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -22,7 +22,7 @@ from optimizely import exceptions from optimizely import logger from optimizely import optimizely -from optimizely import project_config +from optimizely import config_manager from optimizely import version from optimizely.helpers import enums from optimizely.notification_center import NotificationCenter @@ -164,7 +164,7 @@ def test_init__unsupported_datafile_version__logs_error(self): def test_init_with_supported_datafile_version(self): """ Test that datafile with supported version works as expected. """ - self.assertTrue(self.config_dict['version'] in project_config.SUPPORTED_VERSIONS) + self.assertTrue(self.config_dict['version'] in config_manager.SUPPORTED_VERSIONS) mock_client_logger = mock.MagicMock() with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger): From e9afc1113ca8e37a46fda9100588e4576e1c2af4 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Wed, 15 May 2019 17:43:24 -0700 Subject: [PATCH 03/14] More changes. WIP. --- optimizely/config_manager.py | 62 +++++++++++++++++++++--------------- optimizely/helpers/enums.py | 5 +++ 2 files changed, 41 insertions(+), 26 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 2546c4e75..38e0abc5e 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -15,15 +15,17 @@ import http import requests import threading +import time from . import project_config +from .helpers import enums DATAFILE_URL_TEMPLATE = 'https://cdn.optimizely.com/datafiles/{sdk_key}.json' class BaseConfigManager(object): - def get_config(self, *args, **kwargs): + def get_config(self): raise NotImplementedError @@ -37,34 +39,45 @@ def get_config(self, *args, **kwargs): class PollingConfigManager(BaseConfigManager): + MIN_UPDATE_INTERVAL = 1 + DEFAULT_UPDATE_INTERVAL = 5 * 60 + + def __init__(self, + sdk_key=None, + update_interval=None, + url=None, + url_template=None): + self._set_datafile_url(sdk_key, url, url_template) + self._set_update_interval(update_interval) + self.datafile = None + self.last_modified = None + self._polling_thread = threading.Thread(target=self._run) + self.is_running = False - def __init__(self, **kwargs): - self.is_started = False - sdk_key = kwargs.get('sdk_key') - url = kwargs.get('url') - url_template = kwargs.get('url_template') + def _set_datafile_url(self, sdk_key, url, url_template): assert sdk_key is not None or url is not None, 'Must provide at least one of sdk_key or url.' - datafile_url_template = url_template or DATAFILE_URL_TEMPLATE + url_template = url_template or DATAFILE_URL_TEMPLATE if url is None: - self.datafile_url = datafile_url_template.format(sdk_key=sdk_key) + self.datafile_url = url_template.format(sdk_key=sdk_key) else: self.datafile_url = url - self.datafile = None - self.last_modified = None - self.polling_thread = threading.Thread(target=self.fetch_datafile) - self._is_running = False + + def _set_update_interval(self, update_interval): + self.update_interval = update_interval or self.DEFAULT_UPDATE_INTERVAL + if update_interval < self.MIN_UPDATE_INTERVAL: + self.update_interval = self.DEFAULT_UPDATE_INTERVAL def set_last_modified(self, response): - self.last_modified = response.headers['Last-Modified'] + self.last_modified = response.headers[enums.HTTPHeaders.LAST_MODIFIED] def set_datafile(self, response): if response.status_code == http.HTTPStatus.NOT_MODIFIED: return - self.datafile = response.json() + self.datafile = response.text def fetch_datafile(self): request_headers = { - 'If-Modified-Since': self.last_modified + enums.HTTPHeaders.IF_MODIFIED_SINCE: self.last_modified } response = requests.get(self.datafile_url, headers=request_headers) @@ -72,19 +85,16 @@ def fetch_datafile(self): self.set_datafile(response) self.set_last_modified(response) - async def _run(self): - while True: + def _run(self): + while self.is_running: self.fetch_datafile() - await asyncio.sleep(5) + time.sleep(self.update_interval) def start(self): - if not self._is_running: - self._is_running= True - event_loop = asyncio.get_event_loop() - event_loop.run_until_complete(self._run()) + if not self.is_running: + self.is_running= True + self._polling_thread.start() def stop(self): - if self.is_started: - self.is_started = False - event_loop = asyncio.get_event_loop() - event_loop.close() + if self.is_running: + self.is_running = False diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 25f6da591..087c6a463 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -79,6 +79,11 @@ class Errors(object): UNSUPPORTED_DATAFILE_VERSION = 'This version of the Python SDK does not support the given datafile version: "{}".' +class HTTPHeaders(object): + IF_MODIFIED_SINCE = 'If-Modified-Since' + LAST_MODIFIED = 'Last-Modified' + + class HTTPVerbs(object): GET = 'GET' POST = 'POST' From 44e72e59118a28d41ec153c72d666f41494a3276 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Thu, 16 May 2019 14:44:49 -0700 Subject: [PATCH 04/14] Introducing config manager with support for polling. --- optimizely/config_manager.py | 170 +++++++++++++++++++++++++++-------- optimizely/helpers/enums.py | 8 ++ optimizely/logger.py | 6 +- optimizely/optimizely.py | 3 +- tests/test_optimizely.py | 7 +- 5 files changed, 150 insertions(+), 44 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 38e0abc5e..2bbe86a4d 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -16,85 +16,183 @@ import requests import threading import time +from requests import exceptions as requests_exceptions -from . import project_config -from .helpers import enums - - -DATAFILE_URL_TEMPLATE = 'https://cdn.optimizely.com/datafiles/{sdk_key}.json' +from optimizely import exceptions +from optimizely import logger as _logging +from optimizely import project_config +from optimizely.error_handler import NoOpErrorHandler as noop_error_handler +from optimizely.helpers import enums class BaseConfigManager(object): + """ Base class for Optimizely's config manager. """ + def get_config(self): + """ Get config for use by optimizely.Optimizely. + The config should be an instance of project_config.ProjectConfig.""" raise NotImplementedError -class StaticDatafileManager(BaseConfigManager): +class StaticConfigManager(BaseConfigManager): + """ Config manager that returns ProjectConfig based on provided datafile. """ - def __init__(self, datafile): - self.datafile = datafile + def __init__(self, + datafile, + logger=None, + error_handler=None): + self.logger = logger or _logging.adapt_logger(logger or _logging.NoOpLogger()) + self.error_handler = error_handler or noop_error_handler + self._datafile = datafile + self._config = project_config.ProjectConfig(self._datafile, self.logger, self.error_handler) + + def get_config(self): + """ Returns instance of ProjectConfig. - def get_config(self, *args, **kwargs): - return project_config.ProjectConfig(self.datafile, *args, **kwargs) + Returns: + ProjectConfig. + """ + return self._config class PollingConfigManager(BaseConfigManager): - MIN_UPDATE_INTERVAL = 1 - DEFAULT_UPDATE_INTERVAL = 5 * 60 + """ Config manager that polls for the datafile and updated ProjectConfig based on an update interval. """ def __init__(self, sdk_key=None, update_interval=None, url=None, - url_template=None): - self._set_datafile_url(sdk_key, url, url_template) - self._set_update_interval(update_interval) - self.datafile = None + url_template=None, + logger=None, + error_handler=None): + """ Initialize config manager. One of sdk_key or url has to be set to be able to use. + + Args: + sdk_key: Optional string uniquely identifying the datafile. + update_interval: Optional floating point number representing time interval in seconds + at which to request datafile and set ProjectConfig. + 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. + logger: Provides a logger instance. + error_handler: Provides a handle_error method to handle exceptions. + """ + self.datafile_url = self.get_datafile_url(sdk_key, url, url_template) + self.update_interval = self.get_update_interval(update_interval) self.last_modified = None + self._datafile = None + self._config = None self._polling_thread = threading.Thread(target=self._run) self.is_running = False + self.logger = logger or _logging.adapt_logger(logger or _logging.SimpleLogger(min_level=enums.LogLevels.DEBUG)) + self.error_handler = error_handler or noop_error_handler - def _set_datafile_url(self, sdk_key, url, url_template): - assert sdk_key is not None or url is not None, 'Must provide at least one of sdk_key or url.' - url_template = url_template or DATAFILE_URL_TEMPLATE + @staticmethod + def get_datafile_url(sdk_key, url, url_template): + """ Helper method to determine URL from where to fetch the datafile. + + Args: + sdk_key: Key uniquely identifying the datafile. + url: String representing URL from which to fetch the datafile. + url_template: String representing template which is filled in with + SDK key to determine URL from which to fetch the datafile. + + Returns: + String representing URL to fetch datafile from. + """ + # Ensure that either is provided by the user. + if sdk_key is None and url is None: + raise exceptions.InvalidInputException('Must provide at least one of sdk_key or url.') + + url_template = url_template or enums.ConfigManager.DATAFILE_URL_TEMPLATE + + # Return URL if one is provided or use template and SDK key to get it. if url is None: - self.datafile_url = url_template.format(sdk_key=sdk_key) - else: - self.datafile_url = url + return url_template.format(sdk_key=sdk_key) + + return url - def _set_update_interval(self, update_interval): - self.update_interval = update_interval or self.DEFAULT_UPDATE_INTERVAL - if update_interval < self.MIN_UPDATE_INTERVAL: - self.update_interval = self.DEFAULT_UPDATE_INTERVAL + def get_update_interval(self, update_interval): + """ Helper method to determine frequency at which datafile has to be polled and ProjectConfig updated. + + Args: + update_interval: Time in seconds optionally sent in by the user. + """ + polling_interval = update_interval or enums.ConfigManager.DEFAULT_UPDATE_INTERVAL + + # If polling interval is less than minimum allowed interval then set it to default update interval. + if update_interval < enums.ConfigManager.MIN_UPDATE_INTERVAL: + self.logger.debug('Invalid update_interval {} provided. Defaulting to {}'.format( + update_interval, + enums.ConfigManager.DEFAULT_UPDATE_INTERVAL) + ) + polling_interval = enums.ConfigManager.DEFAULT_UPDATE_INTERVAL + + return polling_interval def set_last_modified(self, response): - self.last_modified = response.headers[enums.HTTPHeaders.LAST_MODIFIED] + """ Looks up and sets last modified time based on Last-Modified header in the response. + + Args: + response: requests.Response + """ + self.last_modified = response.headers.get(enums.HTTPHeaders.LAST_MODIFIED) + + def set_config(self, response): + """ Looks up and sets datafile and config based on response body. + + Args: + response: requests.Response + """ + try: + response.raise_for_status() + except requests_exceptions.HTTPError as err: + self.logger.error('Fetching datafile from {} failed. Error: {}'.format(self.datafile_url, str(err))) + return - def set_datafile(self, response): + # Leave datafile and config unchanged if it has not been modified. if response.status_code == http.HTTPStatus.NOT_MODIFIED: + self.logger.debug('Not updating config as datafile has not updated since {}.'.format(self.last_modified)) return - self.datafile = response.text + + self.set_last_modified(response) + # TODO(ali): Add validation here to make sure that we do not update datafile and config if not a valid datafile. + self._datafile = response.text + # TODO(ali): Add notification listener. + self._config = project_config.ProjectConfig(self._datafile, self.logger, self.error_handler) + self.logger.info('Received new datafile and updated config.') + + def get_config(self): + """ Returns instance of ProjectConfig. + + Returns: + ProjectConfig. + """ + return self._config def fetch_datafile(self): - request_headers = { - enums.HTTPHeaders.IF_MODIFIED_SINCE: self.last_modified - } - response = requests.get(self.datafile_url, headers=request_headers) + """ Fetch datafile and set ProjectConfig. """ - if response.status_code == http.HTTPStatus.OK: - self.set_datafile(response) - self.set_last_modified(response) + request_headers = {} + if self.last_modified: + request_headers[enums.HTTPHeaders.IF_MODIFIED_SINCE] = self.last_modified + + response = requests.get(self.datafile_url, headers=request_headers) + self.set_config(response) def _run(self): + """ Triggered as part of the thread which fetches the datafile and sleeps until next update interval. """ while self.is_running: self.fetch_datafile() time.sleep(self.update_interval) def start(self): + """ Start the config manager and the thread to periodically fetch datafile. """ if not self.is_running: self.is_running= True self._polling_thread.start() def stop(self): + """ Stops the config manager. """ if self.is_running: self.is_running = False diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 087c6a463..2fb9516e5 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -36,6 +36,14 @@ class AudienceEvaluationLogs(object): 'newer release of the Optimizely SDK.' +class ConfigManager(object): + DATAFILE_URL_TEMPLATE = 'https://cdn.optimizely.com/datafiles/{sdk_key}.json' + # Default config update interval of 5 minutes + DEFAULT_UPDATE_INTERVAL = 5 * 60 + # Minimum config update interval of 1 second + MIN_UPDATE_INTERVAL = 1 + + class ControlAttributes(object): BOT_FILTERING = '$opt_bot_filtering' BUCKETING_ID = '$opt_bucketing_id' diff --git a/optimizely/logger.py b/optimizely/logger.py index 293172074..80c1e5472 100644 --- a/optimizely/logger.py +++ b/optimizely/logger.py @@ -1,4 +1,4 @@ -# Copyright 2016, Optimizely +# Copyright 2016, 2018, Optimizely # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at @@ -46,8 +46,8 @@ def reset_logger(name, level=None, handler=None): handler.setFormatter(logging.Formatter(_DEFAULT_LOG_FORMAT)) # We don't use ``.addHandler``, since this logger may have already been - # instantiated elsewhere with a different handler. It should only ever - # have one, not many. + # instantiated elsewhere with a different handler. It should only ever + # have one, not many. logger.handlers = [handler] return logger diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index bc42e10df..ea51822c2 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -12,7 +12,6 @@ # limitations under the License. from six import string_types -from . import config_manager from . import decision_service from . import entities from . import event_builder @@ -59,7 +58,7 @@ def __init__(self, except exceptions.InvalidInputException as error: self.is_valid = False # We actually want to log this error to stderr, so make sure the logger - # has a handler capable of doing that. + # has a handler capable of doing that. self.logger = _logging.reset_logger(self.logger_name) self.logger.exception(str(error)) return diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index c40259bf3..659a3ee25 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -12,17 +12,18 @@ # limitations under the License. import json -import mock from operator import itemgetter -from optimizely import decision_service +import mock + +import optimizely.config_manager +from optimizely import decision_service, config_manager from optimizely import entities from optimizely import error_handler from optimizely import event_builder from optimizely import exceptions from optimizely import logger from optimizely import optimizely -from optimizely import config_manager from optimizely import version from optimizely.helpers import enums from optimizely.notification_center import NotificationCenter From e9530fe3091928eae1913bc1a2f48892a959c3d0 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Thu, 16 May 2019 14:51:03 -0700 Subject: [PATCH 05/14] undoing some changes --- optimizely/optimizely.py | 2 +- optimizely/project_config.py | 10 +++++----- tests/test_optimizely.py | 9 ++++----- 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/optimizely/optimizely.py b/optimizely/optimizely.py index ea51822c2..74f921acb 100644 --- a/optimizely/optimizely.py +++ b/optimizely/optimizely.py @@ -16,8 +16,8 @@ from . import entities from . import event_builder from . import exceptions -from . import project_config from . import logger as _logging +from . import project_config from .error_handler import NoOpErrorHandler as noop_error_handler from .event_dispatcher import EventDispatcher as default_event_dispatcher from .helpers import enums diff --git a/optimizely/project_config.py b/optimizely/project_config.py index 85956fd1c..dea4ac9d8 100644 --- a/optimizely/project_config.py +++ b/optimizely/project_config.py @@ -13,11 +13,11 @@ import json -from optimizely.helpers import condition as condition_helper -from optimizely.helpers import enums -from optimizely.helpers import validator -from optimizely import entities -from optimizely import exceptions +from .helpers import condition as condition_helper +from .helpers import enums +from .helpers import validator +from . import entities +from . import exceptions SUPPORTED_VERSIONS = [enums.DatafileVersions.V2, enums.DatafileVersions.V3, enums.DatafileVersions.V4] diff --git a/tests/test_optimizely.py b/tests/test_optimizely.py index 659a3ee25..b9d02c45c 100644 --- a/tests/test_optimizely.py +++ b/tests/test_optimizely.py @@ -12,18 +12,17 @@ # limitations under the License. import json -from operator import itemgetter - import mock +from operator import itemgetter -import optimizely.config_manager -from optimizely import decision_service, config_manager +from optimizely import decision_service from optimizely import entities from optimizely import error_handler from optimizely import event_builder from optimizely import exceptions from optimizely import logger from optimizely import optimizely +from optimizely import project_config from optimizely import version from optimizely.helpers import enums from optimizely.notification_center import NotificationCenter @@ -165,7 +164,7 @@ def test_init__unsupported_datafile_version__logs_error(self): def test_init_with_supported_datafile_version(self): """ Test that datafile with supported version works as expected. """ - self.assertTrue(self.config_dict['version'] in config_manager.SUPPORTED_VERSIONS) + self.assertTrue(self.config_dict['version'] in project_config.SUPPORTED_VERSIONS) mock_client_logger = mock.MagicMock() with mock.patch('optimizely.logger.reset_logger', return_value=mock_client_logger): From fc9dc328302fb4ae69693bad946a8567b3b42b7a Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Thu, 16 May 2019 14:51:53 -0700 Subject: [PATCH 06/14] Fixing year --- optimizely/logger.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/logger.py b/optimizely/logger.py index 80c1e5472..9530b132e 100644 --- a/optimizely/logger.py +++ b/optimizely/logger.py @@ -1,4 +1,4 @@ -# Copyright 2016, 2018, Optimizely +# Copyright 2016, 2018-2019, Optimizely # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at From 8bb65fdd3556e4648144b17543a5966143c7fb65 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Thu, 23 May 2019 10:02:20 -0700 Subject: [PATCH 07/14] Updating as per feedback --- optimizely/config_manager.py | 51 ++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 2bbe86a4d..c140d0de1 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -19,7 +19,7 @@ from requests import exceptions as requests_exceptions from optimizely import exceptions -from optimizely import logger as _logging +from optimizely import logger as optimizely_logger from optimizely import project_config from optimizely.error_handler import NoOpErrorHandler as noop_error_handler from optimizely.helpers import enums @@ -41,7 +41,7 @@ def __init__(self, datafile, logger=None, error_handler=None): - self.logger = logger or _logging.adapt_logger(logger or _logging.NoOpLogger()) + self.logger = logger or optimizely_logger.adapt_logger(logger or optimizely_logger.NoOpLogger()) self.error_handler = error_handler or noop_error_handler self._datafile = datafile self._config = project_config.ProjectConfig(self._datafile, self.logger, self.error_handler) @@ -77,14 +77,15 @@ def __init__(self, logger: Provides a logger instance. error_handler: Provides a handle_error method to handle exceptions. """ - self.datafile_url = self.get_datafile_url(sdk_key, url, url_template) + self.datafile_url = self.get_datafile_url(sdk_key, url, url_template or enums.ConfigManager.DATAFILE_URL_TEMPLATE) self.update_interval = self.get_update_interval(update_interval) self.last_modified = None self._datafile = None self._config = None self._polling_thread = threading.Thread(target=self._run) self.is_running = False - self.logger = logger or _logging.adapt_logger(logger or _logging.SimpleLogger(min_level=enums.LogLevels.DEBUG)) + self.logger = logger or optimizely_logger.adapt_logger( + logger or optimizely_logger.SimpleLogger(min_level=enums.LogLevels.DEBUG)) self.error_handler = error_handler or noop_error_handler @staticmethod @@ -104,8 +105,6 @@ def get_datafile_url(sdk_key, url, url_template): if sdk_key is None and url is None: raise exceptions.InvalidInputException('Must provide at least one of sdk_key or url.') - url_template = url_template or enums.ConfigManager.DATAFILE_URL_TEMPLATE - # Return URL if one is provided or use template and SDK key to get it. if url is None: return url_template.format(sdk_key=sdk_key) @@ -118,7 +117,7 @@ def get_update_interval(self, update_interval): Args: update_interval: Time in seconds optionally sent in by the user. """ - polling_interval = update_interval or enums.ConfigManager.DEFAULT_UPDATE_INTERVAL + update_interval = update_interval or enums.ConfigManager.DEFAULT_UPDATE_INTERVAL # If polling interval is less than minimum allowed interval then set it to default update interval. if update_interval < enums.ConfigManager.MIN_UPDATE_INTERVAL: @@ -126,9 +125,9 @@ def get_update_interval(self, update_interval): update_interval, enums.ConfigManager.DEFAULT_UPDATE_INTERVAL) ) - polling_interval = enums.ConfigManager.DEFAULT_UPDATE_INTERVAL + update_interval = enums.ConfigManager.DEFAULT_UPDATE_INTERVAL - return polling_interval + return update_interval def set_last_modified(self, response): """ Looks up and sets last modified time based on Last-Modified header in the response. @@ -144,18 +143,6 @@ def set_config(self, response): Args: response: requests.Response """ - try: - response.raise_for_status() - except requests_exceptions.HTTPError as err: - self.logger.error('Fetching datafile from {} failed. Error: {}'.format(self.datafile_url, str(err))) - return - - # Leave datafile and config unchanged if it has not been modified. - if response.status_code == http.HTTPStatus.NOT_MODIFIED: - self.logger.debug('Not updating config as datafile has not updated since {}.'.format(self.last_modified)) - return - - self.set_last_modified(response) # TODO(ali): Add validation here to make sure that we do not update datafile and config if not a valid datafile. self._datafile = response.text # TODO(ali): Add notification listener. @@ -170,6 +157,26 @@ def get_config(self): """ return self._config + def _handle_response(self, response): + """ Helper method to handle response containing datafile. + + Args: + response: requests.Response + """ + try: + response.raise_for_status() + except requests_exceptions.HTTPError as err: + self.logger.error('Fetching datafile from {} failed. Error: {}'.format(self.datafile_url, str(err))) + return + + # Leave datafile and config unchanged if it has not been modified. + if response.status_code == http.HTTPStatus.NOT_MODIFIED: + self.logger.debug('Not updating config as datafile has not updated since {}.'.format(self.last_modified)) + return + + self.set_last_modified(response) + self.set_config(response) + def fetch_datafile(self): """ Fetch datafile and set ProjectConfig. """ @@ -178,7 +185,7 @@ def fetch_datafile(self): request_headers[enums.HTTPHeaders.IF_MODIFIED_SINCE] = self.last_modified response = requests.get(self.datafile_url, headers=request_headers) - self.set_config(response) + self._handle_response(response) def _run(self): """ Triggered as part of the thread which fetches the datafile and sleeps until next update interval. """ From f238d6000bf2d7c544c8d63af384c35e50aff320 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Thu, 30 May 2019 21:50:00 -0700 Subject: [PATCH 08/14] Responding to feedback --- optimizely/config_manager.py | 378 +++++++++++++++++++---------------- tests/test_config_manager.py | 186 +++++++++++++++++ 2 files changed, 389 insertions(+), 175 deletions(-) create mode 100644 tests/test_config_manager.py diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index c140d0de1..f300762f5 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -11,195 +11,223 @@ # See the License for the specific language governing permissions and # limitations under the License. - -import http +import abc import requests import threading import time +from requests import codes as http_status_codes from requests import exceptions as requests_exceptions -from optimizely import exceptions -from optimizely import logger as optimizely_logger -from optimizely import project_config -from optimizely.error_handler import NoOpErrorHandler as noop_error_handler -from optimizely.helpers import enums +from . import exceptions as optimizely_exceptions +from . import logger as optimizely_logger +from . import project_config +from .error_handler import NoOpErrorHandler as noop_error_handler +from .helpers import enums + + +ABC = abc.ABCMeta('ABC', (object,), {'__slots__': ()}) -class BaseConfigManager(object): - """ Base class for Optimizely's config manager. """ +class BaseConfigManager(ABC): + """ Base class for Optimizely's config manager. """ - def get_config(self): - """ Get config for use by optimizely.Optimizely. - The config should be an instance of project_config.ProjectConfig.""" - raise NotImplementedError + def __init__(self, + logger=None, + error_handler=None, + **kwargs): + """ Initialize config manager. + + Args: + logger: Provides a logger instance. + error_handler: Provides a handle_error method to handle exceptions. + """ + self.logger = logger or optimizely_logger.adapt_logger(logger or optimizely_logger.NoOpLogger()) + self.error_handler = error_handler or noop_error_handler + + @abc.abstractmethod + def get_config(self): + """ Get config for use by optimizely.Optimizely. + The config should be an instance of project_config.ProjectConfig.""" + pass class StaticConfigManager(BaseConfigManager): - """ Config manager that returns ProjectConfig based on provided datafile. """ + """ Config manager that returns ProjectConfig based on provided datafile. """ + + def __init__(self, + datafile=None, + logger=None, + error_handler=None): + """ Initialize config manager. Datafile has to be provided to use. - def __init__(self, - datafile, - logger=None, - error_handler=None): - self.logger = logger or optimizely_logger.adapt_logger(logger or optimizely_logger.NoOpLogger()) - self.error_handler = error_handler or noop_error_handler - self._datafile = datafile - self._config = project_config.ProjectConfig(self._datafile, self.logger, self.error_handler) + Args: + datafile: JSON string representing the Optimizely project. + logger: Provides a logger instance. + error_handler: Provides a handle_error method to handle exceptions. + """ + super(StaticConfigManager, self).__init__(logger=logger, error_handler=error_handler) + self._config = None + if datafile: + self._config = project_config.ProjectConfig(datafile, self.logger, self.error_handler) - def get_config(self): - """ Returns instance of ProjectConfig. + def get_config(self): + """ Returns instance of ProjectConfig. - Returns: - ProjectConfig. - """ - return self._config + Returns: + ProjectConfig. + """ + return self._config class PollingConfigManager(BaseConfigManager): - """ Config manager that polls for the datafile and updated ProjectConfig based on an update interval. """ - - def __init__(self, - sdk_key=None, - update_interval=None, - url=None, - url_template=None, - logger=None, - error_handler=None): - """ Initialize config manager. One of sdk_key or url has to be set to be able to use. - - Args: - sdk_key: Optional string uniquely identifying the datafile. - update_interval: Optional floating point number representing time interval in seconds - at which to request datafile and set ProjectConfig. - 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. - logger: Provides a logger instance. - error_handler: Provides a handle_error method to handle exceptions. - """ - self.datafile_url = self.get_datafile_url(sdk_key, url, url_template or enums.ConfigManager.DATAFILE_URL_TEMPLATE) - self.update_interval = self.get_update_interval(update_interval) - self.last_modified = None - self._datafile = None - self._config = None - self._polling_thread = threading.Thread(target=self._run) - self.is_running = False - self.logger = logger or optimizely_logger.adapt_logger( - logger or optimizely_logger.SimpleLogger(min_level=enums.LogLevels.DEBUG)) - self.error_handler = error_handler or noop_error_handler - - @staticmethod - def get_datafile_url(sdk_key, url, url_template): - """ Helper method to determine URL from where to fetch the datafile. - - Args: - sdk_key: Key uniquely identifying the datafile. - url: String representing URL from which to fetch the datafile. - url_template: String representing template which is filled in with - SDK key to determine URL from which to fetch the datafile. - - Returns: - String representing URL to fetch datafile from. - """ - # Ensure that either is provided by the user. - if sdk_key is None and url is None: - raise exceptions.InvalidInputException('Must provide at least one of sdk_key or url.') - - # Return URL if one is provided or use template and SDK key to get it. - if url is None: - return url_template.format(sdk_key=sdk_key) - - return url - - def get_update_interval(self, update_interval): - """ Helper method to determine frequency at which datafile has to be polled and ProjectConfig updated. - - Args: - update_interval: Time in seconds optionally sent in by the user. - """ - update_interval = update_interval or enums.ConfigManager.DEFAULT_UPDATE_INTERVAL - - # If polling interval is less than minimum allowed interval then set it to default update interval. - if update_interval < enums.ConfigManager.MIN_UPDATE_INTERVAL: - self.logger.debug('Invalid update_interval {} provided. Defaulting to {}'.format( - update_interval, - enums.ConfigManager.DEFAULT_UPDATE_INTERVAL) - ) - update_interval = enums.ConfigManager.DEFAULT_UPDATE_INTERVAL - - return update_interval - - def set_last_modified(self, response): - """ Looks up and sets last modified time based on Last-Modified header in the response. - - Args: - response: requests.Response - """ - self.last_modified = response.headers.get(enums.HTTPHeaders.LAST_MODIFIED) - - def set_config(self, response): - """ Looks up and sets datafile and config based on response body. - - Args: - response: requests.Response - """ - # TODO(ali): Add validation here to make sure that we do not update datafile and config if not a valid datafile. - self._datafile = response.text - # TODO(ali): Add notification listener. - self._config = project_config.ProjectConfig(self._datafile, self.logger, self.error_handler) - self.logger.info('Received new datafile and updated config.') - - def get_config(self): - """ Returns instance of ProjectConfig. - - Returns: - ProjectConfig. - """ - return self._config - - def _handle_response(self, response): - """ Helper method to handle response containing datafile. - - Args: - response: requests.Response - """ - try: - response.raise_for_status() - except requests_exceptions.HTTPError as err: - self.logger.error('Fetching datafile from {} failed. Error: {}'.format(self.datafile_url, str(err))) - return - - # Leave datafile and config unchanged if it has not been modified. - if response.status_code == http.HTTPStatus.NOT_MODIFIED: - self.logger.debug('Not updating config as datafile has not updated since {}.'.format(self.last_modified)) - return - - self.set_last_modified(response) - self.set_config(response) - - def fetch_datafile(self): - """ Fetch datafile and set ProjectConfig. """ - - request_headers = {} - if self.last_modified: - request_headers[enums.HTTPHeaders.IF_MODIFIED_SINCE] = self.last_modified - - response = requests.get(self.datafile_url, headers=request_headers) - self._handle_response(response) - - def _run(self): - """ Triggered as part of the thread which fetches the datafile and sleeps until next update interval. """ - while self.is_running: - self.fetch_datafile() - time.sleep(self.update_interval) - - def start(self): - """ Start the config manager and the thread to periodically fetch datafile. """ - if not self.is_running: - self.is_running= True - self._polling_thread.start() - - def stop(self): - """ Stops the config manager. """ - if self.is_running: - self.is_running = False + """ Config manager that polls for the datafile and updated ProjectConfig based on an update interval. """ + + def __init__(self, + sdk_key=None, + datafile=None, + update_interval=None, + url=None, + url_template=None, + logger=None, + error_handler=None): + """ Initialize config manager. One of sdk_key or url has to be set to be able to use. + + Args: + sdk_key: Optional string uniquely identifying the datafile. + update_interval: Optional floating point number representing time interval in seconds + at which to request datafile and set ProjectConfig. + 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. + logger: Provides a logger instance. + error_handler: Provides a handle_error method to handle exceptions. + """ + super(PollingConfigManager, self).__init__(logger=logger, error_handler=error_handler) + 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.last_modified = None + self._datafile = datafile + self._config = None + self._polling_thread = threading.Thread(target=self._run) + self._polling_thread.setDaemon(True) + if self._datafile: + self.set_config(self._datafile) + + @staticmethod + def get_datafile_url(sdk_key, url, url_template): + """ Helper method to determine URL from where to fetch the datafile. + + Args: + sdk_key: Key uniquely identifying the datafile. + url: String representing URL from which to fetch the datafile. + url_template: String representing template which is filled in with + SDK key to determine URL from which to fetch the datafile. + + Returns: + String representing URL to fetch datafile from. + + Raises: + optimizely.exceptions.InvalidInputException if: + - One of sdk_key or url is not provided. + - url_template is invalid. + """ + # Ensure that either is provided by the user. + if sdk_key is None and url is None: + raise optimizely_exceptions.InvalidInputException('Must provide at least one of sdk_key or url.') + + # Return URL if one is provided or use template and SDK key to get it. + if url is None: + try: + return url_template.format(sdk_key=sdk_key) + except (AttributeError, KeyError): + raise optimizely_exceptions.InvalidInputException( + 'Invalid url_template {} provided.'.format(url_template)) + + return url + + def set_update_interval(self, update_interval): + """ Helper method to set frequency at which datafile has to be polled and ProjectConfig updated. """ + self.update_interval = update_interval or enums.ConfigManager.DEFAULT_UPDATE_INTERVAL + + # If polling interval is less than minimum allowed interval then set it to default update interval. + if self.update_interval < enums.ConfigManager.MIN_UPDATE_INTERVAL: + self.logger.debug('Invalid update_interval {} provided. Defaulting to {}'.format( + update_interval, + enums.ConfigManager.DEFAULT_UPDATE_INTERVAL) + ) + self.update_interval = enums.ConfigManager.DEFAULT_UPDATE_INTERVAL + + def set_last_modified(self, response_headers): + """ Looks up and sets last modified time based on Last-Modified header in the response. + + Args: + response_headers: requests.Response.headers + """ + self.last_modified = response_headers.get(enums.HTTPHeaders.LAST_MODIFIED) + + def set_config(self, datafile): + """ Looks up and sets datafile and config based on response body. + + Args: + datafile: JSON string representing the Optimizely project. + """ + # TODO(ali): Add validation here to make sure that we do not update datafile and config if not a valid datafile. + self._datafile = datafile + # TODO(ali): Add notification listener. + self._config = project_config.ProjectConfig(self._datafile, self.logger, self.error_handler) + self.logger.info('Received new datafile and updated config.') + + def get_config(self): + """ Returns instance of ProjectConfig. + + Returns: + ProjectConfig. + """ + return self._config + + def _handle_response(self, response): + """ Helper method to handle response containing datafile. + + Args: + response: requests.Response + """ + try: + response.raise_for_status() + except requests_exceptions.HTTPError as err: + self.logger.error('Fetching datafile from {} failed. Error: {}'.format(self.datafile_url, str(err))) + return + + # Leave datafile and config unchanged if it has not been modified. + if response.status_code == http_status_codes.not_modified: + self.logger.debug('Not updating config as datafile has not updated since {}.'.format(self.last_modified)) + return + + self.set_last_modified(response.headers) + self.set_config(response.text) + + def fetch_datafile(self): + """ Fetch datafile and set ProjectConfig. """ + + request_headers = {} + if self.last_modified: + request_headers[enums.HTTPHeaders.IF_MODIFIED_SINCE] = self.last_modified + + response = requests.get(self.datafile_url, headers=request_headers) + self._handle_response(response) + + @property + def is_running(self): + """ Check if polling thread is alive or not. """ + return self._polling_thread.is_alive() + + def _run(self): + """ Triggered as part of the thread which fetches the datafile and sleeps until next update interval. """ + while self.is_running: + self.fetch_datafile() + time.sleep(self.update_interval) + + def start(self): + """ Start the config manager and the thread to periodically fetch datafile. """ + if not self.is_running: + self._polling_thread.start() diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py new file mode 100644 index 000000000..0458ef9af --- /dev/null +++ b/tests/test_config_manager.py @@ -0,0 +1,186 @@ +# Copyright 2019, Optimizely +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 + +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import json +import mock +import requests +import unittest + +from optimizely import config_manager +from optimizely import exceptions as optimizely_exceptions +from optimizely import project_config +from optimizely.helpers import enums + + +class StaticConfigManagerTest(unittest.TestCase): + def test_get_config(self): + test_datafile = json.dumps({ + 'some_datafile_key': 'some_datafile_value', + 'version': project_config.SUPPORTED_VERSIONS[0] + }) + project_config_manager = config_manager.StaticConfigManager(datafile=test_datafile) + + # Assert that config is set. + self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig) + + +class PollingConfigManagerTest(unittest.TestCase): + def test_init__no_sdk_key_no_url__fails(self): + """ Test that initialization fails if there is no sdk_key or url provided. """ + self.assertRaisesRegexp(optimizely_exceptions.InvalidInputException, + 'Must provide at least one of sdk_key or url.', + config_manager.PollingConfigManager, sdk_key=None, url=None) + + def test_get_datafile_url__no_sdk_key_no_url_raises(self): + """ Test that get_datafile_url raises exception if no sdk_key or url is provided. """ + self.assertRaisesRegexp(optimizely_exceptions.InvalidInputException, + 'Must provide at least one of sdk_key or url.', + config_manager.PollingConfigManager.get_datafile_url, None, None, 'url_template') + + def test_get_datafile_url__invalid_url_template_raises(self): + """ Test that get_datafile_url raises if url_template is invalid. """ + # No url_template provided + self.assertRaisesRegexp(optimizely_exceptions.InvalidInputException, + 'Invalid url_template None provided', + config_manager.PollingConfigManager.get_datafile_url, 'optly_datafile_key', None, None) + + # Incorrect url_template provided + test_url_template = 'invalid_url_template_without_sdk_key_field_{key}' + self.assertRaisesRegexp(optimizely_exceptions.InvalidInputException, + 'Invalid url_template {} provided'.format(test_url_template), + config_manager.PollingConfigManager.get_datafile_url, + 'optly_datafile_key', None, test_url_template) + + def test_get_datafile_url__sdk_key_and_template_provided(self): + """ Test get_datafile_url when sdk_key and template are provided. """ + test_sdk_key = 'optly_key' + test_url_template = 'www.optimizelydatafiles.com/{sdk_key}.json' + expected_url = test_url_template.format(sdk_key=test_sdk_key) + self.assertEqual(expected_url, + config_manager.PollingConfigManager.get_datafile_url(test_sdk_key, None, test_url_template)) + + def test_get_datafile_url__url_and_template_provided(self): + """ Test get_datafile_url when url and url_template are provided. """ + test_url_template = 'www.optimizelydatafiles.com/{sdk_key}.json' + test_url = 'www.myoptimizelydatafiles.com/my_key.json' + self.assertEqual(test_url, config_manager.PollingConfigManager.get_datafile_url(None, + test_url, + test_url_template)) + + def test_get_datafile_url__sdk_key_and_url_and_template_provided(self): + """ Test get_datafile_url when sdk_key, url and url_template are provided. """ + test_sdk_key = 'optly_key' + test_url_template = 'www.optimizelydatafiles.com/{sdk_key}.json' + test_url = 'www.myoptimizelydatafiles.com/my_key.json' + + # Assert that if url is provided, it is always returned + self.assertEqual(test_url, config_manager.PollingConfigManager.get_datafile_url(test_sdk_key, + test_url, + test_url_template)) + + def test_set_update_interval(self): + project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') + + # Assert that update_interval cannot be set to less than allowed minimum and instead is set to default value. + project_config_manager.set_update_interval(0.42) + self.assertEqual(enums.ConfigManager.DEFAULT_UPDATE_INTERVAL, project_config_manager.update_interval) + + # Assert that if no update_interval is provided, it is set to default value. + project_config_manager.set_update_interval(None) + self.assertEqual(enums.ConfigManager.DEFAULT_UPDATE_INTERVAL, project_config_manager.update_interval) + + # Assert that if valid update_interval is provided, it is set to that value. + project_config_manager.set_update_interval(42) + self.assertEqual(42, project_config_manager.update_interval) + + 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') + self.assertIsNone(project_config_manager.last_modified) + + last_modified_time = 'Test Last Modified Time' + test_response_headers = { + 'Last-Modified': last_modified_time, + 'Some-Other-Important-Header': 'some_value' + } + project_config_manager.set_last_modified(test_response_headers) + self.assertEqual(last_modified_time, project_config_manager.last_modified) + + def test_set_and_get_config(self): + """ Test that set_last_modified sets config field based on datafile. """ + project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') + + # Assert that config is not set. + self.assertIsNone(project_config_manager.get_config()) + + # Set and check config. + project_config_manager.set_config(json.dumps({ + 'some_datafile_key': 'some_datafile_value', + 'version': project_config.SUPPORTED_VERSIONS[0] + })) + self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig) + + def test_fetch_datafile(self): + """ Test that fetch_datafile sets config and last_modified based on response. """ + project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') + expected_datafile_url = 'https://cdn.optimizely.com/datafiles/some_key.json' + test_headers = { + 'Last-Modified': 'New Time' + } + test_datafile = json.dumps({ + 'some_datafile_key': 'some_datafile_value', + 'version': project_config.SUPPORTED_VERSIONS[0] + }) + test_response = requests.Response() + test_response.status_code = 200 + test_response.headers = test_headers + test_response._content = test_datafile + with mock.patch('requests.get', return_value=test_response) as mock_requests: + project_config_manager.fetch_datafile() + + mock_requests.assert_called_once_with(expected_datafile_url, headers={}) + self.assertEqual(test_headers['Last-Modified'], project_config_manager.last_modified) + self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig) + + # Call fetch_datafile again and assert that request to URL is with If-Modified-Since header. + with mock.patch('requests.get', return_value=test_response) as mock_requests: + project_config_manager.fetch_datafile() + + mock_requests.assert_called_once_with(expected_datafile_url, + headers={'If-Modified-Since': test_headers['Last-Modified']}) + self.assertEqual(test_headers['Last-Modified'], project_config_manager.last_modified) + self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig) + + def test_is_running(self): + """ Test is_running before and after starting thread. """ + project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') + self.assertFalse(project_config_manager.is_running) + with mock.patch('optimizely.config_manager.PollingConfigManager.fetch_datafile') as mock_fetch_datafile, \ + mock.patch('time.sleep') as mock_sleep: + project_config_manager.start() + self.assertTrue(project_config_manager.is_running) + + mock_fetch_datafile.assert_called_with() + mock_sleep.assert_called_with(enums.ConfigManager.DEFAULT_UPDATE_INTERVAL) + + def test_start(self): + """ Test that calling start starts the polling thread. """ + project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') + self.assertFalse(project_config_manager._polling_thread.is_alive()) + with mock.patch('optimizely.config_manager.PollingConfigManager.fetch_datafile') as mock_fetch_datafile, \ + mock.patch('time.sleep') as mock_sleep: + project_config_manager.start() + self.assertTrue(project_config_manager._polling_thread.is_alive()) + + mock_fetch_datafile.assert_called_with() + mock_sleep.assert_called_with(enums.ConfigManager.DEFAULT_UPDATE_INTERVAL) From 67dc020d6b66c5e4b88f29de9c6cf628c445b8a5 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Fri, 31 May 2019 10:07:47 -0700 Subject: [PATCH 09/14] Fixing lint errors --- optimizely/config_manager.py | 2 +- tests/test_config_manager.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index f300762f5..5593d01fd 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -2,7 +2,7 @@ # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at -# +# # http://www.apache.org/licenses/LICENSE-2.0 # Unless required by applicable law or agreed to in writing, software diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index 0458ef9af..a65d1d1a4 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -2,7 +2,7 @@ # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at -# +# # http://www.apache.org/licenses/LICENSE-2.0 # Unless required by applicable law or agreed to in writing, software @@ -166,7 +166,7 @@ def test_is_running(self): project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') self.assertFalse(project_config_manager.is_running) with mock.patch('optimizely.config_manager.PollingConfigManager.fetch_datafile') as mock_fetch_datafile, \ - mock.patch('time.sleep') as mock_sleep: + mock.patch('time.sleep') as mock_sleep: project_config_manager.start() self.assertTrue(project_config_manager.is_running) @@ -178,7 +178,7 @@ def test_start(self): project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') self.assertFalse(project_config_manager._polling_thread.is_alive()) with mock.patch('optimizely.config_manager.PollingConfigManager.fetch_datafile') as mock_fetch_datafile, \ - mock.patch('time.sleep') as mock_sleep: + mock.patch('time.sleep') as mock_sleep: project_config_manager.start() self.assertTrue(project_config_manager._polling_thread.is_alive()) From 25b116a08d4dc757c184d88bc9b4b3c7d7152e59 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Mon, 3 Jun 2019 17:45:03 -0700 Subject: [PATCH 10/14] Responding to all feedback. --- optimizely/config_manager.py | 16 +++++++++++----- optimizely/helpers/enums.py | 2 ++ tests/test_config_manager.py | 7 +++++-- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 5593d01fd..75dd801a7 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -33,8 +33,7 @@ class BaseConfigManager(ABC): def __init__(self, logger=None, - error_handler=None, - **kwargs): + error_handler=None): """ Initialize config manager. Args: @@ -94,6 +93,7 @@ def __init__(self, Args: 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. url: Optional string representing URL from where to fetch the datafile. If set it supersedes the sdk_key. @@ -147,7 +147,11 @@ def get_datafile_url(sdk_key, url, url_template): return url def set_update_interval(self, update_interval): - """ Helper method to set frequency at which datafile has to be polled and ProjectConfig updated. """ + """ Helper method to set frequency at which datafile has to be polled and ProjectConfig updated. + + Args: + update_interval: Time in seconds after which to update datafile. + """ self.update_interval = update_interval or enums.ConfigManager.DEFAULT_UPDATE_INTERVAL # If polling interval is less than minimum allowed interval then set it to default update interval. @@ -176,7 +180,7 @@ def set_config(self, datafile): self._datafile = datafile # TODO(ali): Add notification listener. self._config = project_config.ProjectConfig(self._datafile, self.logger, self.error_handler) - self.logger.info('Received new datafile and updated config.') + self.logger.debug('Received new datafile and updated config.') def get_config(self): """ Returns instance of ProjectConfig. @@ -213,7 +217,9 @@ def fetch_datafile(self): if self.last_modified: request_headers[enums.HTTPHeaders.IF_MODIFIED_SINCE] = self.last_modified - response = requests.get(self.datafile_url, headers=request_headers) + response = requests.get(self.datafile_url, + headers=request_headers, + timeout=enums.ConfigManager.REQUEST_TIMEOUT) self._handle_response(response) @property diff --git a/optimizely/helpers/enums.py b/optimizely/helpers/enums.py index 2fb9516e5..f86f584e4 100644 --- a/optimizely/helpers/enums.py +++ b/optimizely/helpers/enums.py @@ -42,6 +42,8 @@ class ConfigManager(object): DEFAULT_UPDATE_INTERVAL = 5 * 60 # Minimum config update interval of 1 second MIN_UPDATE_INTERVAL = 1 + # Time in seconds before which request for datafile times out + REQUEST_TIMEOUT = 10 class ControlAttributes(object): diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index a65d1d1a4..d26a8c0cb 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -148,7 +148,9 @@ def test_fetch_datafile(self): with mock.patch('requests.get', return_value=test_response) as mock_requests: project_config_manager.fetch_datafile() - mock_requests.assert_called_once_with(expected_datafile_url, headers={}) + mock_requests.assert_called_once_with(expected_datafile_url, + headers={}, + timeout=enums.ConfigManager.REQUEST_TIMEOUT) self.assertEqual(test_headers['Last-Modified'], project_config_manager.last_modified) self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig) @@ -157,7 +159,8 @@ def test_fetch_datafile(self): project_config_manager.fetch_datafile() mock_requests.assert_called_once_with(expected_datafile_url, - headers={'If-Modified-Since': test_headers['Last-Modified']}) + headers={'If-Modified-Since': test_headers['Last-Modified']}, + timeout=enums.ConfigManager.REQUEST_TIMEOUT) self.assertEqual(test_headers['Last-Modified'], project_config_manager.last_modified) self.assertIsInstance(project_config_manager.get_config(), project_config.ProjectConfig) From 02503666ca3acef1c4da701c3efdbebf7647efe3 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 4 Jun 2019 13:04:36 -0700 Subject: [PATCH 11/14] Fixing for Python 3+ --- optimizely/config_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index 75dd801a7..ff1ef9465 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -208,7 +208,7 @@ def _handle_response(self, response): return self.set_last_modified(response.headers) - self.set_config(response.text) + self.set_config(str.encode(response.text)) def fetch_datafile(self): """ Fetch datafile and set ProjectConfig. """ From 7f7eddcda0444535101bbad54cc2fc990c71daf5 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 4 Jun 2019 15:27:17 -0700 Subject: [PATCH 12/14] Fixing encoding --- optimizely/config_manager.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index ff1ef9465..e2c9d0011 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -208,7 +208,7 @@ def _handle_response(self, response): return self.set_last_modified(response.headers) - self.set_config(str.encode(response.text)) + self.set_config(response.text) def fetch_datafile(self): """ Fetch datafile and set ProjectConfig. """ @@ -220,6 +220,7 @@ def fetch_datafile(self): response = requests.get(self.datafile_url, headers=request_headers, timeout=enums.ConfigManager.REQUEST_TIMEOUT) + response.encoding = 'utf-8' self._handle_response(response) @property From 0bcec94b77c11f727b65fdd51a06cc6487ea3f22 Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 4 Jun 2019 15:41:29 -0700 Subject: [PATCH 13/14] using response.content --- optimizely/config_manager.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/optimizely/config_manager.py b/optimizely/config_manager.py index e2c9d0011..1eacd279a 100644 --- a/optimizely/config_manager.py +++ b/optimizely/config_manager.py @@ -208,7 +208,7 @@ def _handle_response(self, response): return self.set_last_modified(response.headers) - self.set_config(response.text) + self.set_config(response.content) def fetch_datafile(self): """ Fetch datafile and set ProjectConfig. """ @@ -220,7 +220,6 @@ def fetch_datafile(self): response = requests.get(self.datafile_url, headers=request_headers, timeout=enums.ConfigManager.REQUEST_TIMEOUT) - response.encoding = 'utf-8' self._handle_response(response) @property From 0d2c6dfea5ed143a782166b71c77b2ed3687353c Mon Sep 17 00:00:00 2001 From: aliabbasrizvi Date: Tue, 4 Jun 2019 15:58:29 -0700 Subject: [PATCH 14/14] Trying to fix test --- tests/test_config_manager.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/tests/test_config_manager.py b/tests/test_config_manager.py index d26a8c0cb..0d86b055e 100644 --- a/tests/test_config_manager.py +++ b/tests/test_config_manager.py @@ -168,22 +168,18 @@ def test_is_running(self): """ Test is_running before and after starting thread. """ project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') self.assertFalse(project_config_manager.is_running) - with mock.patch('optimizely.config_manager.PollingConfigManager.fetch_datafile') as mock_fetch_datafile, \ - mock.patch('time.sleep') as mock_sleep: + with mock.patch('optimizely.config_manager.PollingConfigManager.fetch_datafile') as mock_fetch_datafile: project_config_manager.start() self.assertTrue(project_config_manager.is_running) mock_fetch_datafile.assert_called_with() - mock_sleep.assert_called_with(enums.ConfigManager.DEFAULT_UPDATE_INTERVAL) def test_start(self): """ Test that calling start starts the polling thread. """ project_config_manager = config_manager.PollingConfigManager(sdk_key='some_key') self.assertFalse(project_config_manager._polling_thread.is_alive()) - with mock.patch('optimizely.config_manager.PollingConfigManager.fetch_datafile') as mock_fetch_datafile, \ - mock.patch('time.sleep') as mock_sleep: + with mock.patch('optimizely.config_manager.PollingConfigManager.fetch_datafile') as mock_fetch_datafile: project_config_manager.start() self.assertTrue(project_config_manager._polling_thread.is_alive()) mock_fetch_datafile.assert_called_with() - mock_sleep.assert_called_with(enums.ConfigManager.DEFAULT_UPDATE_INTERVAL)