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

Cleanup and remove unnecessary code #1

Merged
merged 1 commit into from Sep 4, 2019

Conversation

lpatalas
Copy link
Collaborator

@lpatalas lpatalas commented Sep 4, 2019

I think we don't need this IFeatureToggleConfigurationReader interface
and it's implementations because needed functionality is already provided
by AspNet configuration system. WebHost.CreateDefaultBuilder method that
is called in Program.cs registers default configuration sources which are
exactly the same as you added in Startup.cs so they can be removed as well.

All we have to do is add new configuration section to appsettings.json file
and define class to hold options and inject it wherever we need it using
IOptionsSnapshot<FeatureToggleOptions>. Configuration watches appsettings
for changes so we always get latest values.

To override toggles from environment variables we also don't have to do
anything extra. Any variable named FeatureToggles__PropertyName will
overwrite existing property in "FeatureToggles" section from appsettings.

Additional configuration sources should be registered in Program.cs by
calling .ConfigureAppConfiguration(...) method on WebHostBuilder.

I think we don't need this `IFeatureToggleConfigurationReader` interface
and it's implementations because needed functionality is already provided
by AspNet configuration system. `WebHost.CreateDefaultBuilder` method that
is called in `Program.cs` registers default configuration sources which are
exactly the same as you added in `Startup.cs` so they can be removed as well.

All we have to do is add new configuration section to `appsettings.json` file
and define class to hold options and inject it wherever we need it using
`IOptionsSnapshot<FeatureToggleOptions>`. Configuration watches appsettings
for changes so we always get latest values.

To override toggles from environment variables we also don't have to do
anything extra. Any variable named `FeatureToggles__PropertyName` will
overwrite existing property in "FeatureToggles" section from appsettings.

Additional configuration sources should be registered in `Program.cs` by
calling `.ConfigureAppConfiguration(...)` method on WebHostBuilder.
Copy link
Owner

@wojciech-dabrowski wojciech-dabrowski left a comment

Choose a reason for hiding this comment

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

I didn't know that environment variables would behave like that. I understand the point of simplifying the code, but I'm not sure about this approach. Right now FeatureToggleOptions will be choosen arbitraly by the runtime. It is not configured expicitly to use configuration file or environment variables.

However, you probably convinced me. It is all about understanding of AspNet configuration system and if you are more familiar with that maybe it is as obvious behaviour.

@wojciech-dabrowski wojciech-dabrowski merged commit 513d02d into wojciech-dabrowski:master Sep 4, 2019
@lpatalas
Copy link
Collaborator Author

lpatalas commented Sep 5, 2019

That's how AspNet configuration is designed and it's actually very useful thing. It means that application does not does not have to be aware from where configuration comes at runtime and it makes it easy to build binaries only once and still be able to run them on different environments.

Usually in appsettings.json you have all settings set to some default values and when you run app locally that's what is used by application. When you deploy the same app to cloud you override only those entries that actually are specific to given environment like connection strings or some service URLs. All other settings are taken from base appsettings.json. If you get the same binaries and deploy them to some other environment that have some properties already specified in environment variables they will be picked up automatically and so on.

Same thing with toggles. We can have default settings in appsettings.json and per-environment overrides in ParameterStore. You can also do things like running app from command line with specific toggle set like: dotnet run /FeatureToggles:IsFirstFeatureEnabled=true. We could also have feature toggles in separate file featuretoggles.json and also add it as source to ConfigurationBuilder and it will be merged with all the rest. Overall it's pretty flexible system.

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.

None yet

2 participants