[ML] Add some ML config classes to protocol library #32502

droberts195 · 2018-07-31T14:28:26Z

This commit adds four ML config classes to the X-Pack protocol
library used by the high level REST client.

(Other commits will add the remaining config classes, plus results
and stats classes.)

These classes:

Are immutable
Have little/no validation of field values beyond null checks
Are convertible to and from X-Content, but NOT wire transportable
Have lenient parsers to maximize compatibility across versions
Have the same class names, member names and getter/setter names
as the corresponding classes in X-Pack core to ease migration
for transport client users
Don't reproduce all the methods that do calculations or
transformations that the the corresponding classes in X-Pack core
have

This commit adds four ML config classes to the X-Pack protocol library used by the high level REST client. (Other commits will add the remaining config classes, plus results and stats classes.) These classes: - Are immutable - Have little/no validation of field values beyond null checks - Are convertible to and from X-Content, but NOT wire transportable - Have lenient parsers to maximize compatibility across versions - Have the same class names, member names and getter/setter names as the corresponding classes in X-Pack core to ease migration for transport client users - Don't reproduce all the methods that do calculations or transformations that the the corresponding classes in X-Pack core have

elasticmachine · 2018-07-31T14:28:29Z

Pinging @elastic/ml-core

droberts195 · 2018-07-31T14:29:08Z

This is labelled >non-issue as it's useless on its own. It's groundwork for future changes that will make ML endpoints available in the HLRC.

davidkyle

I left a comment on the comments.

davidkyle · 2018-08-02T10:27:42Z

...c/main/java/org/elasticsearch/protocol/xpack/ml/job/config/CategorizationAnalyzerConfig.java

