Add Eager Options Validation: ValidateOnStart API#47821
Add Eager Options Validation: ValidateOnStart API#47821maryamariyan merged 9 commits intodotnet:masterfrom
Conversation
|
Note regarding the This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change. |
|
Tagging subscribers to this area: @maryamariyan Issue Details
Note: Microsoft.Extensions.Options would need a new dependency to Microsoft.Extensions.Hosting.Abstractions because we use IHostedService to implement this feature. Minor TODO:
|
src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderValidationExtensions.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Options/src/ValidationHostedService.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Options/src/ValidationHostedService.cs
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Options/src/ValidatorOptions.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderValidationExtensions.cs
Outdated
Show resolved
Hide resolved
- Drop new() constraint - Remove _ in most cases - Remove the ConcurrentDictionary - Slight comment cleanup - Skip tests on mono: due to PNSE on host.StartAsync()
3bb0e6f to
68e1b66
Compare
...Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderValidationTests.cs
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderValidationExtensions.cs
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderValidationExtensions.cs
Outdated
Show resolved
Hide resolved
a97f701 to
1c2c367
Compare
src/libraries/Microsoft.Extensions.Options/src/Microsoft.Extensions.Options.csproj
Outdated
Show resolved
Hide resolved
Keeps DataAnnotations tests in Options and rest in Hosting
466b85e to
9107eab
Compare
|
The CI failure is unrelated. @davidfowl @eerhardt The PR should be ready for another review. |
...s.Options/tests/Microsoft.Extensions.Options.Tests/Microsoft.Extensions.Options.Tests.csproj
Show resolved
Hide resolved
.../Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs
Show resolved
Hide resolved
.../Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs
Outdated
Show resolved
Hide resolved
.../Microsoft.Extensions.Options/tests/Microsoft.Extensions.Options.Tests/OptionsBuilderTest.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Hosting/src/OptionsBuilderExtensions.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/OptionsBuilderExtensionsTests.cs
Outdated
Show resolved
Hide resolved
eerhardt
left a comment
There was a problem hiding this comment.
Looks good. Thanks for driving this through, @maryamariyan.
davidfowl
left a comment
There was a problem hiding this comment.
Still want to discuss the home
OK, marking unresolved the conversation thread above: #47821 (comment) Here's a summary of the two approaches tried:
|
Note: Microsoft.Extensions.Options would need a new dependency to Microsoft.Extensions.Hosting.Abstractions because we use IHostedService to implement this feature.
Minor TODO:
Fixes #36391