-
Notifications
You must be signed in to change notification settings - Fork 36
feat: get_optimizely_config API #226
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.
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
Changes from 15 commits
b46b95e
0f2cf5a
e2cccb8
4deaf60
6f2a79f
b7e25fb
7709dff
7a9cb08
15bcb76
720d350
ec4f84f
caad32d
1704ada
dcc7389
19bb7fd
2aa243c
a95c669
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,225 @@ | ||
# Copyright 2020, 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 copy | ||
|
||
from .project_config import ProjectConfig | ||
|
||
|
||
class OptimizelyConfig(object): | ||
def __init__(self, revision, experiments_map, features_map): | ||
self.revision = revision | ||
self.experiments_map = experiments_map | ||
self.features_map = features_map | ||
|
||
|
||
class OptimizelyExperiment(object): | ||
def __init__(self, id, key, variations_map): | ||
self.id = id | ||
self.key = key | ||
self.variations_map = variations_map | ||
|
||
|
||
class OptimizelyFeature(object): | ||
def __init__(self, id, key, experiments_map, variables_map): | ||
self.id = id | ||
self.key = key | ||
self.experiments_map = experiments_map | ||
self.variables_map = variables_map | ||
|
||
|
||
class OptimizelyVariation(object): | ||
def __init__(self, id, key, feature_enabled, variables_map): | ||
self.id = id | ||
self.key = key | ||
self.feature_enabled = feature_enabled | ||
self.variables_map = variables_map | ||
|
||
|
||
class OptimizelyVariable(object): | ||
def __init__(self, id, key, variable_type, value): | ||
self.id = id | ||
self.key = key | ||
self.type = variable_type | ||
self.value = value | ||
|
||
|
||
class OptimizelyConfigService(object): | ||
""" Class encapsulating methods to be used in creating instance of OptimizelyConfig. """ | ||
|
||
def __init__(self, project_config): | ||
""" | ||
Args: | ||
project_config ProjectConfig | ||
""" | ||
self.is_valid = True | ||
|
||
if not isinstance(project_config, ProjectConfig): | ||
self.is_valid = False | ||
return | ||
|
||
self.experiments = project_config.experiments | ||
self.feature_flags = project_config.feature_flags | ||
self.groups = project_config.groups | ||
self.revision = project_config.revision | ||
|
||
self._create_lookup_maps() | ||
|
||
def get_config(self): | ||
""" Gets instance of OptimizelyConfig | ||
|
||
Returns: | ||
Optimizely Config instance or None if OptimizelyConfigService is invalid. | ||
""" | ||
|
||
if not self.is_valid: | ||
return None | ||
|
||
experiments_key_map, experiments_id_map = self._get_experiments_maps() | ||
features_map = self._get_features_map(experiments_id_map) | ||
|
||
return OptimizelyConfig(self.revision, experiments_key_map, features_map) | ||
|
||
def _create_lookup_maps(self): | ||
""" Creates lookup maps to avoid redundant iteration of config objects. """ | ||
|
||
self.exp_id_to_feature_map = {} | ||
self.feature_key_variable_key_to_variable_map = {} | ||
self.feature_key_variable_id_to_variable_map = {} | ||
|
||
for feature in self.feature_flags: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. am unable to find the difference between this |
||
for experiment_id in feature['experimentIds']: | ||
self.exp_id_to_feature_map[experiment_id] = feature | ||
|
||
variables_key_map = {} | ||
variables_id_map = {} | ||
for variable in feature.get('variables', []): | ||
opt_variable = OptimizelyVariable( | ||
variable['id'], variable['key'], variable['type'], variable['defaultValue'] | ||
) | ||
variables_key_map[variable['key']] = opt_variable | ||
variables_id_map[variable['id']] = opt_variable | ||
|
||
self.feature_key_variable_key_to_variable_map[feature['key']] = variables_key_map | ||
self.feature_key_variable_id_to_variable_map[feature['key']] = variables_id_map | ||
|
||
def _get_variables_map(self, variation, experiment): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit. I personally prefer arranging this as |
||
""" Gets variables map for given variation and experiment. | ||
|
||
Args: | ||
variation dict | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is very vague. Dict consisting of what? |
||
experiment dict | ||
|
||
Returns: | ||
dict - Map of variable key to OptimizelyVariable for the given variation. | ||
""" | ||
feature_flag = self.exp_id_to_feature_map.get(experiment['id'], None) | ||
if feature_flag is None: | ||
return {} | ||
|
||
# set default variables for each variation | ||
variables_map = {} | ||
variables_map = copy.deepcopy(self.feature_key_variable_key_to_variable_map[feature_flag['key']]) | ||
|
||
# set variation specific variable value if any | ||
if variation.get('featureEnabled'): | ||
for variable in variation.get('variables', []): | ||
feature_variable = self.feature_key_variable_id_to_variable_map[feature_flag['key']][variable['id']] | ||
variables_map[feature_variable.key].value = variable['value'] | ||
|
||
return variables_map | ||
|
||
def _get_variations_map(self, experiment): | ||
""" Gets variation map for the given experiment. | ||
|
||
Args: | ||
experiment dict | ||
|
||
Returns: | ||
dict -- Map of variation key to OptimizelyVariation. | ||
""" | ||
variations_map = {} | ||
|
||
for variation in experiment.get('variations', []): | ||
variables_map = self._get_variables_map(variation, experiment) | ||
feature_enabled = variation.get('featureEnabled', None) | ||
|
||
optly_variation = OptimizelyVariation( | ||
variation['id'], variation['key'], feature_enabled, variables_map | ||
) | ||
|
||
variations_map[variation['key']] = optly_variation | ||
|
||
return variations_map | ||
|
||
def _get_all_experiments(self): | ||
""" Gets all experiments in the project config. | ||
|
||
Returns: | ||
list -- List of dicts of experiments. | ||
""" | ||
experiments = self.experiments | ||
|
||
for group in self.groups: | ||
experiments = experiments + group['experiments'] | ||
|
||
return experiments | ||
|
||
def _get_experiments_maps(self): | ||
""" Gets maps for all the experiments in the project config. | ||
|
||
Returns: | ||
dict, dict -- experiment key/id to OptimizelyExperiment maps. | ||
""" | ||
# Key map is required for the OptimizelyConfig response. | ||
experiments_key_map = {} | ||
# Id map comes in handy to figure out feature experiment. | ||
experiments_id_map = {} | ||
|
||
all_experiments = self._get_all_experiments() | ||
for exp in all_experiments: | ||
optly_exp = OptimizelyExperiment( | ||
exp['id'], exp['key'], self._get_variations_map(exp) | ||
) | ||
|
||
experiments_key_map[exp['key']] = optly_exp | ||
experiments_id_map[exp['id']] = optly_exp | ||
|
||
return experiments_key_map, experiments_id_map | ||
|
||
def _get_features_map(self, experiments_id_map): | ||
""" Gets features map for the project config. | ||
|
||
Args: | ||
experiments_id_map dict -- experiment id to OptimizelyExperiment map | ||
|
||
Returns: | ||
dict -- feaure key to OptimizelyFeature map | ||
""" | ||
features_map = {} | ||
|
||
for feature in self.feature_flags: | ||
exp_map = {} | ||
for experiment_id in feature.get('experimentIds', []): | ||
optly_exp = experiments_id_map[experiment_id] | ||
exp_map[optly_exp.key] = optly_exp | ||
|
||
variables_map = self.feature_key_variable_key_to_variable_map[feature['key']] | ||
|
||
optly_feature = OptimizelyFeature( | ||
feature['id'], feature['key'], exp_map, variables_map | ||
) | ||
|
||
features_map[feature['key']] = optly_feature | ||
|
||
return features_map |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
# Copyright 2016-2019, Optimizely | ||
# Copyright 2016-2020, 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 | ||
|
@@ -182,14 +182,12 @@ def setUp(self, config_dict='config_dict'): | |
{ | ||
'id': '122239', | ||
'key': 'control', | ||
'featureEnabled': True, | ||
'variables': [{'id': '155551', 'value': '42.42'}], | ||
'variables': [], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed these since this is not a feature experiment so shouldn't have these props. |
||
}, | ||
{ | ||
'id': '122240', | ||
'key': 'variation', | ||
'featureEnabled': True, | ||
'variables': [{'id': '155551', 'value': '13.37'}], | ||
'variables': [], | ||
}, | ||
], | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
# Copyright 2016-2019, Optimizely | ||
# Copyright 2016-2020, 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 | ||
|
@@ -23,6 +23,7 @@ | |
from optimizely import exceptions | ||
from optimizely import logger | ||
from optimizely import optimizely | ||
from optimizely import optimizely_config | ||
from optimizely import project_config | ||
from optimizely import version | ||
from optimizely.event.event_factory import EventFactory | ||
|
@@ -3911,6 +3912,39 @@ def test_get_feature_variable_returns__default_value__complex_audience_match(sel | |
self.assertEqual(10, opt_obj.get_feature_variable_integer('feat2_with_var', 'z', 'user1', {})) | ||
self.assertEqual(10, opt_obj.get_feature_variable('feat2_with_var', 'z', 'user1', {})) | ||
|
||
def test_get_optimizely_config__invalid_object(self): | ||
""" Test that get_optimizely_config logs error if Optimizely instance is invalid. """ | ||
|
||
class InvalidConfigManager(object): | ||
pass | ||
|
||
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict), config_manager=InvalidConfigManager()) | ||
|
||
with mock.patch.object(opt_obj, 'logger') as mock_client_logging: | ||
self.assertIsNone(opt_obj.get_optimizely_config()) | ||
|
||
mock_client_logging.error.assert_called_once_with( | ||
'Optimizely instance is not valid. Failing "get_optimizely_config".') | ||
|
||
def test_get_optimizely_config__invalid_config(self): | ||
""" Test that get_optimizely_config logs error if config is invalid. """ | ||
|
||
opt_obj = optimizely.Optimizely('invalid_datafile') | ||
|
||
with mock.patch.object(opt_obj, 'logger') as mock_client_logging: | ||
self.assertIsNone(opt_obj.get_optimizely_config()) | ||
|
||
mock_client_logging.error.assert_called_once_with( | ||
'Invalid config. Optimizely instance is not valid. ' 'Failing "get_optimizely_config".' | ||
) | ||
|
||
def test_get_optimizely_config_returns_instance_of_optimizely_config(self): | ||
""" Test that get_optimizely_config returns an instance of OptimizelyConfig. """ | ||
|
||
opt_obj = optimizely.Optimizely(json.dumps(self.config_dict_with_features)) | ||
opt_config = opt_obj.get_optimizely_config() | ||
self.assertIsInstance(opt_config, optimizely_config.OptimizelyConfig) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add tests for OptConfig contents validation as well? |
||
|
||
class OptimizelyWithExceptionTest(base.BaseTest): | ||
def setUp(self): | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need to check
instance
ofProjectConfig
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't expect to be used elsewhere. And in the main class, we validate project_config before using OptimizelyService. I can still validate if you so, but will have to keep a validity flag so that get_config returns nil and does not break.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
my suggestion is to validate but @aliabbasrizvi will you suggest for validation here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with @msohailhussain, we should validate here.