+    }
+
+    /**
+     * Parse a <code>categorization_analyzer</code> from configuration or cluster state.  A custom parser is needed


Nit: This is just a parser for configuration no need to reference cluster state. And remove the comment "The parser is strict when parsing config and lenient when parsing cluster state." below as the ignoreUnknownFields parameter hasn't been moved.

dimitris-athanasiou · 2018-08-02T11:30:51Z

...k/protocol/src/main/java/org/elasticsearch/protocol/xpack/ml/job/config/ModelPlotConfig.java

+    private final boolean enabled;
+    private final String terms;
+
+    public ModelPlotConfig() {


This 'ctor exists in the backend only for testing really. I do not think it's a helpful constructor for library users. If anything, it's probably a bit cryptic. I think we could live without it (and the test for it) in the client.

benwtrent · 2018-08-02T13:14:24Z

...c/main/java/org/elasticsearch/protocol/xpack/ml/job/config/CategorizationAnalyzerConfig.java

+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;


(o instanceof NameOrDefinition == false) would be more consistent with all the other equals methods.

EDIT: never mind. The fight for equality symmetry continues :)

Yes, it's a complex problem in the general case. If you ask IntelliJ to generate the equals() method for you it even asks which option to use, so that pretty much proves there's no global best practice. There's a thorough discussion in http://www.angelikalanger.com/Articles/JavaSolutions/SecretsOfEquals/Equals.html

For final classes the choice between getClass() and instanceof is not important, and there's no reason for NameOrDefinition to be overridden, so I'll make it final.

dimitris-athanasiou

LGTM

This commit adds four ML config classes to the X-Pack protocol library used by the high level REST client. (Other commits will add the remaining config classes, plus results and stats classes.) These classes: - Are immutable - Have little/no validation of field values beyond null checks - Are convertible to and from X-Content, but NOT wire transportable - Have lenient parsers to maximize compatibility across versions - Have the same class names, member names and getter/setter names as the corresponding classes in X-Pack core to ease migration for transport client users - Don't reproduce all the methods that do calculations or transformations that the the corresponding classes in X-Pack core have

…pe-detection-with-leading-whitespace * elastic/master: (34 commits) Cross-cluster search: preserve cluster alias in shard failures (elastic#32608) Handle AlreadyClosedException when bumping primary term [TEST] Allow to run in FIPS JVM (elastic#32607) [Test] Add ckb to the list of unsupported languages (elastic#32611) SCRIPTING: Move Aggregation Scripts to their own context (elastic#32068) Painless: Use LocalMethod Map For Lookup at Runtime (elastic#32599) [TEST] Enhance failure message when bulk updates have failures [ML] Add ML result classes to protocol library (elastic#32587) Suppress LicensingDocumentationIT.testPutLicense in release builds (elastic#32613) [Rollup] Update wire version check after backport Suppress Wildfly test in FIPS JVMs (elastic#32543) [Rollup] Improve ID scheme for rollup documents (elastic#32558) ingest: doc: move Dot Expander Processor doc to correct position (elastic#31743) [ML] Add some ML config classes to protocol library (elastic#32502) [TEST]Split transport verification mode none tests (elastic#32488) Core: Move helper date formatters over to java time (elastic#32504) [Rollup] Remove builders from DateHistogramGroupConfig (elastic#32555) [TEST} unmutes SearchAsyncActionTests and adds debugging info [ML] Add Detector config classes to protocol library (elastic#32495) [Rollup] Remove builders from MetricConfig (elastic#32536) ...

* 6.x: [Kerberos] Use canonical host name (#32588) Cross-cluster search: preserve cluster alias in shard failures (#32608) [TEST] Allow to run in FIPS JVM (#32607) Handle AlreadyClosedException when bumping primary term [Test] Add ckb to the list of unsupported languages (#32611) SCRIPTING: Move Aggregation Scripts to their own context (#32068) (#32629) [TEST] Enhance failure message when bulk updates have failures [ML] Add ML result classes to protocol library (#32587) Suppress LicensingDocumentationIT.testPutLicense in release builds (#32613) [Rollup] Improve ID scheme for rollup documents (#32558) Mutes failing SQL string function tests due to #32589 Suppress Wildfly test in FIPS JVMs (#32543) Add cluster UUID to Cluster Stats API response (#32206) [ML] Add some ML config classes to protocol library (#32502) [TEST]Split transport verification mode none tests (#32488) [Rollup] Remove builders from DateHistogramGroupConfig (#32555) [ML] Add Detector config classes to protocol library (#32495) [Rollup] Remove builders from MetricConfig (#32536) Fix race between replica reset and primary promotion (#32442) HLRC: Move commercial clients from XPackClient (#32596) Security: move User to protocol project (#32367) Minor fix for javadoc (applicable for java 11). (#32573) Painless: Move Some Lookup Logic to PainlessLookup (#32565) Core: Minor size reduction for AbstractComponent (#32509) INGEST: Enable default pipelines (#32286) (#32591) TEST: Avoid merges in testSeqNoAndCheckpoints [Rollup] Remove builders from HistoGroupConfig (#32533) fixed elements in array of produced terms (#32519) Mutes ReindexFailureTests.searchFailure dues to #28053 Mutes LicensingDocumentationIT due to #32580 Remove the SATA controller from OpenSUSE box [ML] Rename JobProvider to JobResultsProvider (#32551)

* master: Cross-cluster search: preserve cluster alias in shard failures (#32608) Handle AlreadyClosedException when bumping primary term [TEST] Allow to run in FIPS JVM (#32607) [Test] Add ckb to the list of unsupported languages (#32611) SCRIPTING: Move Aggregation Scripts to their own context (#32068) Painless: Use LocalMethod Map For Lookup at Runtime (#32599) [TEST] Enhance failure message when bulk updates have failures [ML] Add ML result classes to protocol library (#32587) Suppress LicensingDocumentationIT.testPutLicense in release builds (#32613) [Rollup] Update wire version check after backport Suppress Wildfly test in FIPS JVMs (#32543) [Rollup] Improve ID scheme for rollup documents (#32558) ingest: doc: move Dot Expander Processor doc to correct position (#31743) [ML] Add some ML config classes to protocol library (#32502) [TEST]Split transport verification mode none tests (#32488) Core: Move helper date formatters over to java time (#32504) [Rollup] Remove builders from DateHistogramGroupConfig (#32555) [TEST} unmutes SearchAsyncActionTests and adds debugging info [ML] Add Detector config classes to protocol library (#32495) [Rollup] Remove builders from MetricConfig (#32536) Tests: Add rolling upgrade tests for watcher (#32428) Fix race between replica reset and primary promotion (#32442)

droberts195 added >non-issue review v7.0.0 :ml Machine learning v6.5.0 labels Jul 31, 2018

davidkyle reviewed Aug 2, 2018

View reviewed changes

dimitris-athanasiou reviewed Aug 2, 2018

View reviewed changes

benwtrent reviewed Aug 2, 2018

View reviewed changes

droberts195 added 2 commits August 2, 2018 15:41

Address review comments

73fd0ec

Merge branch 'master' into ml_configs_in_protocol_library

0c8a04d

dimitris-athanasiou approved these changes Aug 2, 2018

View reviewed changes

Merge branch 'master' into ml_configs_in_protocol_library

9782523

droberts195 merged commit bc274b2 into elastic:master Aug 3, 2018

droberts195 deleted the ml_configs_in_protocol_library branch August 3, 2018 12:21

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] Add some ML config classes to protocol library #32502

[ML] Add some ML config classes to protocol library #32502

droberts195 commented Jul 31, 2018

elasticmachine commented Jul 31, 2018

droberts195 commented Jul 31, 2018

davidkyle left a comment

davidkyle Aug 2, 2018

dimitris-athanasiou Aug 2, 2018

benwtrent Aug 2, 2018 •

edited

Loading

droberts195 Aug 2, 2018

dimitris-athanasiou left a comment

[ML] Add some ML config classes to protocol library #32502

[ML] Add some ML config classes to protocol library #32502

Conversation

droberts195 commented Jul 31, 2018

elasticmachine commented Jul 31, 2018

droberts195 commented Jul 31, 2018

davidkyle left a comment

Choose a reason for hiding this comment

davidkyle Aug 2, 2018

Choose a reason for hiding this comment

dimitris-athanasiou Aug 2, 2018

Choose a reason for hiding this comment

benwtrent Aug 2, 2018 • edited Loading

Choose a reason for hiding this comment

droberts195 Aug 2, 2018

Choose a reason for hiding this comment

dimitris-athanasiou left a comment

Choose a reason for hiding this comment

benwtrent Aug 2, 2018 •

edited

Loading