Skip to content
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

Need ability to get all features #29

Closed
millicandavid opened this issue Dec 2, 2019 · 11 comments · Fixed by #36
Closed

Need ability to get all features #29

millicandavid opened this issue Dec 2, 2019 · 11 comments · Fixed by #36

Comments

@millicandavid
Copy link

I need the ability to get a list of all features and potentially get only enabled features as well.

@mariotee
Copy link

mariotee commented Dec 3, 2019

in Startup, can do like

using System.Linq;
//...
private IConfiguration AppSettings;
//...
//this gets the list of keys under the "FeatureManagement" object value, 
//assuming following their convention
AppSettings.GetSection("FeatureManagement").GetChildren().Select((f) => f.Key);

getting list of enabled features may be more tricky if you are using the more advanced features. if you are only using the "key": boolean method, then first filter with LINQ .Where on bool.Parse(feature.Value) == true and then .Select on the keys to return the list of keys

@millicandavid
Copy link
Author

I already had something similar in place actually. It just seems like something that would be convenient for FeatureManager to provide and potentially more useful than just asking for each feature one by one if you have a long list of features. Something like FeatureManager.GetAllFeatures() and FeatureManager.GetEnabledFeatures() would reduce the number of round trips on a higher latency device with questionable network quality.

@zhenlan
Copy link
Member

zhenlan commented Dec 4, 2019

@millicandavid the code snippet @mariotee suggested is searching configuration from the memory. It won't cause any network calls.

Just for my learning, @millicandavid, do you mind to elaborate on what are the scenarios you will want those APIs?

@millicandavid
Copy link
Author

@zhenlan I understand that iterating the list of keys does not cause any network traffic, and I suppose I'm making an assumption that may not necessarily be true, but I thought my calls to _featureManager.IsEnabled(key) which are inside that iteration would be making calls to some feature management API. I suppose I should verify if that is the case or not. So if FeatureManager is only using in memory IConfiguration to resolve IsEnabled() then that's not an issue.

We front end devs were asking to just get a list of all enabled features with one call and I have this workaround so it's not a big deal. I just thought providing this within FeatureManager would make for a cleaner solution and it seemed to be within its domain. For my solution I suppose I don't need FeatureManager at all and can just use IConfiguration for everything.

@zhenlan
Copy link
Member

zhenlan commented Dec 5, 2019

@millicandavid thanks for sharing. The IsEnabled() API will call feature filters (if any) as part of feature flag evaluation, so it depends on whether any of the feature filters making network calls. However, when feature filters are used, the feature flag value will depend on the context where feature filters are evaluated. This means a FeatureManager.GetEnabledFeatures() API could potentially return different results for every request (in a web application). This is why I'm interested to learn what are the scenarios this will be useful.

@jimmyca15
Copy link
Member

@zhenlan the ask here is a bit simpler I believe. We can expose an api IEnumerable<string> GetFeatures(). This is useful otherwise someone has to interact with IConfiguration directly as Mariotee mentioned in his workaround. The user should not have to interact with IConfiguration to use feature manager.

Exposing GetEnabledFeatures would be unnecessary. Rather one would just iterate over the features returned by GetFeatures() and then evaluate whether they are enabled.

@zhenlan
Copy link
Member

zhenlan commented Dec 6, 2019

Yes, a helper to get the list of all features makes sense. Maybe a property FeatureManager.Features.

@millicandavid
Copy link
Author

If GetEnabledFeatures() could be IEnumerable because you expect them all to be enabled. However, GetFeatures() would have to be something like IEnumerable<KeyValuePair<string, bool>> right? And yes, just having GetFeatures() would be enough if you had the feature and bool value.

@jimmyca15
Copy link
Member

The proposal is to not do any evaluation of the feature state with the GetFeatures() method. It also may be clearer to call it GetFeatureNames(). All that is needed from the feature manager is to be able to retrieve a list of registered features. If the caller wants to evaluate all their states (which requires running filters) and only use enabled features then that could be done like this:

Proposed Snippet

List<string> enabledFeatures = new List<string>();

foreach (string feature in await _featureManager.GetFeatureNames())
{
    if (await _featureManager.IsEnabled(feature))
    {
        enabledFeatures.Add(feature);
    }
}

@millicandavid
Copy link
Author

Any idea when you expect to deliver this in a package?

@jimmyca15
Copy link
Member

@millicandavid I would estimate early March. At the moment the Azure App Configuration team is buckled down to prepare for the GA of the service. There are a few open issues that we will tackle/fix all at once and this will be included in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants