Skip to content

Conversation

@jimmyca15
Copy link
Member

The current feature manager implementation only logs a warning when a feature filter is missing for a given feature. This makes missing feature filters tricky to diagnose and can lead to unintended behavior. This enhancement changes the default behavior of the feature manager so that it will throw a FeatureManagementException when it encounters a missing feature filter.

This new behavior can be disabled by configuring the feature management options.

services.Configure<FeatureManagementOptions>(options =>
{
    options.IgnoreMissingFeatureFilters = true;
});

This resolves #13

@codapus-co

codapus-co and others added 2 commits December 5, 2019 01:06
…r and the feature hasn't been registered, an exception will be thrown when the feature will be evaluated. (microsoft#13)
{
/// <summary>
/// Controls the behavior of feature evaluation when dependent feature filters are missing.
/// If missing features filters are not ignored an exception will be thrown when attempting to evaluate a feature that depends on a missing feature filter.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NIT: ... features filters ...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks ! I'll update before merge.

Copy link
Member

@zhenlan zhenlan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

_logger = loggerFactory.CreateLogger<FeatureManager>();
_filterMetadataCache = new ConcurrentDictionary<string, IFeatureFilterMetadata>();
_contextualFeatureFilterCache = new ConcurrentDictionary<string, ContextualFeatureFilterEvaluator>();
_options = options?.Value ?? throw new ArgumentNullException(nameof(options));

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

options?.Value [](start = 23, length = 14)

so we don't expect a default value?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default value should be returned by Value.

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 this pull request may close these issues.

An exception should be thrown if a configured feature filter has not been registered.

4 participants