Support loading configuration for plugins (DB-624)#4130
Merged
pvanbuijtene merged 1 commit intomasterfrom Jan 29, 2024
Merged
Conversation
2709b34 to
dd3d10d
Compare
RagingKore
reviewed
Jan 29, 2024
| bool optional = false, | ||
| bool reloadOnChange = false) { | ||
|
|
||
| if (!Locations.TryLocateConfigFile(configFilePath, out var directory, out var fileName)) { |
Contributor
There was a problem hiding this comment.
The function hides this custom behaviour that tries to locate the files in yet another path.
We should consider passing the path to the function upfront.
Contributor
Author
There was a problem hiding this comment.
yeeeah it's from the previous behviour that checks for kestrelsettings etc in two places. i'd prefer to leave that for now
| $"Could not find {configFilePath} in the following directories: {string.Join(", ", Locations.GetPotentialConfigurationDirectories())}"); | ||
| } | ||
|
|
||
| builder.AddJsonFile(config => { |
Contributor
There was a problem hiding this comment.
Could we instead pass just the filename, optional and reloadOnChange parameters?
Contributor
Author
There was a problem hiding this comment.
im not sure, i've kept it the same way as it was being called before
We now load the whole configuration (ClusterVNodeOptions, Metrics, Kestrel, Logging, and any additional config json files) into a single IConfiguration object that we add to the DI and will (shortly) make available to the plugins in ConfigureServices The json files are loaded into the root of the IConfiguration and are expected to put their values in the right sections. For historical reasons the metrics config is an exception to this and is loaded into the section EventStore:Metrics Additional config files are any json files in the following two directories: - DefaultConfigurationDirectory/config - ApplicationDirectory/config We use the `config` subdirectory in each case to ensure that we don't load other json files that are not rooted config files (*deps.json, metrics etc) Additional config can also be set via - command line: --eventstore:plugins:key=value - environment: EventStore__Plugins__Key
RagingKore
approved these changes
Jan 29, 2024
841ffad to
436c443
Compare
pvanbuijtene
approved these changes
Jan 29, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Added: General support for plugin configuration
We now load the whole configuration (ClusterVNodeOptions, Metrics, Kestrel,
Logging, and any additional config json files) into a single IConfiguration object that we
add to the DI and will (shortly) make available to the plugins in ConfigureServices
The json files are loaded into the root of the IConfiguration and are expected
to put their values in the right sections. For historical reasons the metrics
config is an exception to this and is loaded into the section EventStore:Metrics
Additional config files are any json files in the following two directories:
We use the
configsubdirectory in each case to ensure that we don't loadother json files that are not rooted config files (*deps.json, metrics etc)
Additional config can also be set via
Limitations: ClusterVNodeOptions cannot yet be configured in the additional json files. This is because ClusterVNodeOptions is not yet resolved out of the main IConfiguration object. (Edit: this was addressed in #4144)