1. Allow to use well-known standard secrets.json file for storing a private key… #653
Conversation
… for a local development. 2. Client api application can use appSettings.xxxxx.json and appSettings.yyyyy.json to accomodate various deployment environments, which OKTA's default implementation doesn't accomodate. 3. Finally this extensbility prevents null reference exceptions thrown if client app opts to provide a configuration file path. There are a few things to consider to optimize the code in GetConfigurationOrDefault method: 4. Avoid calling configBuilder.Build() multiple times. Instead, call it once and store the result in a variable, then use that variable to retrieve the sections you need. This will reduce the number of times the configuration is built and improve performance. 5. Consider using AddJsonStream() instead of AddJsonFile() to read the appsettings.json file. This will avoid the overhead of opening and reading the file from disk. 6. Consider removing unnecessary configuration sources, such as AddYamlFile() and AddEnvironmentVariables() - not all applications use them. 7. Implemented the first optimization in the original code section of the GetConfigurationOrDefault method.
|
Hi @mmusaev, Thanks for your contribution ⭐ ! Our team will review it soon. In the meantime, can you please sign the CLA? Check out the contribution guidelines to learn more. Internal Ref: OKTA-637420 |
|
I forwarded the contract to my manager. As soon as I get the response, I will sign CLA. Just got confirmation from my manager. Signed the contract and sent it. Sorry for the delay - the lawyers were busy. :) |
|
Hi @mmusaev, I appreciate your patience. We received your CLA, and will review your PR as soon as possible. In the meantime, can you please update the configuration reference section in the readme with a minimal code sample of how to use your proposed feature? Thank you! |
|
@mmusaev I've invited @rcollette to provide feedback on this PR, since he recently opened a related issue #656 |
docs: update the configuration reference section in the readme with a minimal code sample of how to use my proposed feature
|
@laura-rodriguez Updated README.md as per request with a code sample usage. |
docs: use environmentName for json file
|
@rcollette Please see updated README.md with code sample, the proposed PR (being reviewed by OKTA team) will address your issue and other related issues. |
|
How configuration happens (where it comes from) should not be a concern of this library. Configuration mechanisms should be determined by the application. |
|
@rcollette The extension to the library that I am proposing moves the library specific configuration responsibility to a client app instead of the library. I agree the separation of concerns must be adhered by the authors of the library. |
fix: correct misspelling
… for a local development.
There are a few things to consider optimizing the code in GetConfigurationOrDefault method:
4. Avoid calling configBuilder.Build() multiple times. Instead, call it once and store the result in a variable, then use that variable to retrieve the sections you need. This will reduce the number of times the configuration is built and improve performance.
5. Consider using AddJsonStream() instead of AddJsonFile() to read the appsettings.json file. This will avoid the overhead of opening and reading the file from disk.
6. Consider removing unnecessary configuration sources, such as AddYamlFile() and AddEnvironmentVariables() - not all applications use them.
7. Implemented the first optimization in the original code section of the GetConfigurationOrDefault method.
Summary
Fixes #
Related to #656
Type of PR
Test Information
.NET Version:
Os Version:
Signoff
Obvious Fix