-
Notifications
You must be signed in to change notification settings - Fork 36
[AI-FSSDK] [FSSDK-12337] Add Feature Rollout support #499
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 9 commits
159ee1b
2fba50e
d7387ad
ff06d05
397bacd
d95f1e8
3f05ad8
c73f410
147b0da
d114cc6
14d288d
3da60b5
3a52b56
0c56c4d
39f9cec
04ea47f
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 |
|---|---|---|
|
|
@@ -232,6 +232,37 @@ def __init__(self, datafile: str | bytes, logger: Logger, error_handler: Any): | |
| self.experiment_feature_map[exp_id] = [feature.id] | ||
| rules.append(self.experiment_id_map[exp_id]) | ||
|
|
||
| # Feature Rollout support: inject the "everyone else" variation | ||
| # into any experiment with type == "feature_rollout" | ||
| everyone_else_variation = self._get_everyone_else_variation(feature) | ||
| if everyone_else_variation is not None: | ||
|
Contributor
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. We need this condition to the ticket prompt - |
||
| for experiment in rules: | ||
| if experiment.type == 'feature_rollout': | ||
| experiment.variations.append({ | ||
| 'id': everyone_else_variation.id, | ||
| 'key': everyone_else_variation.key, | ||
| 'featureEnabled': everyone_else_variation.featureEnabled, | ||
| 'variables': cast( | ||
| list[types.VariableDict], | ||
| everyone_else_variation.variables, | ||
| ), | ||
| }) | ||
| experiment.trafficAllocation.append({ | ||
| 'entityId': everyone_else_variation.id, | ||
| 'endOfRange': 10000, | ||
| }) | ||
| self.variation_key_map[experiment.key][everyone_else_variation.key] = everyone_else_variation | ||
| self.variation_id_map[experiment.key][everyone_else_variation.id] = everyone_else_variation | ||
| self.variation_id_map_by_experiment_id[experiment.id][everyone_else_variation.id] = ( | ||
| everyone_else_variation | ||
| ) | ||
| self.variation_key_map_by_experiment_id[experiment.id][everyone_else_variation.key] = ( | ||
| everyone_else_variation | ||
| ) | ||
| self.variation_variable_usage_map[everyone_else_variation.id] = self._generate_key_map( | ||
| everyone_else_variation.variables, 'id', entities.Variation.VariableUsage | ||
| ) | ||
|
|
||
| flag_id = feature.id | ||
| applicable_holdouts: list[entities.Holdout] = [] | ||
|
|
||
|
|
@@ -667,6 +698,41 @@ def get_rollout_from_id(self, rollout_id: str) -> Optional[entities.Layer]: | |
| self.logger.error(f'Rollout with ID "{rollout_id}" is not in datafile.') | ||
| return None | ||
|
|
||
| def _get_everyone_else_variation(self, flag: entities.FeatureFlag) -> Optional[entities.Variation]: | ||
| """ Get the "everyone else" variation for a feature flag. | ||
|
|
||
| The "everyone else" rule is the last experiment in the flag's rollout, | ||
| and its first variation is the "everyone else" variation. | ||
|
|
||
| Args: | ||
| flag: The feature flag to get the everyone else variation for. | ||
|
|
||
| Returns: | ||
| The "everyone else" Variation entity, or None if not available. | ||
| """ | ||
| if not flag.rolloutId: | ||
| return None | ||
|
|
||
| rollout = self.get_rollout_from_id(flag.rolloutId) | ||
| if not rollout or not rollout.experiments: | ||
| return None | ||
|
|
||
| everyone_else_rule = rollout.experiments[-1] | ||
| variations = everyone_else_rule.get('variations', []) | ||
| if not variations: | ||
| return None | ||
|
|
||
| variation_dict = variations[0] | ||
|
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. Is it save to access? What happen if it is empty
Contributor
Author
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. It is safe. Line 722-723 ensures its not empty and if empty None is returned. |
||
| return entities.Variation( | ||
| id=variation_dict['id'], | ||
| key=variation_dict['key'], | ||
| featureEnabled=bool(variation_dict.get('featureEnabled', False)), | ||
| variables=cast( | ||
| Optional[list[entities.Variable]], | ||
| variation_dict.get('variables'), | ||
| ), | ||
| ) | ||
|
|
||
| def get_variable_value_for_variation( | ||
| self, variable: Optional[entities.Variable], variation: Optional[Union[entities.Variation, VariationDict]] | ||
| ) -> Optional[str]: | ||
|
|
||
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.
flags have these types -
class RuleTypes(enum.Enum):
targeted_delivery = "targeted_delivery"
ab = "a/b"
multi_armed_bandit = "multi_armed_bandit"
contextual_multi_armed_bandit = "contextual_multi_armed_bandit"
if we want to keep short names - let's consistent. what about
{"ab", "mab", "cmab", "td", "fr"}
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.
We can specify these types in the ticket.
Also need to update TDD as well.