feat: add support for loading multiple config files and staleness check - #908
feat: add support for loading multiple config files and staleness check#908Elia-Renzoni wants to merge 3 commits into
Conversation
Signed-off-by: Elia Renzoni <elia.renzoni03@gmail.com>
Pull Request Test Coverage Report for Build 20577480854Details
💛 - Coveralls |
Signed-off-by: Elia Renzoni <elia.renzoni03@gmail.com>
|
@Elia-Renzoni thanks so much for this, we haven't forgotten about it. We were already feature-locked and in the release candidate phase for v2.0.0 when this PR was opened. We are targeting this for v2.1.0. I'll review and test it out this week. Thanks again! |
houyuwushang
left a comment
houyuwushang
left a comment
There was a problem hiding this comment.
I rebased these two commits onto current main and traced the directory path through daemon.Hup. The basic case with two existing files still works, but I found several cases that can miss configuration changes or break existing callers. I left the reproductions and affected behavior inline. The main structural issue is that staleness is represented as parallel slices for only the files present at startup, rather than as a snapshot of the directory contents.
| if len(c.Main.configFilesPath) > 0 { | ||
| for index, file := range c.Main.configFilesPath { | ||
| t := c.CheckFileLastModified(file) | ||
| if t.IsZero() { |
There was a problem hiding this comment.
This only walks configFilesPath captured during the initial load. A newly added YAML file is never examined, while a removed file returns a zero timestamp and is skipped here. daemon.Hup calls this method before loading again, so both changes make an explicit reload report "not stale" and leave the running config unchanged. I reproduced both cases after rebasing onto current main. Could this compare a fresh directory snapshot (names and modification times) with the loaded snapshot instead?
4 participants
@jranson @crandles
This PR improves Trickster’s configuration system by extending support from a single configuration file to multiple YAML configuration files loaded from a directory.
The implementation assumes that users who want to use multiple configuration files will provide only the directory path as a flag
(e.g., /etc/trickster) and the code will automatically look for all YAML files in that directory and load them in order, merging the configurations.
To support this feature, some methods were refactored, any feedback on these changes would be appreciated.
Related Issue: #493