Skip to content

Conversation

@zhiyuanliang-ms
Copy link
Member

Resolve #308

@zhiyuanliang-ms zhiyuanliang-ms marked this pull request as draft November 14, 2023 02:52
options.RequireFeatureManagementSection = false;
});

services.AddSingleton<IFeatureDefinitionProvider>(sp => new ConfigurationFeatureDefinitionProvider(
Copy link
Member

Choose a reason for hiding this comment

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

I don't see a need for FeatureManagementOptions to be updated. In fact it's dangerous. If IFeatureDefinitionProvider that is created with the inline configuration is not the one that ends up being used (multiple addfeaturemanagmeent calls), then the options won't match the added service.

I think it needs to be

            services.TryAddSingleton<IFeatureDefinitionProvider>(sp =>
                new ConfigurationFeatureDefinitionProvider(
                    configuration, 
                    sp.GetRequiredService<ILoggerFactory>())
                {
                    UseTopLevelConfiguration = true
                });

@zhiyuanliang-ms zhiyuanliang-ms marked this pull request as ready for review November 15, 2023 08:24
@zhiyuanliang-ms zhiyuanliang-ms merged commit 17a9af5 into main Nov 16, 2023
@zhiyuanliang-ms zhiyuanliang-ms deleted the zhiyuanliang/option-for-no-fm-section-found branch November 17, 2023 04:41
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.

builder.Services.AddFeatureManagement() with configuration section does not work since v3.0.0

3 participants