Skip to content

Commit

Permalink
Responding to all feedback.
Browse files Browse the repository at this point in the history
  • Loading branch information
aliabbasrizvi committed Jun 4, 2019
1 parent 67dc020 commit 25b116a
Show file tree
Hide file tree
Showing 3 changed files with 18 additions and 7 deletions.
16 changes: 11 additions & 5 deletions optimizely/config_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,7 @@ class BaseConfigManager(ABC):

def __init__(self,
logger=None,
error_handler=None,
**kwargs):
error_handler=None):
""" Initialize config manager.
Args:
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions optimizely/helpers/enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
7 changes: 5 additions & 2 deletions tests/test_config_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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)

Expand Down

0 comments on commit 25b116a

Please sign in to comment.