Refactored settings handling #3682
Merged
Conversation
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
ramonsmits
force-pushed
the
settings
branch
2 times, most recently
from
efc94c1 to
97783f9
Compare
…ault values on settings type
…the persister specific settings can serialized to JSON for diagnostics output in the REST API
…t.ChangeType` can't convert to TimeSpan and now supports TryRead on on SettingsReader to deal with non-nulleable types
… which has its `PersisterSettings property change from Dictionary to IPersisterSettings. This allows for a persister specific implementation type to be passed as value. This means that in core for propagating settings only the `Settings` object is needed. The `PersistenceFactory` will create a persister specific settings implementation via the resolved persister configuration type if core settings yet. This makes it easy to initialized settings type-safe in tests.
…with optional arguments for these and have the test runner pass explicit values for these removing the need to copy settings runtime values back to ConfigurationManager.AppSettings
# Conflicts: # src/ServiceControl.AcceptanceTests/TestSupport/ServiceControlComponentRunner.cs # src/ServiceControl.Persistence.RavenDb/RavenBootstrapper.cs
…g tab indentation so aligned all files. Removed all references to c:\ and use %TEMP%
…eplacements for slash '/' and dot '.' in key names
DavidBoike
approved these changes
Sep 5, 2023
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.
Type safe settings instead of dictionaries
For the primary instance the settings for SC core and RavenDB persisted were resolved at various places and not only at start. This PR changes how the settings are initialized. The settings are now type-safe and initialized as early as possible.
Non-static settings init for tests
For the unittests this means settings can now easily be constructed type-safe in code as part of test arrange. This removes the need for the tests to propagate setting values to the static ConfigurationManager.AppSettings. No dependency on shared static resources in the tests where values from test A could affect test B based on their order of execution.
Deduple of persisted settings
It also removes the duplication between SC core settings and persisted specific settings. Some application persistence settings are now moved to
PersisterSettingsso that these only exist in one type.TEMP folder
Tests settings no longer use the C: drive but use the TEMP folder for initializing storage folders to store artifacts created as part of running the tests.
Settings object do not have logic
While constructing the core and persisted settings for SC Primary it does not perform validation except for type validation. State validation is handled in the bootstrapper. This makes the settings behave like a DTO which makes is easier to be used for unit testing.