-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
feat: add conf load api #11180
feat: add conf load api #11180
Conversation
2985e14
to
f04aa2b
Compare
@@ -208,15 +207,6 @@ schema_module() -> | |||
Value -> list_to_existing_atom(Value) | |||
end. | |||
|
|||
check_config(Mod, Raw) -> |
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.
move this function to emqx_conf_cli
{error, Reason} -> | ||
emqx_ctl:warning("load ~ts failed~n~p~n", [Path, Reason]), | ||
{error, bad_hocon_file} | ||
end; | ||
load_config(Bin, ReplaceOrMerge) when is_binary(Bin) -> |
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.
load from binary is for HTTP API.
@@ -186,37 +191,47 @@ get_config(Key) -> | |||
end. | |||
|
|||
-define(OPTIONS, #{rawconf_with_defaults => true, override_to => cluster}). | |||
load_config(Path, ReplaceOrMerge) -> | |||
load_config(Path, ReplaceOrMerge) when is_list(Path) -> |
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.
load from file is for CLI
parameters => [ | ||
{node, |
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.
origin GET /configs
is unused, So I think incompatibility here is accepted.
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.
I suggest that it should be kept, even if no one is using it?
Then, we can add a check on the HTTP Header of the request. If the Accept
header is set to application/json
or is not specified, we will return the response according to the original logic. If the Accept
header is set to text/plain
, we will return the response in hocon file format.
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.
Okay, I'll change it back right away, although personally I really dislike doing so.
5461160
to
0b68782
Compare
0b68782
to
d509c47
Compare
Pull Request Test Coverage Report for Build 5451451624
💛 - Coveralls |
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.
LGTM
Fixes https://emqx.atlassian.net/browse/EMQX-9180
Summary
🤖 Generated by Copilot at c07db9d
This pull request improves the configuration management features of emqx by enhancing the
emqx_conf_cli
module and its related modules, adding a new configuration API that supports binary input and output, and updating the i18n file and the swagger documentation accordingly. It also removes some unused code and fixes some error handling cases.PR Checklist
Please convert it to a draft if any of the following conditions are not met. Reviewers may skip over until all the items are checked:
changes/{ce,ee}/(feat|perf|fix)-<PR-id>.en.md
filesChecklist for CI (.github/workflows) changes
changes/
dir for user-facing artifacts update