-
Notifications
You must be signed in to change notification settings - Fork 119
Suggested Readme Changes #173
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
Conversation
README.md
Outdated
| ### Feature Variant Assignment | ||
|
|
||
| When requesting the value of a dynamic feature the feature manager needs to determine which variant of the feature should be used. The act of choosing which variant should be used is called assignment. A built-in method of assignment is provided that allows the variants of a dynamic features to be assigned to segments of an application's audience. This is the same [targeting](./README.md#MicrosoftTargeting-Feature-Variant-Assigner) strategy introduced by the targeting feature filter. | ||
| When requesting the value of a dynamic feature, the feature manager needs to determine which variant of the feature to assign. The act of choosing which of the variants to be used is called "assignment." A built-in method of assignment allows the variants of a dynamic feature to be assigned to segments of an application's audience. This is the same [targeting](./README.md#MicrosoftTargeting-Feature-Variant-Assigner) strategy used by the targeting feature filter. |
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.
"to assign" is avoided purposefully to lead into the introduction of the assignment terminology.
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.
So, for that first sentence, something just rubs me the wrong way when I read it. It is a bit wordy, does this work better.
"When requesting the value of a dynamic feature, the feature manager needs to determine which variant to use."
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.
That works for me.
README.md
Outdated
| This feature variant assigner provides the capability to assign the variants of a dynamic feature to targeted audiences. An in-depth explanation of targeting is explained in the [targeting](./README.md#Targeting) section. | ||
|
|
||
| The assignment parameters used by the targeting feature variant assigner include an audience object which describes users, groups, and a default percentage of the user base that should receive the associated variant. Each group object that is listed in the target audience must also specify what percentage of the group's members should have receive the variant. If a user is specified in the users section directly, or if the user is in the included percentage of any of the group rollouts, or if the user falls into the default rollout percentage then that user will receive the associated variant. | ||
| The assignment parameters used by the targeting feature variant assigner include an audience object which describes the user base that should receive the associated variant. The user base is made of users, groups, and a default percentage of the users. Each group object that is listed in the target audience is required to specify what percentage of the group's members should have receive the variant. If a user is specified in the users section directly, or if the user is in the included percentage of any of the group rollouts, or if the user falls into the default rollout percentage then that user will receive the associated variant. |
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.
The user base is made of users, groups, and a default percentage of the users.
Seems like it should be "The audience is made of users, groups, and a percentage of the entire user base."
README.md
Outdated
| ``` | ||
|
|
||
| Based on the configured audiences for the variants included in this feature, if the application is executing under the context of a user named `Alec` then the value of the `Big` variant will be returned. If the application is executing under the context of a user named `Susan` then the value of the `Small` variant will be returned. If a user match does not occur, then group matches are evaluated. If the application is executing under the context of a user in the group `Ring0` then the `Big` variant will be returned. If the user's group is `Ring1` instead, then the user has a 50% chance between being assigned to `Big` or `Small`. If there is not user match nor group match then the default rollout percentage is used. In this case, 80% of unmatched users will get the `Small` variant leaving the other 20% to get the `Big` variant since it is marked as the `Default`. | ||
| Based on the configured audiences for the variants included in this feature, if the application is executed under the context of a user named `Alec` then the value of the `Big` variant will be returned. If the application is executing under the context of a user named `Susan` then the value of the `Small` variant will be returned, unless `Susan` is part of one of the groups or `DefaultRolloutPercentage` of `Big`. If a user match does not occur, then group matches are evaluated. If the application is executed under the context of a user in the group `Ring0` then the `Big` variant will be returned. If the user's group is `Ring1` instead, then the user has a 50% chance of being assigned to `Small`. If there is no user match nor group match, then the default rollout percentage is used. In this case, 80% of unmatched users will get the `Small` variant, leaving the other 20% to get the `Big` variant since it is marked as the `Default`. |
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.
User override should take precedent over group override.
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.
Ah... I don't think that is how the Spring implementation works. And I thought I had the same implementation as you. I loop through the Variants, first checking User, then Group(s), then default percentage. If anything returns true, you get assigned that variant.
I would expect it to work this way as it is the same with Feature Flags, if anything returns true we stop processing and return true.
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.
Sounds like spring and .net have different behaviors. The point of specifying users directly is to ensure they get a given variant.
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.
Looks like the targeting assignment in the feature/v3 branch doesn't work as expected either. Seems this pointed out a bug.
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.
Ok, so this was one of the things bugging me about the implementation, which is why I called it out. So, should it work like this?
Check the assigned users of each variant.
Check the assigned groups of each variant, we can't do by group as they don't have to be the same list.
Run defaultRolloutPercentage checks for each one
If still false, use the default
?
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.
Yes
No description provided.