Add possibility to configure options validation in order to validate options at the start up time (not at runtime). #2392
Comments
|
From @dnfclas on Monday, February 20, 2017 9:50:05 AM Hi @MaKCbIMKo, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! TTYL, DNFBOT; |
|
From @dnfclas on Monday, February 20, 2017 10:02:42 AM @MaKCbIMKo, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR. |
|
From @HaoK on Tuesday, March 7, 2017 1:19:24 PM Take a look at aspnet/Options#161 which is tracking some features which would be useful for options to provide for the Auth 2.0 work, named options will add a bit of a wrinkle since there'd need to be the ability to target All instances vs named instances, vs the default instance. We haven't decided that we are bringing back named options yet, but its something to consider |
|
From @MaKCbIMKo on Tuesday, March 14, 2017 11:21:37 AM @HaoK, I've modified (a lot) of validation stuff. Moreover, I took a look on feature (about multi-tenant) and found that it might require some validation 'on creation' time. I also added that (wasn't not a big deal). About multi-tenancy: it all depends on how we will configure all these 'names', where we will store cached instances (for different names), but I think it won't be so difficult to modify current validation to work in a new way. But, only if you have some roadblocks that you still not sure (or don't know) how to break... |
|
From @HaoK on Tuesday, March 14, 2017 11:27:10 AM Thanks, I'll try to take a look this week |
|
From @HaoK on Wednesday, March 15, 2017 5:16:25 PM So I'm going to be starting a PR for aspnet/Options#161, it will be a pretty lightweight feature to start, I'll submit it by EOD tomorrow, so you can provide input and insight since you've been playing around with validation for a while. The initial critical feature is named options support, so the first iteration of the PR will likely have very minimal validation support (think |
|
From @MaKCbIMKo on Thursday, March 16, 2017 6:13:16 AM Ok, waiting for your PR. |
|
From @HaoK on Thursday, March 16, 2017 12:22:25 PM Take a look at aspnet/Options#176 |
|
From @MaKCbIMKo on Thursday, March 16, 2017 1:19:58 PM @HaoK - I reviewed aspnet/Options#176 and left a comment. What I found is that your PR might reduce the amount of changes and be some kind of 'base' for this PR. (see the comments for aspnet/Options#176). |
|
From @MaKCbIMKo on Wednesday, March 29, 2017 2:30:21 PM @HaoK - Is it fine to base this PR over your initial Options 2.0 changes? Or there more changes are comming? Maybe something in my PR need to be reconsidered... |
|
From @HaoK on Wednesday, March 29, 2017 2:43:03 PM Feel free to rebase if you want, there likely will be more changes coming, but it won't be for a few weeks |
|
From @HaoK on Wednesday, April 26, 2017 1:00:28 PM See aspnet/Options#187, adding an Initialize set of actions that runs after Configure for fixup, you could potentially use these for validation as well. Auth is going to run validation outside of options since it needs to run every request potentially, so it will only use this feature for fixup |
|
From @MaKCbIMKo on Thursday, April 27, 2017 6:51:51 AM @HaoK - At first view, 'Initialization' step might be used for validation, but this validation will be applied all the time. Is that fine? What if want to validate only one time at start up (is that real case ?) ? |
|
From @MaKCbIMKo on Thursday, April 27, 2017 6:54:26 AM update: I implemented a common validation (for unnamed options). Faced some troubles with named options: If we want to validate the options at start up, then we need to know all 'names'. It means that we need to store somewhere all names. But it argues with ability to inject the options that wasn't configured at all (because of |
|
From @MaKCbIMKo on Monday, June 26, 2017 9:36:54 AM @HaoK - I have added a possibility to validate all named options. Please take a look. p.s. there is some problems with Travis CI check (Error: Unable to locate libunwind. Install libunwind to continue) - not sure how to solve that weather the problem with my changes. |
|
From @MaKCbIMKo on Tuesday, July 18, 2017 2:23:25 AM Changes from dev branch has been merged. @HaoK - may I ask you to review the PR? |
|
From @MaKCbIMKo on Tuesday, July 18, 2017 2:26:27 AM About 'AppVeyor build failed' - seems like it faced the exception during downloading "https://dotnetcli.azureedge.net/dotnet/Sdk/2.0.0-preview3-006785/dotnet-sdk-2.0.0-preview3-006785-win-x64.zip" (I managed to download it locally). then it switched to alt download link "https://dotnetcli.azureedge.net/dotnet/Sdk/2.0.0-preview3-006785/dotnet-dev-win-x64.2.0.0-preview3-006785.zip" - which seems to be not working. @HaoK - maybe we just need to try to re-run the AppVeyor build? |
|
From @MaKCbIMKo on Friday, August 25, 2017 2:41:11 AM @HaoK - did you have a chance to take a look at this PR? |
|
From @HaoK on Friday, August 25, 2017 2:57:31 AM I took a look, but we haven't signed up to do this work yet, so drilling into the details/implementation is a bit premature until we actually are trying to get this feature implemented. |
|
From @MaKCbIMKo on Friday, August 25, 2017 3:01:57 AM Do you mean that this feature is not in roadmap? Can we do anything in order to sign this up? |
|
From @HaoK on Friday, August 25, 2017 3:05:09 AM Yes, feedback/comments in aspnet/Options#227 saying why you want/need this feature will help get it prioritized higher/sooner. |
ghost
locked as resolved and limited conversation to collaborators
From @MaKCbIMKo on Monday, February 20, 2017 9:50:01 AM
This PR introduces the possibility to 'initialize' options at the start up time. It will prevent possible config issues that might occur in runtime because of incorrect config file.
Also, it allows to use initializers to write some custom logic to instantiate options objects.
Copied from original issue: aspnet/Options#171
The text was updated successfully, but these errors were encountered: