- Notifications
You must be signed in to change notification settings - Fork 37
Add Audiences to OptimizelyConfig and expose in OptimizelyExperiment #342
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
65 commits Select commit Hold shift + click to select a range
e80fdfb added sdk and environment key
ozayr-zaviar d58b71a [MAINTENANCE] Remove Deprecated warnings during build
The-inside-man a583ea5 [OASIS-7757] Fix spelling of environment to fix testcases from failing
The-inside-man 5b2196c [OASIS-7757] - Added additional test cases to test_optimizely and tes…
The-inside-man f964f94 [OASIS-7800] Updated optimizely_config with attributes and events
The-inside-man f60ab0a [OASIS-7757] - update copyright years and add more testcases for sdk_…
The-inside-man df1b081 [OASIS-7800] Updated optimizely_config with attributes and events
The-inside-man 9c9bcea Run autopep8 to fix formatting issues after rebase
The-inside-man 61d5c24 [OASIS-7800] - Added test cases for attribute map and events map
The-inside-man 13703ca Remove unused import from optimizely config
The-inside-man ef95cd8 Corrected comment wording in get functions for events.
The-inside-man b8828b9 [OASIS-7812] - Add Audiences to OpotimizelyConfig to expose to users
The-inside-man c3bc816 Remove unused import for Experiment in OptimizelyConfig
The-inside-man f1bd533 Update Formatting issues for lint
The-inside-man 0ad43d0 Update Import for conditions in optimizely_config.py
The-inside-man 942bfe9 Additional changes for audiences and added functions to create attrib…
The-inside-man 528b8f2 Add delivery_rules to OptimizelyConfig and testcases to support
The-inside-man c3c3d2c Formatting issues
The-inside-man 2fef52a Fix line length in comments and statement
The-inside-man d3b2a46 Shortened return statement
The-inside-man dda6e76 Updated list copy to use slice for backwards compatibility
The-inside-man 4f2d7fb Add in test datafile for typed audiences
The-inside-man f90e3e3 remove unused import
The-inside-man 4471555 Merge branch 'master' into jbrown/oasis-7812
The-inside-man bb98741 Modify test_update_experiment to pass audiences as OptimizelyExperiment
The-inside-man 48f604b Fix linting issues
The-inside-man 8179f2b Style changes as requested to comments and variable naming.
The-inside-man d694c7f Lint Update
The-inside-man ec39096 Fix line length
The-inside-man a687a9b Move upper() to central location to only call once
The-inside-man 11be430 Indent fix
The-inside-man ecf2e45 Broke statement into two for Lint issues
The-inside-man f568aad Added test case for variation
The-inside-man 3b8706f Add changes to decision service to us experiment ID instead of KEY.
The-inside-man 2504f98 Missing newline added
The-inside-man 1cd6aab Remove redundant update function. Ad
The-inside-man bc98adc Rename testcase to remove replace_ids_with_names.
The-inside-man 98ff2b6 Duplicate experiment Key issue with multi feature flag (#347)
The-inside-man d6fa32d Update delivery rules test.
The-inside-man 16059d7 Explenation added to flake8 for ignoring W504.
The-inside-man 0320627 Changes made to move delivery rules and experiment rules into Optimiz…
The-inside-man 8187f6f Formatter on new changes
The-inside-man 0804bb8 Merge branch 'master' into jbrown/oasis-7812
The-inside-man ba3bbef Remove TODO and other dev comments
The-inside-man 2567bac Remove TODO and other dev comments
The-inside-man d401c72 Remove unused imoprt
The-inside-man 6b4a00d Added nl after imports to follow Flake8
The-inside-man a1ed815 Add except for if incorrect type used in lookup_name_from_id() to pre…
The-inside-man ec4218a Correct except to assign audience_id instead of blank string
The-inside-man 508f82f Casting name to string in case non string type for testing
The-inside-man 26ad430 Added test and modification to account for scenario when NOT is follo…
The-inside-man 1f35dda Update from isinstance to type and change List to list for python 3.4
The-inside-man a4e497f Simplified stringifyConditions algorithm to be more readable and less…
The-inside-man bf92fd7 Updated changes - reuse test logic, modify stringify to proper handle…
The-inside-man 2d6def3 Change to single test to run all cases for stringify.
The-inside-man bec791a Move config_service out of loop.
The-inside-man 286f178 Added missed step in delivery rules.
The-inside-man c3faba3 Update to properly refect datafile examples.
The-inside-man 199a35f Remove unused import
The-inside-man 4fa5d05 Cleaned up get functions till overall cleanup of code.
The-inside-man 4356a4d Merge branch 'master' into jbrown/oasis-7812
The-inside-man a273d11 Merge branch 'master' into jbrown/oasis-7812
The-inside-man c8d68f7 Added comments to stringify_conditions, added testcase to test string…
The-inside-man a9ac50c Correct spelling of precedence
The-inside-man 6ce98e3 Update comment to be more readable.
The-inside-man File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Update Formatting issues for lint
- Loading branch information
commit f1bd5337cfd8bbe42b4abc9472e27e2704d9683a
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| | @@ -70,22 +70,6 @@ def get_events(self): | |
| """ | ||
| return self.events | ||
| | ||
| def get_attributes(self): | ||
| """ Get the attributes associated with OptimizelyConfig | ||
| | ||
| returns: | ||
| A list of attributes. | ||
| """ | ||
| return self.attributes | ||
| | ||
| def get_events(self): | ||
| """ Get the events associated with OptimizelyConfig | ||
| | ||
| returns: | ||
| A list of events. | ||
| """ | ||
| return self.events | ||
| | ||
| def get_audiences(self): | ||
| """ Get the audiences associated with OptimizelyConfig | ||
| | ||
| | @@ -182,7 +166,8 @@ def __init__(self, project_config): | |
| | ||
| for old_audience in project_config.audiences: | ||
| # check if old_audience.id exists in new_audiences.id from typed_audiences | ||
| if len([new_audience for new_audience in typed_audiences if new_audience.get('id') == old_audience.get('id')]) == 0: | ||
| if len([new_audience for new_audience in typed_audiences | ||
| if new_audience.get('id') == old_audience.get('id')]) == 0: | ||
| if old_audience.get('id') == "$opt_dummy_audience": | ||
| continue | ||
| else: | ||
| | @@ -195,7 +180,7 @@ def __init__(self, project_config): | |
| for audience in self.audiences: | ||
| audiences_map[audience.get('id')] = audience.get('name') | ||
| | ||
| # Updating each entities.Experiment found in the experiment_key_map | ||
| # Updating each OptimizelyExperiment based on the ID from entities.Experiment found in the experiment_key_map | ||
| for ent_exp in project_config.experiment_key_map.values(): | ||
| experiments_by_key, experiments_by_id = self._get_experiments_maps() | ||
| try: | ||
| | @@ -206,18 +191,39 @@ def __init__(self, project_config): | |
| continue | ||
| | ||
| def update_experiment(self, experiment, conditions, audiences_map): | ||
| ''' | ||
| Gets an OptimizelyExperiment to update, conditions from | ||
| the corresponding entities.Experiment and an audiences_map | ||
| in the form of [id:name] | ||
| | ||
| Updates the OptimizelyExperiment.audiences with a string | ||
| of conditions and audience names. | ||
| ''' | ||
| audiences = self.replace_ids_with_names(conditions, audiences_map) | ||
| experiment.audiences = audiences | ||
| | ||
| def replace_ids_with_names(self, conditions, audiences_map): | ||
| # Confirm where conditions are coming from... | ||
| if conditions != None: | ||
| ''' | ||
| Gets conditions and audiences_map [id:name] | ||
| | ||
| Returns: | ||
| a string of conditions with id's swapped with names | ||
| or None if no conditions found. | ||
| | ||
| ''' | ||
| if conditions is not None: | ||
| return self.stringify_conditions(conditions, audiences_map) | ||
| else: | ||
| return None | ||
| | ||
| def lookup_name_from_id(self, audience_id, audiences_map): | ||
| ''' | ||
| Gets and audience ID and audiences map | ||
| | ||
| Returns: | ||
| The name corresponding to the ID | ||
| or None if not found | ||
| ''' | ||
| name = "" | ||
| try: | ||
| name = audiences_map[audience_id] | ||
| | @@ -227,6 +233,14 @@ def lookup_name_from_id(self, audience_id, audiences_map): | |
| return name | ||
| | ||
| def stringify_conditions(self, conditions, audiences_map): | ||
| ''' | ||
| Gets a list of conditions from an entities.Experiment | ||
| and an audiences_map [id:name] | ||
| | ||
| Returns: | ||
| A string of conditions and names for the provided | ||
| list of conditions. | ||
| ''' | ||
| ARGS = ConditionOperatorTypes.operators | ||
The-inside-man marked this conversation as resolved. Show resolved Hide resolved | ||
| condition = "" | ||
| ret = "(" | ||
| | @@ -248,15 +262,16 @@ def stringify_conditions(self, conditions, audiences_map): | |
| condition = conditions[i] | ||
| else: | ||
| if type(conditions[i]) == list: | ||
| # If the next item is a list, recursively call function on list | ||
| if i + 1 < length: | ||
| ret += self.stringify_conditions(conditions[i], | ||
| audiences_map) + ' ' + condition.upper() + ' ' | ||
| else: | ||
| ret += self.stringify_conditions(conditions[i], audiences_map) | ||
| else: | ||
| # Handle ID's here - Lookup required | ||
| # Handle ID's here - Lookup name based on ID | ||
| ||
| audience_name = self.lookup_name_from_id(conditions[i], audiences_map) | ||
| if audience_name != None: | ||
| if audience_name is not None: | ||
| if i + 1 < length: | ||
| ret += '"' + audience_name + '" ' + condition.upper() + ' ' | ||
| else: | ||
| | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit. This suggestion is invalid because no changes were made to the code. Suggestions cannot be applied while the pull request is closed. Suggestions cannot be applied while viewing a subset of changes. Only one suggestion per line can be applied in a batch. Add this suggestion to a batch that can be applied as a single commit. Applying suggestions on deleted lines is not supported. You must change the existing code in this line in order to create a valid suggestion. Outdated suggestions cannot be applied. This suggestion has been applied or marked resolved. Suggestions cannot be applied from pending reviews. Suggestions cannot be applied on multi-line comments. Suggestions cannot be applied while the pull request is queued to merge. Suggestion cannot be applied right now. Please check back later.
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.
This covers "experiment" rules. We also need to do the same to experiments in rollout ("delivery" rules) as well.
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.
Delivery Rules added to OptimizelyConfig and test cases to cover