Add support for layered and plugin configurations - #1543
Conversation
This adds support for reading configurations from a three layer hierarchy: 1. User provided roml 2. Install root toml 3. Code defaults Additionally we add some code to support plugin configurations via the ConfigurationLoader. Each plugin can provide a struct with an accompanying id that gets used to parse the scoped section of the TOML.
Code Coverage
|
|
|
||
| public func run() async throws { | ||
| let containerSystemConfig: ContainerSystemConfig = try ConfigurationLoader.load() | ||
| let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() |
There was a problem hiding this comment.
Wouldn't these not get the right environment variables for when the app root and install root are set by the user in system start since system start only sets the environment variables on the APIServer's service?
There was a problem hiding this comment.
Which env var do you mean? Could you give a concrete example for my understanding?
There was a problem hiding this comment.
If a user ran system start --app-root <test app root> --install-root <test install root>, the configuration would be loaded correctly in the system start code and any services created by APIServer. For the other CLI calls where we fallback to the default config files (aka we just call ConfigurationLoader.load()), the base config path for the app root configuration file would first check for the environment variable CONTAINER_APP_ROOT which would not be set in this shell instance (but is set in the APIServer's env), then fallback to the default ~/Library/Application\ Support/com.apple.container app root path (see here). Same for the install root.
| /// - decodeErrorContext: Prefix used in the `invalidArgument` error thrown on decode failure. | ||
| private static func loadAndDecode<T: LoadableConfiguration>( | ||
| _ type: T.Type, | ||
| configurationFiles: [FilePath], |
There was a problem hiding this comment.
Should this just be
configurationFiles: [FilePath] = defaultConfigFiles()
like the other functions?
There was a problem hiding this comment.
We can do that, but the two call sites of this private are using that same defaultConfigFiles logic so the outcome would be unchanged.
| do { | ||
| try providers.append(await FileProvider<TOMLSnapshot>(filePath: path, allowMissing: true)) | ||
| } catch { | ||
| throw ContainerizationError( |
There was a problem hiding this comment.
Looking at this and L155: if none of the files exist, we return an empty config, but if some exist and some don't we throw?
There was a problem hiding this comment.
Right now the logic is:
- If no files exist return
T() - else make a provider for every file
- providers allow for missing files via
allowMissing: true, any missing files will have no change to theConfigReaderview
- providers allow for missing files via
- an exception is thrown in the case that creating a
FileProvidercauses some unexpected exception like malformed text, etc
Adds new entry in NOTICE.md for the swift-configuration-toml dependency added by #1543 Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com>
- This adds support for reading configurations from a three layer hierarchy: 1. User provided TOML 2. Install root TOML 3. Code defaults - We add some code to support plugin configurations via the ConfigurationLoader. Each plugin can provide a struct with an accompanying id that gets used to parse the scoped section of the TOML.
Adds new entry in NOTICE.md for the swift-configuration-toml dependency added by apple#1543 Signed-off-by: Kathryn Baldauf <k_baldauf@apple.com>
3 participants
This adds support for reading configurations from a three layer hierarchy:
Additionally we add some code to support plugin configurations via the ConfigurationLoader. Each plugin can provide a struct with an accompanying id that gets used to parse the scoped section of the TOML.
Type of Change
Motivation and Context
Allows users at install time to configure package widgets defaults without having to recompile
containerTesting