From 548e6a8a0c13347cfa981293a1ff0809bcb99e3d Mon Sep 17 00:00:00 2001 From: Jacob Brown <jacob.brown@optimizely.com> Date: Fri, 23 Jul 2021 15:08:40 -0400 Subject: [PATCH 01/13] Fix delivery rules to use experiment as key instead of experiments_map, which was invalid. --- optimizely/optimizely_config.py | 2 +- tests/test_optimizely_config.py | 194 +++++++++++++++++++++++++++++++- 2 files changed, 190 insertions(+), 6 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index a5bb7566..4b4b140f 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -436,7 +436,7 @@ def _get_delivery_rules(self, rollouts, rollout_id): audiences_map[optly_audience.id] = optly_audience.name # Get the experiments_map for that rollout - experiments = rollout.get('experiments_map') + experiments = rollout.get('experiments') if experiments: for experiment in experiments: optly_exp = OptimizelyExperiment( diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index b7cbbd7b..2c2b0c9e 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -616,7 +616,53 @@ def setUp(self): 'experiments_map': { }, - 'delivery_rules': [], + 'delivery_rules': [ + { + 'id': '211127', + 'key': '211127', + 'variations_map': { + '211129': { + 'id': '211129', + 'key': '211129', + 'feature_enabled': True, + 'variables_map': {} + }, + '211229': { + 'id': '211229', + 'key': '211229', + 'feature_enabled': False, + 'variables_map': {} + } + }, + 'audiences': '' + }, + { + 'id': '211137', + 'key': '211137', + 'variations_map': { + '211139': { + 'id': '211139', + 'key': '211139', + 'feature_enabled': True, + 'variables_map': {} + } + }, + 'audiences': '' + }, + { + 'id': '211147', + 'key': '211147', + 'variations_map': { + '211149': { + 'id': '211149', + 'key': '211149', + 'feature_enabled': True, + 'variables_map': {} + } + }, + 'audiences': '' + } + ], 'experiment_rules': [], 'id': '91112', 'key': 'test_feature_in_rollout' @@ -677,7 +723,7 @@ def setUp(self): }, 'test_feature_in_experiment_and_rollout': { 'variables_map': { - + }, 'experiments_map': { 'group_exp_2': { @@ -704,7 +750,53 @@ def setUp(self): 'audiences': '' } }, - 'delivery_rules': [], + 'delivery_rules': [ + { + 'id': '211127', + 'key': '211127', + 'variations_map': { + '211129': { + 'id': '211129', + 'key': '211129', + 'feature_enabled': True, + 'variables_map': {} + }, + '211229': { + 'id': '211229', + 'key': '211229', + 'feature_enabled': False, + 'variables_map': {} + } + }, + 'audiences': '' + }, + { + 'id': '211137', + 'key': '211137', + 'variations_map': { + '211139': { + 'id': '211139', + 'key': '211139', + 'feature_enabled': True, + 'variables_map': {} + } + }, + 'audiences': '' + }, + { + 'id': '211147', + 'key': '211147', + 'variations_map': { + '211149': { + 'id': '211149', + 'key': '211149', + 'feature_enabled': True, + 'variables_map': {} + } + }, + 'audiences': '' + } + ], 'experiment_rules': [ { 'id': '32223', @@ -780,7 +872,53 @@ def setUp(self): 'audiences': '"Test attribute users 3"' } }, - 'delivery_rules': [], + 'delivery_rules': [ + { + 'id': '211127', + 'key': '211127', + 'variations_map': { + '211129': { + 'id': '211129', + 'key': '211129', + 'feature_enabled': True, + 'variables_map': {} + }, + '211229': { + 'id': '211229', + 'key': '211229', + 'feature_enabled': False, + 'variables_map': {} + } + }, + 'audiences': '' + }, + { + 'id': '211137', + 'key': '211137', + 'variations_map': { + '211139': { + 'id': '211139', + 'key': '211139', + 'feature_enabled': True, + 'variables_map': {} + } + }, + 'audiences': '' + }, + { + 'id': '211147', + 'key': '211147', + 'variations_map': { + '211149': { + 'id': '211149', + 'key': '211149', + 'feature_enabled': True, + 'variables_map': {} + } + }, + 'audiences': '' + } + ], 'experiment_rules': [ { 'id': '42222', @@ -876,7 +1014,53 @@ def setUp(self): 'audiences': '"Test attribute users 3"' } }, - 'delivery_rules': [], + 'delivery_rules': [ + { + 'id': '211127', + 'key': '211127', + 'variations_map': { + '211129': { + 'id': '211129', + 'key': '211129', + 'feature_enabled': True, + 'variables_map': {} + }, + '211229': { + 'id': '211229', + 'key': '211229', + 'feature_enabled': False, + 'variables_map': {} + } + }, + 'audiences': '' + }, + { + 'id': '211137', + 'key': '211137', + 'variations_map': { + '211139': { + 'id': '211139', + 'key': '211139', + 'feature_enabled': True, + 'variables_map': {} + } + }, + 'audiences': '' + }, + { + 'id': '211147', + 'key': '211147', + 'variations_map': { + '211149': { + 'id': '211149', + 'key': '211149', + 'feature_enabled': True, + 'variables_map': {} + } + }, + 'audiences': '' + } + ], 'experiment_rules': [ { 'id': '111134', From 2d53fe1d5ed76184bd6c9560ec431b791e2d2485 Mon Sep 17 00:00:00 2001 From: Jacob Brown <jacob.brown@optimizely.com> Date: Fri, 23 Jul 2021 15:12:02 -0400 Subject: [PATCH 02/13] Ran linter for trailing spaces. --- tests/test_optimizely_config.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 2c2b0c9e..a97f8d10 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -723,7 +723,7 @@ def setUp(self): }, 'test_feature_in_experiment_and_rollout': { 'variables_map': { - + }, 'experiments_map': { 'group_exp_2': { @@ -793,7 +793,7 @@ def setUp(self): 'feature_enabled': True, 'variables_map': {} } - }, + }, 'audiences': '' } ], @@ -915,7 +915,7 @@ def setUp(self): 'feature_enabled': True, 'variables_map': {} } - }, + }, 'audiences': '' } ], @@ -1057,7 +1057,7 @@ def setUp(self): 'feature_enabled': True, 'variables_map': {} } - }, + }, 'audiences': '' } ], From fe3e5914eb320274d4eb9b5afb27215bba297d83 Mon Sep 17 00:00:00 2001 From: Jacob Brown <jacob.brown@optimizely.com> Date: Mon, 26 Jul 2021 11:26:03 -0400 Subject: [PATCH 03/13] Added testcase to confirm Delivery Rules use Optimizely Experiments. --- tests/test_optimizely_config.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index a97f8d10..ebed7150 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -1650,3 +1650,15 @@ def test_get_variations_from_experiments_map(self): self.assertEqual(variation.key, 'all_traffic_variation') else: self.assertEqual(variation.key, 'no_traffic_variation') + + def test_get_delivery_rules(self): + expected_features_map_dict = self.expected_config.get('features_map') + actual_features_map_dict = self.actual_config_dict.get('features_map') + actual_features_map = self.actual_config.features_map + + for optly_feature in actual_features_map.values(): + self.assertIsInstance(optly_feature, optimizely_config.OptimizelyFeature) + for delivery_rule in optly_feature.delivery_rules: + self.assertIsInstance(delivery_rule, optimizely_config.OptimizelyExperiment) + + self.assertEqual(expected_features_map_dict, actual_features_map_dict) From 3743771e2d40f182b6984491d591100197bc62d7 Mon Sep 17 00:00:00 2001 From: Jacob Brown <jacob.brown@optimizely.com> Date: Tue, 27 Jul 2021 09:13:34 -0400 Subject: [PATCH 04/13] Update feature_enabled to be False by default for variations_map as per Config Document. --- optimizely/optimizely_config.py | 2 +- tests/test_optimizely_config.py | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 4b4b140f..bf48c101 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -325,7 +325,7 @@ def _get_variations_map(self, experiment): for variation in experiment.get('variations', []): variables_map = self._get_variables_map(experiment, variation) - feature_enabled = variation.get('featureEnabled', None) + feature_enabled = variation.get('featureEnabled', False) optly_variation = OptimizelyVariation( variation['id'], variation['key'], feature_enabled, variables_map diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index ebed7150..ec18bd0c 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -1584,7 +1584,8 @@ def test_stringify_audience_conditions_all_cases(self): ["not", ["and", "1", "2"]], ["or", "1", "100000"], ["and", "and"], - ["and"] + ["and"], + ["and", ["or", "1", ["and", "2", "3"]], ["and", "11", ["or", "12", "3"]]] ] audiences_output = [ @@ -1601,7 +1602,8 @@ def test_stringify_audience_conditions_all_cases(self): 'NOT ("us" AND "female")', '"us" OR "100000"', '', - '' + '', + '("us" OR ("female" AND "adult")) AND ("fr" AND ("male" OR "adult"))' ] config_service = optimizely_config.OptimizelyConfigService(config) From e21d0e5177604bae1792dc0eb2c3b12ddb651033 Mon Sep 17 00:00:00 2001 From: Jacob Brown <jacob.brown@optimizely.com> Date: Tue, 27 Jul 2021 09:20:31 -0400 Subject: [PATCH 05/13] Update expected config to reflect False instead of None for feature_Enabled in variations_map --- tests/test_optimizely_config.py | 64 ++++++++++++++++----------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index ec18bd0c..08eabe33 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -59,7 +59,7 @@ def setUp(self): }, 'id': '122239', 'key': 'control', - 'feature_enabled': None + 'feature_enabled': False }, 'variation': { 'variables_map': { @@ -67,7 +67,7 @@ def setUp(self): }, 'id': '122240', 'key': 'variation', - 'feature_enabled': None + 'feature_enabled': False } }, 'id': '111133', @@ -187,7 +187,7 @@ def setUp(self): }, 'id': '28902', 'key': 'group_exp_1_variation', - 'feature_enabled': None + 'feature_enabled': False }, 'group_exp_1_control': { 'variables_map': { @@ -195,7 +195,7 @@ def setUp(self): }, 'id': '28901', 'key': 'group_exp_1_control', - 'feature_enabled': None + 'feature_enabled': False } }, 'id': '32222', @@ -210,7 +210,7 @@ def setUp(self): }, 'id': '28906', 'key': 'group_exp_2_variation', - 'feature_enabled': None + 'feature_enabled': False }, 'group_exp_2_control': { 'variables_map': { @@ -218,7 +218,7 @@ def setUp(self): }, 'id': '28905', 'key': 'group_exp_2_control', - 'feature_enabled': None + 'feature_enabled': False } }, 'id': '32223', @@ -233,7 +233,7 @@ def setUp(self): }, 'id': '38901', 'key': 'var_1', - 'feature_enabled': None + 'feature_enabled': False }, }, 'id': '42222', @@ -248,7 +248,7 @@ def setUp(self): }, 'id': '38905', 'key': 'var_1', - 'feature_enabled': None + 'feature_enabled': False }, }, 'id': '42223', @@ -263,7 +263,7 @@ def setUp(self): }, 'id': '38906', 'key': 'var_1', - 'feature_enabled': None + 'feature_enabled': False }, }, 'id': '42224', @@ -278,7 +278,7 @@ def setUp(self): }, 'id': '222239', 'key': 'control', - 'feature_enabled': None + 'feature_enabled': False }, }, 'id': '111134', @@ -293,7 +293,7 @@ def setUp(self): }, 'id': '222240', 'key': 'control', - 'feature_enabled': None + 'feature_enabled': False }, }, 'id': '111135', @@ -308,7 +308,7 @@ def setUp(self): }, 'id': '222241', 'key': 'control', - 'feature_enabled': None + 'feature_enabled': False }, }, 'id': '111136', @@ -680,7 +680,7 @@ def setUp(self): }, 'id': '28902', 'key': 'group_exp_1_variation', - 'feature_enabled': None + 'feature_enabled': False }, 'group_exp_1_control': { 'variables_map': { @@ -688,7 +688,7 @@ def setUp(self): }, 'id': '28901', 'key': 'group_exp_1_control', - 'feature_enabled': None + 'feature_enabled': False } }, 'id': '32222', @@ -705,13 +705,13 @@ def setUp(self): 'group_exp_1_control': { 'id': '28901', 'key': 'group_exp_1_control', - 'feature_enabled': None, + 'feature_enabled': False, 'variables_map': {} }, 'group_exp_1_variation': { 'id': '28902', 'key': 'group_exp_1_variation', - 'feature_enabled': None, + 'feature_enabled': False, 'variables_map': {} } }, @@ -734,7 +734,7 @@ def setUp(self): }, 'id': '28906', 'key': 'group_exp_2_variation', - 'feature_enabled': None + 'feature_enabled': False }, 'group_exp_2_control': { 'variables_map': { @@ -742,7 +742,7 @@ def setUp(self): }, 'id': '28905', 'key': 'group_exp_2_control', - 'feature_enabled': None + 'feature_enabled': False } }, 'id': '32223', @@ -805,13 +805,13 @@ def setUp(self): 'group_exp_2_control': { 'id': '28905', 'key': 'group_exp_2_control', - 'feature_enabled': None, + 'feature_enabled': False, 'variables_map': {} }, 'group_exp_2_variation': { 'id': '28906', 'key': 'group_exp_2_variation', - 'feature_enabled': None, + 'feature_enabled': False, 'variables_map': {} } }, @@ -834,7 +834,7 @@ def setUp(self): }, 'id': '38901', 'key': 'var_1', - 'feature_enabled': None + 'feature_enabled': False }, }, 'id': '42222', @@ -849,7 +849,7 @@ def setUp(self): }, 'id': '38905', 'key': 'var_1', - 'feature_enabled': None + 'feature_enabled': False }, }, 'id': '42223', @@ -864,7 +864,7 @@ def setUp(self): }, 'id': '38906', 'key': 'var_1', - 'feature_enabled': None + 'feature_enabled': False }, }, 'id': '42224', @@ -927,7 +927,7 @@ def setUp(self): 'var_1': { 'id': '38901', 'key': 'var_1', - 'feature_enabled': None, + 'feature_enabled': False, 'variables_map': {} } }, @@ -940,7 +940,7 @@ def setUp(self): 'var_1': { 'id': '38905', 'key': 'var_1', - 'feature_enabled': None, + 'feature_enabled': False, 'variables_map': {} } }, @@ -953,7 +953,7 @@ def setUp(self): 'var_1': { 'id': '38906', 'key': 'var_1', - 'feature_enabled': None, + 'feature_enabled': False, 'variables_map': {} } }, @@ -976,7 +976,7 @@ def setUp(self): }, 'id': '222239', 'key': 'control', - 'feature_enabled': None + 'feature_enabled': False }, }, 'id': '111134', @@ -991,7 +991,7 @@ def setUp(self): }, 'id': '222240', 'key': 'control', - 'feature_enabled': None + 'feature_enabled': False }, }, 'id': '111135', @@ -1006,7 +1006,7 @@ def setUp(self): }, 'id': '222241', 'key': 'control', - 'feature_enabled': None + 'feature_enabled': False }, }, 'id': '111136', @@ -1069,7 +1069,7 @@ def setUp(self): 'control': { 'id': '222239', 'key': 'control', - 'feature_enabled': None, + 'feature_enabled': False, 'variables_map': {} } }, @@ -1082,7 +1082,7 @@ def setUp(self): 'control': { 'id': '222240', 'key': 'control', - 'feature_enabled': None, + 'feature_enabled': False, 'variables_map': {} } }, @@ -1095,7 +1095,7 @@ def setUp(self): 'control': { 'id': '222241', 'key': 'control', - 'feature_enabled': None, + 'feature_enabled': False, 'variables_map': {} } }, From 792607b5b32d6aab65b9d47b5ca9321d4e039b64 Mon Sep 17 00:00:00 2001 From: Jacob Brown <jacob.brown@optimizely.com> Date: Tue, 27 Jul 2021 09:29:39 -0400 Subject: [PATCH 06/13] Revert back to None to test FSC results --- optimizely/optimizely_config.py | 2 +- tests/test_optimizely_config.py | 64 ++++++++++++++++----------------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index bf48c101..4b4b140f 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -325,7 +325,7 @@ def _get_variations_map(self, experiment): for variation in experiment.get('variations', []): variables_map = self._get_variables_map(experiment, variation) - feature_enabled = variation.get('featureEnabled', False) + feature_enabled = variation.get('featureEnabled', None) optly_variation = OptimizelyVariation( variation['id'], variation['key'], feature_enabled, variables_map diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index 08eabe33..ec18bd0c 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -59,7 +59,7 @@ def setUp(self): }, 'id': '122239', 'key': 'control', - 'feature_enabled': False + 'feature_enabled': None }, 'variation': { 'variables_map': { @@ -67,7 +67,7 @@ def setUp(self): }, 'id': '122240', 'key': 'variation', - 'feature_enabled': False + 'feature_enabled': None } }, 'id': '111133', @@ -187,7 +187,7 @@ def setUp(self): }, 'id': '28902', 'key': 'group_exp_1_variation', - 'feature_enabled': False + 'feature_enabled': None }, 'group_exp_1_control': { 'variables_map': { @@ -195,7 +195,7 @@ def setUp(self): }, 'id': '28901', 'key': 'group_exp_1_control', - 'feature_enabled': False + 'feature_enabled': None } }, 'id': '32222', @@ -210,7 +210,7 @@ def setUp(self): }, 'id': '28906', 'key': 'group_exp_2_variation', - 'feature_enabled': False + 'feature_enabled': None }, 'group_exp_2_control': { 'variables_map': { @@ -218,7 +218,7 @@ def setUp(self): }, 'id': '28905', 'key': 'group_exp_2_control', - 'feature_enabled': False + 'feature_enabled': None } }, 'id': '32223', @@ -233,7 +233,7 @@ def setUp(self): }, 'id': '38901', 'key': 'var_1', - 'feature_enabled': False + 'feature_enabled': None }, }, 'id': '42222', @@ -248,7 +248,7 @@ def setUp(self): }, 'id': '38905', 'key': 'var_1', - 'feature_enabled': False + 'feature_enabled': None }, }, 'id': '42223', @@ -263,7 +263,7 @@ def setUp(self): }, 'id': '38906', 'key': 'var_1', - 'feature_enabled': False + 'feature_enabled': None }, }, 'id': '42224', @@ -278,7 +278,7 @@ def setUp(self): }, 'id': '222239', 'key': 'control', - 'feature_enabled': False + 'feature_enabled': None }, }, 'id': '111134', @@ -293,7 +293,7 @@ def setUp(self): }, 'id': '222240', 'key': 'control', - 'feature_enabled': False + 'feature_enabled': None }, }, 'id': '111135', @@ -308,7 +308,7 @@ def setUp(self): }, 'id': '222241', 'key': 'control', - 'feature_enabled': False + 'feature_enabled': None }, }, 'id': '111136', @@ -680,7 +680,7 @@ def setUp(self): }, 'id': '28902', 'key': 'group_exp_1_variation', - 'feature_enabled': False + 'feature_enabled': None }, 'group_exp_1_control': { 'variables_map': { @@ -688,7 +688,7 @@ def setUp(self): }, 'id': '28901', 'key': 'group_exp_1_control', - 'feature_enabled': False + 'feature_enabled': None } }, 'id': '32222', @@ -705,13 +705,13 @@ def setUp(self): 'group_exp_1_control': { 'id': '28901', 'key': 'group_exp_1_control', - 'feature_enabled': False, + 'feature_enabled': None, 'variables_map': {} }, 'group_exp_1_variation': { 'id': '28902', 'key': 'group_exp_1_variation', - 'feature_enabled': False, + 'feature_enabled': None, 'variables_map': {} } }, @@ -734,7 +734,7 @@ def setUp(self): }, 'id': '28906', 'key': 'group_exp_2_variation', - 'feature_enabled': False + 'feature_enabled': None }, 'group_exp_2_control': { 'variables_map': { @@ -742,7 +742,7 @@ def setUp(self): }, 'id': '28905', 'key': 'group_exp_2_control', - 'feature_enabled': False + 'feature_enabled': None } }, 'id': '32223', @@ -805,13 +805,13 @@ def setUp(self): 'group_exp_2_control': { 'id': '28905', 'key': 'group_exp_2_control', - 'feature_enabled': False, + 'feature_enabled': None, 'variables_map': {} }, 'group_exp_2_variation': { 'id': '28906', 'key': 'group_exp_2_variation', - 'feature_enabled': False, + 'feature_enabled': None, 'variables_map': {} } }, @@ -834,7 +834,7 @@ def setUp(self): }, 'id': '38901', 'key': 'var_1', - 'feature_enabled': False + 'feature_enabled': None }, }, 'id': '42222', @@ -849,7 +849,7 @@ def setUp(self): }, 'id': '38905', 'key': 'var_1', - 'feature_enabled': False + 'feature_enabled': None }, }, 'id': '42223', @@ -864,7 +864,7 @@ def setUp(self): }, 'id': '38906', 'key': 'var_1', - 'feature_enabled': False + 'feature_enabled': None }, }, 'id': '42224', @@ -927,7 +927,7 @@ def setUp(self): 'var_1': { 'id': '38901', 'key': 'var_1', - 'feature_enabled': False, + 'feature_enabled': None, 'variables_map': {} } }, @@ -940,7 +940,7 @@ def setUp(self): 'var_1': { 'id': '38905', 'key': 'var_1', - 'feature_enabled': False, + 'feature_enabled': None, 'variables_map': {} } }, @@ -953,7 +953,7 @@ def setUp(self): 'var_1': { 'id': '38906', 'key': 'var_1', - 'feature_enabled': False, + 'feature_enabled': None, 'variables_map': {} } }, @@ -976,7 +976,7 @@ def setUp(self): }, 'id': '222239', 'key': 'control', - 'feature_enabled': False + 'feature_enabled': None }, }, 'id': '111134', @@ -991,7 +991,7 @@ def setUp(self): }, 'id': '222240', 'key': 'control', - 'feature_enabled': False + 'feature_enabled': None }, }, 'id': '111135', @@ -1006,7 +1006,7 @@ def setUp(self): }, 'id': '222241', 'key': 'control', - 'feature_enabled': False + 'feature_enabled': None }, }, 'id': '111136', @@ -1069,7 +1069,7 @@ def setUp(self): 'control': { 'id': '222239', 'key': 'control', - 'feature_enabled': False, + 'feature_enabled': None, 'variables_map': {} } }, @@ -1082,7 +1082,7 @@ def setUp(self): 'control': { 'id': '222240', 'key': 'control', - 'feature_enabled': False, + 'feature_enabled': None, 'variables_map': {} } }, @@ -1095,7 +1095,7 @@ def setUp(self): 'control': { 'id': '222241', 'key': 'control', - 'feature_enabled': False, + 'feature_enabled': None, 'variables_map': {} } }, From 444f5e402fe78b1a69dac626f4066b0c03f847b7 Mon Sep 17 00:00:00 2001 From: Jacob Brown <jacob.brown@optimizely.com> Date: Wed, 4 Aug 2021 15:09:26 -0400 Subject: [PATCH 07/13] Update sdk_key and environment_key to default to empty strings, update delivery_rules to use feature_id to get variation variables. --- optimizely/optimizely_config.py | 42 ++++++---- tests/test_optimizely_config.py | 136 ++++++++++++++++++++++++++++++-- 2 files changed, 155 insertions(+), 23 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 4b4b140f..eb6629e0 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -19,14 +19,14 @@ class OptimizelyConfig(object): def __init__(self, revision, experiments_map, features_map, datafile=None, - sdk_key=None, environment_key=None, attributes=None, events=None, + sdk_key="", environment_key="", attributes=None, events=None, audiences=None): self.revision = revision self.experiments_map = experiments_map self.features_map = features_map self._datafile = datafile - self.sdk_key = sdk_key - self.environment_key = environment_key + self.sdk_key = sdk_key or "" + self.environment_key = environment_key or "" self.attributes = attributes or [] self.events = events or [] self.audiences = audiences or [] @@ -269,6 +269,8 @@ def _create_lookup_maps(self): self.exp_id_to_feature_map = {} self.feature_key_variable_key_to_variable_map = {} self.feature_key_variable_id_to_variable_map = {} + self.feature_id_variable_id_to_feature_variables_map = {} + self.feature_id_variable_key_to_feature_variables_map = {} for feature in self.feature_flags: for experiment_id in feature['experimentIds']: @@ -283,10 +285,12 @@ def _create_lookup_maps(self): variables_key_map[variable['key']] = opt_variable variables_id_map[variable['id']] = opt_variable + self.feature_id_variable_id_to_feature_variables_map[feature['id']] = variables_id_map + self.feature_id_variable_key_to_feature_variables_map[feature['id']] = variables_key_map 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, experiment, variation): + def _get_variables_map(self, experiment, variation, feature_id=None): """ Gets variables map for given experiment and variation. Args: @@ -296,23 +300,27 @@ def _get_variables_map(self, experiment, variation): Returns: dict - Map of variable key to OptimizelyVariable for the given variation. """ + variables_map = {} + feature_flag = self.exp_id_to_feature_map.get(experiment['id'], None) - if feature_flag is None: + if feature_flag is None and feature_id 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']]) + if feature_id: + variables_map = copy.deepcopy(self.feature_id_variable_key_to_feature_variables_map[feature_id]) + else: + 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'] + # 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): + def _get_variations_map(self, experiment, feature_id=None): """ Gets variation map for the given experiment. Args: @@ -324,7 +332,7 @@ def _get_variations_map(self, experiment): variations_map = {} for variation in experiment.get('variations', []): - variables_map = self._get_variables_map(experiment, variation) + variables_map = self._get_variables_map(experiment, variation, feature_id) feature_enabled = variation.get('featureEnabled', None) optly_variation = OptimizelyVariation( @@ -394,7 +402,7 @@ def _get_features_map(self, experiments_id_map): for feature in self.feature_flags: - delivery_rules = self._get_delivery_rules(self.rollouts, feature.get('rolloutId')) + delivery_rules = self._get_delivery_rules(self.rollouts, feature.get('rolloutId'), feature['id']) experiment_rules = [] exp_map = {} @@ -415,7 +423,7 @@ def _get_features_map(self, experiments_id_map): return features_map - def _get_delivery_rules(self, rollouts, rollout_id): + def _get_delivery_rules(self, rollouts, rollout_id, feature_id): """ Gets an array of rollouts for the project config returns: @@ -440,7 +448,7 @@ def _get_delivery_rules(self, rollouts, rollout_id): if experiments: for experiment in experiments: optly_exp = OptimizelyExperiment( - experiment['id'], experiment['key'], self._get_variations_map(experiment) + experiment['id'], experiment['key'], self._get_variations_map(experiment, feature_id) ) audiences = self.replace_ids_with_names(experiment.get('audienceConditions', []), audiences_map) optly_exp.audiences = audiences diff --git a/tests/test_optimizely_config.py b/tests/test_optimizely_config.py index ec18bd0c..c37a8434 100644 --- a/tests/test_optimizely_config.py +++ b/tests/test_optimizely_config.py @@ -26,8 +26,8 @@ def setUp(self): self.opt_config_service = optimizely_config.OptimizelyConfigService(self.project_config) self.expected_config = { - 'sdk_key': None, - 'environment_key': None, + 'sdk_key': '', + 'environment_key': '', 'attributes': [{'key': 'test_attribute', 'id': '111094'}], 'events': [{'key': 'test_event', 'experiment_ids': ['111127'], 'id': '111095'}], 'audiences': [ @@ -625,13 +625,75 @@ def setUp(self): 'id': '211129', 'key': '211129', 'feature_enabled': True, - 'variables_map': {} + 'variables_map': { + 'is_running': { + 'id': '132', + 'key': 'is_running', + 'type': 'boolean', + 'value': 'false' + }, + 'message': { + 'id': '133', + 'key': 'message', + 'type': 'string', + 'value': 'Hello' + }, + 'price': { + 'id': '134', + 'key': 'price', + 'type': 'double', + 'value': '99.99' + }, + 'count': { + 'id': '135', + 'key': 'count', + 'type': 'integer', + 'value': '999' + }, + 'object': { + 'id': '136', + 'key': 'object', + 'type': 'json', + 'value': '{"field": 1}' + } + } }, '211229': { 'id': '211229', 'key': '211229', 'feature_enabled': False, - 'variables_map': {} + 'variables_map': { + 'is_running': { + 'id': '132', + 'key': 'is_running', + 'type': 'boolean', + 'value': 'false' + }, + 'message': { + 'id': '133', + 'key': 'message', + 'type': 'string', + 'value': 'Hello' + }, + 'price': { + 'id': '134', + 'key': 'price', + 'type': 'double', + 'value': '99.99' + }, + 'count': { + 'id': '135', + 'key': 'count', + 'type': 'integer', + 'value': '999' + }, + 'object': { + 'id': '136', + 'key': 'object', + 'type': 'json', + 'value': '{"field": 1}' + } + } } }, 'audiences': '' @@ -644,7 +706,38 @@ def setUp(self): 'id': '211139', 'key': '211139', 'feature_enabled': True, - 'variables_map': {} + 'variables_map': { + 'is_running': { + 'id': '132', + 'key': 'is_running', + 'type': 'boolean', + 'value': 'false' + }, + 'message': { + 'id': '133', + 'key': 'message', + 'type': 'string', + 'value': 'Hello' + }, + 'price': { + 'id': '134', + 'key': 'price', + 'type': 'double', + 'value': '99.99' + }, + 'count': { + 'id': '135', + 'key': 'count', + 'type': 'integer', + 'value': '999' + }, + 'object': { + 'id': '136', + 'key': 'object', + 'type': 'json', + 'value': '{"field": 1}' + } + } } }, 'audiences': '' @@ -657,7 +750,38 @@ def setUp(self): 'id': '211149', 'key': '211149', 'feature_enabled': True, - 'variables_map': {} + 'variables_map': { + 'is_running': { + 'id': '132', + 'key': 'is_running', + 'type': 'boolean', + 'value': 'false' + }, + 'message': { + 'id': '133', + 'key': 'message', + 'type': 'string', + 'value': 'Hello' + }, + 'price': { + 'id': '134', + 'key': 'price', + 'type': 'double', + 'value': '99.99' + }, + 'count': { + 'id': '135', + 'key': 'count', + 'type': 'integer', + 'value': '999' + }, + 'object': { + 'id': '136', + 'key': 'object', + 'type': 'json', + 'value': '{"field": 1}' + } + } } }, 'audiences': '' From 90c01ea531bee5885221357be995897575e1ed32 Mon Sep 17 00:00:00 2001 From: Jacob Brown <jacob.brown@optimizely.com> Date: Wed, 4 Aug 2021 15:11:48 -0400 Subject: [PATCH 08/13] Set constructor back to None for sdk_key and environment_key --- optimizely/optimizely_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index eb6629e0..4ffa46d4 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -19,7 +19,7 @@ class OptimizelyConfig(object): def __init__(self, revision, experiments_map, features_map, datafile=None, - sdk_key="", environment_key="", attributes=None, events=None, + sdk_key=None, environment_key=None, attributes=None, events=None, audiences=None): self.revision = revision self.experiments_map = experiments_map From 9736f9e583260bd56521ef022c495a9a7b984869 Mon Sep 17 00:00:00 2001 From: Jacob Brown <jacob.brown@optimizely.com> Date: Wed, 4 Aug 2021 15:13:29 -0400 Subject: [PATCH 09/13] Update double quotes to single quotes to match rest of code. --- optimizely/optimizely_config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 4ffa46d4..d4690d87 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -25,8 +25,8 @@ def __init__(self, revision, experiments_map, features_map, datafile=None, self.experiments_map = experiments_map self.features_map = features_map self._datafile = datafile - self.sdk_key = sdk_key or "" - self.environment_key = environment_key or "" + self.sdk_key = sdk_key or '' + self.environment_key = environment_key or '' self.attributes = attributes or [] self.events = events or [] self.audiences = audiences or [] From 9c7a1f82e5e3c414e5381ebbfa65d528ad977409 Mon Sep 17 00:00:00 2001 From: Jacob Brown <jacob.brown@optimizely.com> Date: Wed, 4 Aug 2021 15:15:12 -0400 Subject: [PATCH 10/13] Update comment to match variable. --- optimizely/optimizely_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index d4690d87..d2330f28 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -443,7 +443,7 @@ def _get_delivery_rules(self, rollouts, rollout_id, feature_id): for optly_audience in self.audiences: audiences_map[optly_audience.id] = optly_audience.name - # Get the experiments_map for that rollout + # Get the experiments for that rollout experiments = rollout.get('experiments') if experiments: for experiment in experiments: From 806c68d81d44eb17d708e3e95bdcb560058e35a2 Mon Sep 17 00:00:00 2001 From: Jacob Brown <jacob.brown@optimizely.com> Date: Wed, 4 Aug 2021 15:37:10 -0400 Subject: [PATCH 11/13] Convert audience conditions to string to match config design doc. --- optimizely/optimizely_config.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index d2330f28..190f6988 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -133,7 +133,7 @@ def __init__(self, project_config): optly_audience = OptimizelyAudience( typed_audience.get('id'), typed_audience.get('name'), - typed_audience.get('conditions') + str(typed_audience.get('conditions')) ) optly_typed_audiences.append(optly_audience) id_lookup_dict[typed_audience.get('id')] = typed_audience.get('id') @@ -145,7 +145,7 @@ def __init__(self, project_config): optly_audience = OptimizelyAudience( old_audience.get('id'), old_audience.get('name'), - old_audience.get('conditions') + str(old_audience.get('conditions')) ) optly_typed_audiences.append(optly_audience) From f8947f8f970aaa1c2a6236e8243217f54bbb75c0 Mon Sep 17 00:00:00 2001 From: Jacob Brown <jacob.brown@optimizely.com> Date: Thu, 5 Aug 2021 13:13:57 -0400 Subject: [PATCH 12/13] Switch to reference for typed audiences rather than copying items as was unnecessary. Remove str cast as also not needed. --- optimizely/optimizely_config.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 190f6988..2f95b3a5 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -126,14 +126,13 @@ def __init__(self, project_config): The typed_audiences has higher precedence. ''' - typed_audiences = project_config.typed_audiences[:] optly_typed_audiences = [] id_lookup_dict = {} - for typed_audience in typed_audiences: + for typed_audience in project_config.typed_audiences: optly_audience = OptimizelyAudience( typed_audience.get('id'), typed_audience.get('name'), - str(typed_audience.get('conditions')) + typed_audience.get('conditions') ) optly_typed_audiences.append(optly_audience) id_lookup_dict[typed_audience.get('id')] = typed_audience.get('id') @@ -145,7 +144,7 @@ def __init__(self, project_config): optly_audience = OptimizelyAudience( old_audience.get('id'), old_audience.get('name'), - str(old_audience.get('conditions')) + old_audience.get('conditions') ) optly_typed_audiences.append(optly_audience) From cdf407832b1bde07a93fde6837b0ffb95133eb12 Mon Sep 17 00:00:00 2001 From: Jacob Brown <jacob.brown@optimizely.com> Date: Fri, 6 Aug 2021 19:58:14 -0400 Subject: [PATCH 13/13] Trigger CI - Removed unecessary line. --- optimizely/optimizely_config.py | 1 - 1 file changed, 1 deletion(-) diff --git a/optimizely/optimizely_config.py b/optimizely/optimizely_config.py index 2f95b3a5..4dc90bdc 100644 --- a/optimizely/optimizely_config.py +++ b/optimizely/optimizely_config.py @@ -125,7 +125,6 @@ def __init__(self, project_config): Merging typed_audiences with audiences from project_config. The typed_audiences has higher precedence. ''' - optly_typed_audiences = [] id_lookup_dict = {} for typed_audience in project_config.typed_audiences: