Skip to content

Commit

Permalink
addressing PR comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@pawels-optimizely
pawels-optimizely committed Apr 21, 2020
1 parent f4e3031 commit 540e2f5
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 5 deletions.
1 change: 1 addition & 0 deletions optimizely/helpers/enums.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ class DecisionNotificationTypes(object):
FEATURE = 'feature'
FEATURE_TEST = 'feature-test'
FEATURE_VARIABLE = 'feature-variable'
FEATURE_VARIABLES = 'feature-variables'


class DecisionSources(object):
Expand Down
5 changes: 3 additions & 2 deletions optimizely/optimizely.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ def _get_all_feature_variables_for_type(
variable_value = variable.defaultValue
if feature_enabled:
variable_value = project_config.get_variable_value_for_variation(variable, decision.variation)
self.logger.info(
self.logger.debug(
'Got variable value "%s" for variable "%s" of feature flag "%s".'
% (variable_value, variable_key, feature_key)
)
Expand All @@ -365,12 +365,13 @@ def _get_all_feature_variables_for_type(

self.notification_center.send_notifications(
enums.NotificationTypes.DECISION,
enums.DecisionNotificationTypes.FEATURE,
enums.DecisionNotificationTypes.FEATURE_VARIABLES,
user_id,
attributes or {},
{
'feature_key': feature_key,
'feature_enabled': feature_enabled,
'variable_values': all_variables,
'source': decision.source,
'source_info': source_info,
},
Expand Down
4 changes: 4 additions & 0 deletions optimizely/project_config.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,10 @@ def __init__(self, datafile, logger, error_handler):
)

self.feature_key_map = self._generate_key_map(self.feature_flags, 'key', entities.FeatureFlag)

# As we cannot create json variables in datafile directly, here we convert
# the variables of string type and json subType to json type
# This is needed to fully support json variables
for feature in self.feature_key_map:
for variable in self.feature_key_map[feature].variables:
sub_type = variable.get('subType', '')
Expand Down
3 changes: 3 additions & 0 deletions tests/base.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ def setUp(self, config_dict='config_dict'):
{'id': '129', 'value': '10.01'},
{'id': '130', 'value': '4242'},
{'id': '132', 'value': '{"test": 122}'},
{'id': '133', 'value': '{"true_test": 1.3}'},
],
},
{
Expand All @@ -165,6 +166,7 @@ def setUp(self, config_dict='config_dict'):
{'id': '129', 'value': '10.02'},
{'id': '130', 'value': '4243'},
{'id': '132', 'value': '{"test": 123}'},
{'id': '133', 'value': '{"true_test": 1.4}'},
],
},
],
Expand Down Expand Up @@ -330,6 +332,7 @@ def setUp(self, config_dict='config_dict'):
{'id': '131', 'key': 'variable_without_usage', 'defaultValue': '45', 'type': 'integer'},
{'id': '132', 'key': 'object', 'defaultValue': '{"test": 12}', 'type': 'string',
'subType': 'json'},
{'id': '133', 'key': 'true_object', 'defaultValue': '{"true_test": 23.54}', 'type': 'json'},
],
},
{
Expand Down
13 changes: 10 additions & 3 deletions tests/test_optimizely.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2569,6 +2569,7 @@ def test_get_all_feature_variables(self):
'environment': 'staging',
'is_working': True,
'object': {'test': 123},
'true_object': {'true_test': 1.4},
'variable_without_usage': 45}
with mock.patch(
'optimizely.decision_service.DecisionService.get_variation_for_feature',
Expand All @@ -2581,7 +2582,7 @@ def test_get_all_feature_variables(self):
opt_obj.get_all_feature_variables('test_feature_in_experiment', 'test_user'),
)

self.assertEqual(6, mock_config_logging.info.call_count)
self.assertEqual(7, mock_config_logging.info.call_count)

mock_config_logging.info.assert_has_calls(
[
Expand All @@ -2590,20 +2591,24 @@ def test_get_all_feature_variables(self):
mock.call('Variable "variable_without_usage" is not used in variation "variation". \
Assigning default value "45".'),
mock.call('Value for variable "object" for variation "variation" is "{"test": 123}".'),
mock.call('Value for variable "true_object" for variation "variation" is "{"true_test": 1.4}".'),
mock.call('Value for variable "environment" for variation "variation" is "staging".'),
mock.call('Value for variable "cost" for variation "variation" is "10.02".')
], any_order=True
)

mock_broadcast_decision.assert_called_once_with(
enums.NotificationTypes.DECISION,
'feature',
'feature-variables',
'test_user',
{},
{
'feature_key': 'test_feature_in_experiment',
'feature_enabled': True,
'source': 'feature-test',
'variable_values': {'count': 4243, 'is_working': True, 'true_object': {'true_test': 1.4},
'variable_without_usage': 45, 'object': {'test': 123}, 'environment': 'staging',
'cost': 10.02},
'source_info': {'experiment_key': 'test_experiment', 'variation_key': 'variation'},
},
)
Expand Down Expand Up @@ -3001,12 +3006,14 @@ def test_get_all_feature_variables_for_feature_in_rollout(self):
)
mock_broadcast_decision.assert_called_once_with(
enums.NotificationTypes.DECISION,
'feature',
'feature-variables',
'test_user',
{'test_attribute': 'test_value'},
{
'feature_key': 'test_feature_in_rollout',
'feature_enabled': True,
'variable_values': {'count': 399, 'message': 'Hello audience', 'object': {'field': 12},
'price': 39.99, 'is_running': True},
'source': 'rollout',
'source_info': {},
},
Expand Down
30 changes: 30 additions & 0 deletions tests/test_optimizely_config.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ def setUp(self):
'type': 'json',
'value': '{"test": 12}'
},
'true_object': {
'id': '133',
'key': 'true_object',
'type': 'json',
'value': '{"true_test": 23.54}'
},
'variable_without_usage': {
'key': 'variable_without_usage',
'type': 'integer',
Expand Down Expand Up @@ -126,6 +132,12 @@ def setUp(self):
'type': 'json',
'value': '{"test": 123}'
},
'true_object': {
'id': '133',
'key': 'true_object',
'type': 'json',
'value': '{"true_test": 1.4}'
},
'variable_without_usage': {
'key': 'variable_without_usage',
'type': 'integer',
Expand Down Expand Up @@ -219,6 +231,12 @@ def setUp(self):
'type': 'json',
'value': '{"test": 12}'
},
'true_object': {
'id': '133',
'key': 'true_object',
'type': 'json',
'value': '{"true_test": 23.54}'
},
'variable_without_usage': {
'key': 'variable_without_usage',
'type': 'integer',
Expand Down Expand Up @@ -261,6 +279,12 @@ def setUp(self):
'type': 'json',
'value': '{"test": 12}'
},
'true_object': {
'id': '133',
'key': 'true_object',
'type': 'json',
'value': '{"true_test": 23.54}'
},
'variable_without_usage': {
'key': 'variable_without_usage',
'type': 'integer',
Expand Down Expand Up @@ -304,6 +328,12 @@ def setUp(self):
'type': 'json',
'value': '{"test": 123}'
},
'true_object': {
'id': '133',
'key': 'true_object',
'type': 'json',
'value': '{"true_test": 1.4}'
},
'variable_without_usage': {
'key': 'variable_without_usage',
'type': 'integer',
Expand Down

0 comments on commit 540e2f5

Please sign in to comment.