-
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
Allow feature filters to contextually apply #127
Comments
Hi @patrikmolsson, I'm having some trouble digesting the issue here. "So the core logic is then changed from "if any feature filter evaluates to true, that feature will be considered enabled." to instead iterate over the filters and use the result of the first filter that is active, and use its result, disregarding the other filters." Could you give a specific scenario to explain it? Do you mean that if a feature flag has 4 filters, let's say filter1, filter2, filter3 and filter4. The filter1 is the pivot filter which you want it be true all the time. If filter1 is false, then the feature flag will always be disabled. Is my understanding correct? |
Hi @zhiyuanliang-ms thanks for looking into this. My case may not be relevant anymore, since it was more than 2 years ago I wrote it, but I'll give it a try explaining it.
Sure - that sentence was more discussing the current solution, and how you would need to change it to fit my need. You can safely disregard it if you have any other solution. The requirements would rather be: If I have a custom filter built - e.g. QueryParamFilter - and I want this to be able to disable features as well - I would need to be able to configure whether this filter should even be evaluated or not. E.g. if the query param name is not present in the query string, I shouldn't even invoke EvaluateAsync. So the examples would be: I have two filters - PercentageFilter (50%) and my own QueryParamFilter. I have a user, which is a developer for the product, so they need to be able to test around and verify behaviour in production.
Hopefully these examples provide some insight. |
@patrikmolsson Thank you for the detailed explanation. Now I understand the scenario. Currently, we support the parameter "RequirementType", if the "RequirementType" is "All", which means all filters listed in "EnabledFor" should evaluate the feature flag as true. This can solve some simple case. In the issue #28, we have discussed the possibility to support complex logical relation between filters. We may consider to support that in the future. For now, hope "RequirementType" can provide your some convenience. |
Hi I was in progress of switching over to this library from a proprietary solution when I noticed some functionality missing.
In our solution we allow (some) users to override the evaluation of a feature flag, e.g. for developers to test the feature out on production, by overriding the evaluation using a query string:
?myFeatureFlag=1
(also mentioned in #106). Although, I believe it would still be beneficial to apply a "global" set of filters, as explained in #89.Unfortunately, I realized that there is one critical piece left: to be able for a filter to override another positive evaluation. I.e. if one filter evaluates to true - I also want the possibility to override that to force the evaluation to false:
?myFeatureFlag=0
.The current IFeatureFilter interface doesn't allow that granularity. In our proprietary solution we also add another dimension, whether or not the filter should be considered "active". In the example above the logic for whether or not the filter should be active is based off whether the feature name is present in the query string.
So the core logic is then changed from "if any feature filter evaluates to true, that feature will be considered enabled." to instead iterate over the filters and use the result of the first filter that is active, and use its result, disregarding the other filters.
I know that this feature request results in a big overhaul of the current architecture, but I just wanted to throw it out there and see if there are any workarounds for my case.
The text was updated successfully, but these errors were encountered: