-
Notifications
You must be signed in to change notification settings - Fork 1.4k
config: fix duplicate config file loading in config_include_dir #5228
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
3215bf9 to
06b7380
Compare
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
| end | ||
| end | ||
|
|
||
| @on_file_parsed&.call(File.expand_path(File.join(@include_basepath, @fname))) if root_element |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
File.join(@include_basepath, @fname) returns an absolute path,
however it doesn't include the drive letter on Windows systems.
It uses File.expand_path together to get the full absolute path.
kenhys
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just brief review.
| # reload | ||
| supervisor.__send__(:reload_config) | ||
| reload_elements = supervisor.instance_variable_get(:@conf).elements | ||
| assert_equal(elements, reload_elements) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might be better to check log message too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added tests to check log message.
d12af91
|
|
||
| assert_equal('source', elements[3].name) | ||
| assert_equal('obsolete_plugins', elements[3]['@type']) | ||
| end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Signed-off-by: Shizuo Fujita <fujita@clear-code.com>
Which issue(s) this PR fixes:
Fixes #
Ref. #5100
What this PR does / why we need it:
Currently, if a user explicitly
@includes a file that is also locatedin
config_include_dir, the file is loaded twice.This causes startup failures due to port conflicts or ID duplication.
This commit introduces a deduplication mechanism using a callback:
on_file_parsedcallback toFluent::Config.buildto track files loaded during the user config parsing phase.Supervisor, record loaded files via this callback.config_include_dir, skip files that have already been loaded in the user configuration.Docs Changes:
Release Note: