Skip to content

fix(config-manager): Updated polling interval condition. #208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 12, 2019
Merged

fix(config-manager): Updated polling interval condition. #208

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7ad6909
fix(config-manager): Updates polling interval condition
rashidsp Sep 5, 2019
bdcaf64
fix: config_manager testcase.
mariamjamal94 Sep 6, 2019
a281165
Merge branch 'rashid/closeable_optimizely' into rashid/polling_interval
mariamjamal94 Sep 6, 2019
07c43f5
Merge branch 'rashid/closeable_optimizely' into rashid/polling_interval
mariamjamal94 Sep 6, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions optimizely/config_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,8 +246,8 @@ def set_update_interval(self, update_interval):
'Invalid update_interval "{}" provided.'.format(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:
# If polling interval is less than or equal to 0 then set it to default update interval.
if update_interval <= 0:
self.logger.debug('update_interval value {} too small. Defaulting to {}'.format(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update_interval value is not greater than zero, Defaulting to {} value. @aliabbasrizvi WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think current message is fine.

update_interval,
enums.ConfigManager.DEFAULT_UPDATE_INTERVAL)
Expand Down
2 changes: 0 additions & 2 deletions optimizely/helpers/enums.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ 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
# Time in seconds before which request for datafile times out
REQUEST_TIMEOUT = 10

Expand Down
2 changes: 1 addition & 1 deletion tests/test_config_manager.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ def test_set_update_interval(self, _):
project_config_manager.set_update_interval('invalid interval')

# 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)
project_config_manager.set_update_interval(-4.2)
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.
Expand Down