-
Notifications
You must be signed in to change notification settings - Fork 116
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
[WIP] add variant allocation context option #506
base: main
Are you sure you want to change the base?
[WIP] add variant allocation context option #506
Conversation
Thank you for the PR! We've looked into this a few times and haven't aligned on a solution here. I'll add some of my notes here. We considered exactly this design you have here. If I recall correctly, the biggest argument against it was the idea of "stability" for allocation. In short, the idea is:
This is a new concept that filters don't do. Filters break this hard rule, because a filter is free to run any custom code it wants and might simply flip its results on each run. This stability is important- as it gives us new guarantees on users who are assigned variants (we know what they got and what they'll always get). The design I liked a little more was keeping the filters outside, but allowing variant shortcutting. Having the logic be "filters" -> if no variant -> "allocation" Like: {
"id": "TestVariant",
"enabled": true,
"conditions": {
"client_filters": [
{
"name": "TestFilter",
"variant": "V1",
"parameters": {
"SomeParam": "SomeValue"
}
},
{
"name": "TestFilter",
"variant": "V2",
"parameters": {
"SomeParam": "SomeValue2"
}
}
]
},
"variants": [
{
"name": "Default",
"configuration_value": "Default"
},
{
"name": "V1",
"configuration_value": "Variant 1"
},
{
"name": "V2",
"configuration_value": "Variant 2"
}
]
} The pushback to this design is two main items:
|
Either way- I think this PR is valuable. Both as a place to discuss and a resource for others looking to do the same thing. |
@rossgrambo - I think I like the short circuit option more actually. It's similar to a few other systems I've used before. Some thoughts:
Makes sense, in this PR if you provide a context that implements
I think it's fair that if a developer writes custom filters, they're also on the hook for maintaining that consistency (or choosing not to). This could be particularly relevant for third-party
I ran into this as well, while it pained me to duplicate much of the filter evaluation code, I couldn't think of a clean way to keep the Any vs All paradigm. It made the most sense that the first matching filter would win but this might not be desirable in all scenarios. I do believe that
Just wild spit balling here, but what if there was no allocation section and we simply relied on You do lose the ability to enable or disable variants with the filters with this approach, but unlike boolean flags it's more likely that the variant's filters would be more exhaustive. From a glance this also seems similar to how OpenFeature works, but I've never used that: https://openfeature.dev/specification/glossary#evaluating-flag-values |
This is quick and dirty, but I needed it for a POC project and thought getting it some attention would be helpful. I still need to add unit tests. Resolves #460.
Usage: