Introduce formal node ML role #45174
Conversation
This commit builds on the ability for plugins to introduce new roles to add a formal node ML role.
|
Pinging @elastic/ml-core |
| public static final Setting<Boolean> ML_ENABLED = | ||
| Setting.boolSetting("node.ml", XPackSettings.MACHINE_LEARNING_ENABLED, Property.NodeScope); | ||
|
|
||
| static DiscoveryNodeRole ML_ROLE = new DiscoveryNodeRole("ml", "l") { |
| protected Setting<Boolean> roleSetting() { | ||
| return ML_ENABLED; | ||
| } | ||
|
|
There was a problem hiding this comment.
nit: Excessive blank line
There was a problem hiding this comment.
We have very little in the way of style agreements, mostly what is enforced by checkstyle and a few loosely-enforced guidelines. We've discussed over the years adding an auto-formatter to avoid style issues once and for all, but we've not had the time to invest in it. Until then, we've mostly left style up to individual developers, although for particularly contentious issues we can have discussions amongst the teams that work in this codebase to see if we want to formalize a rule. Regarding this specific one, our codebase is inconsistently full of white space or not between the opening brace of a class and its body, and the closing brace of a class and its body. My preference is to have this whitespace.
There was a problem hiding this comment.
Thanks for the explanation. +1 for the idea of automated formatting some day...
| // TESTRESPONSE[s/3.07/(\\d+\\.\\d+( \\d+\\.\\d+ (\\d+\\.\\d+)?)?)?/] | ||
| // TESTRESPONSE[s/65 99 42/\\d+ \\d+ \\d+/] | ||
| // TESTRESPONSE[s/[*]/[*]/ s/mJw06l1/.+/ non_json] | ||
| // TESTRESPONSE[s/dim/.+/ s/[*]/[*]/ s/mJw06l1/.+/ non_json] |
There was a problem hiding this comment.
Line 101 of this file has the old mdi ordering.
Also, is it worth adding l to the docs on line 101 or is it deliberate to leave roles added by modules out of the table?
There was a problem hiding this comment.
Let's leave tweaking that to a possible follow-up, as it would include adding voting-only nodes too.
| protected Setting<Boolean> roleSetting() { | ||
| return ML_ENABLED; | ||
| } | ||
|
|
There was a problem hiding this comment.
Thanks for the explanation. +1 for the idea of automated formatting some day...
|
@elasticmachine run elasticsearch-ci/2 |
This commit builds on the ability for plugins to introduce new roles to add a formal node ML role.
|
@droberts195 I missed the discussion on #29943 until I saw that when I merged this issue, it closed that one. About the abbreviation, I want to double check that you're okay with |
I think whether it's |
|
The important thing here is the anagrams. With |
6 participants
This commit builds on the ability for plugins to introduce new roles to add a formal node ML role.
Relates #43175
Closes #29943