Skip to content
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

[ML] Parse JSON format config updates #1682

Merged
merged 4 commits into from
Jan 21, 2021

Conversation

edsavage
Copy link
Contributor

Expect configuration update requests to be received in JSON formatted
strings. Remove support for old, ini file parsing.

Relates elastic/elasticsearch#67721, #1253

Expect configuration update requests to be received in JSON formatted
strings. Remove support for old, ini file parsing.
Copy link
Contributor

@droberts195 droberts195 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

The only two comments are minor nits

Comment on lines 25 to 26
LOG_ERROR(<< "An error occurred while parsing pattern set from JSON: " +
std::string(rapidjson::GetParseError_En(doc.GetParseError())));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
LOG_ERROR(<< "An error occurred while parsing pattern set from JSON: " +
std::string(rapidjson::GetParseError_En(doc.GetParseError())));
LOG_ERROR(<< "An error occurred while parsing pattern set from JSON: "
<< rapidjson::GetParseError_En(doc.GetParseError()));

@@ -24,13 +24,13 @@ namespace api {
//! update, a control message is being sent with the requested
//! configuration changes. This class is responsible for parsing
//! text with the requested configuration changes and apply them.
//! The changes are expected in an ini type of syntax.
//! The changes are expected in an JSON formatted syntax.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
//! The changes are expected in an JSON formatted syntax.
//! The changes are expected in a JSON document.

@edsavage edsavage merged commit 1914109 into elastic:master Jan 21, 2021
edsavage added a commit to edsavage/ml-cpp that referenced this pull request Jan 21, 2021
* Expect configuration update requests to be received in JSON formatted
strings. 
* Remove support for old, ini file parsing.

Relates elastic/elasticsearch#67721, elastic#1253
@edsavage edsavage deleted the autodetect_control_messages_json branch January 21, 2021 10:11
edsavage added a commit that referenced this pull request Jan 21, 2021
* Expect configuration update requests to be received in JSON formatted
strings. 
* Remove support for old, ini file parsing.

Relates elastic/elasticsearch#67721, #1253
Backports #1682
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants