Add ReloadOnChange to KeyPerFile configuration provider #2808
Conversation
| public bool Optional { [System.Runtime.CompilerServices.CompilerGeneratedAttribute] get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute] set { } } | ||
| public int ReloadDelay { [System.Runtime.CompilerServices.CompilerGeneratedAttribute] get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute] set { } } | ||
| public bool ReloadOnChange { [System.Runtime.CompilerServices.CompilerGeneratedAttribute] get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute] set { } } | ||
| public Microsoft.Extensions.Configuration.IConfigurationProvider Build(Microsoft.Extensions.Configuration.IConfigurationBuilder builder) { throw null; } |
There was a problem hiding this comment.
Will need API review (even if its the same as what other providers do).
| /// <param name="optional">Whether the directory is optional.</param> | ||
| /// <returns>The <see cref="IConfigurationBuilder"/>.</returns> | ||
| public static IConfigurationBuilder AddKeyPerFile(this IConfigurationBuilder builder, string directoryPath, bool optional) | ||
| => builder.AddKeyPerFile(directoryPath, optional, reloadOnChange: false); |
There was a problem hiding this comment.
question - do our other config providers do reloading by default? Looks like JSON does not: https://github.com/aspnet/Extensions/blob/master/src/Configuration/Config.Json/src/JsonConfigurationExtensions.cs#L38
There was a problem hiding this comment.
We should be consistent so it should be off by default.
There was a problem hiding this comment.
Yeah this is expensive and has some unexpected side effects so generally reload should be off by default unless people explicitly make it enabled
src/Configuration/Config.KeyPerFile/src/KeyPerFileConfigurationProvider.cs
Outdated
Show resolved
Hide resolved
src/Configuration/Config.KeyPerFile/src/KeyPerFileConfigurationBuilderExtensions.cs
Show resolved
Hide resolved
|
Sure i can drive this |
| Assert.Equal("Foo", options.Text); | ||
| } | ||
|
|
||
| [Fact] |
There was a problem hiding this comment.
Can you add a test where nothing exists to start, and the directory/files get created later so its the nothing found to something case as opposed to just modification scenarios?
|
Looks good, thanks @Kahbazi ! |
…sions#2808) \n\nCommit migrated from dotnet/extensions@cca1c7c
…sions#2808) \n\nCommit migrated from dotnet/extensions@cca1c7c
ghost
locked as resolved and limited conversation to collaborators
6 participants
Addresses #1085