[ML] move find file structure to a new API endpoint #67123

benwtrent · 2021-01-06T19:58:37Z

This introduces a new text-structure plugin. This is the new home of the find file structure API.

The old REST URL is still available but is deprecated.

The new URL is: _text_structure/find_structure. All parameters and behavior are unchanged.

Changes to the high-level REST client and docs will be in separate commit.

related to: #67001

benwtrent

Notes for reviewer:

I have verified that Kibana file upload still works fine. I think the deprecation headers are simply being dropped by the kibana server
The VAST majority of the changes are simple file moves + formatting updates (formatting is enforced in the new text-structure plugin).
I am planning on high level client changes in an other PR as those could be considered "breaking" (didn't want to mark this PR as breaking, and the churn is big enough already).
Doc changes will also come later

benwtrent · 2021-01-06T20:05:08Z

client/rest-high-level/src/test/java/org/elasticsearch/client/MachineLearningIT.java

@@ -2895,8 +2895,17 @@ public void testFindFileStructure() throws IOException {
        FindFileStructureRequest request = new FindFileStructureRequest();
        request.setSample(sample.getBytes(StandardCharsets.UTF_8));

-        FindFileStructureResponse response =
-            execute(request, machineLearningClient::findFileStructure, machineLearningClient::findFileStructureAsync);
+        FindFileStructureResponse response = execute(


This will probably be removed once the high level client is updated

benwtrent · 2021-01-06T20:05:12Z

...high-level/src/test/java/org/elasticsearch/client/documentation/MlClientDocumentationIT.java

@@ -1870,8 +1870,17 @@ public void testFindFileStructure() throws Exception {
            // end::find-file-structure-request-options

            // tag::find-file-structure-execute
-            FindFileStructureResponse findFileStructureResponse =
-                client.machineLearning().findFileStructure(findFileStructureRequest, RequestOptions.DEFAULT);
+            FindFileStructureResponse findFileStructureResponse = client


This will probably be removed once the high level client is updated

benwtrent · 2021-01-06T20:06:24Z

...n/ml/src/test/java/org/elasticsearch/xpack/ml/filestructurefinder/FileStructureTestCase.java

@@ -1,89 +0,0 @@
-/*


While this shows only a delete, it is effectively a file move (it is added later in the diff). The issue is, I think, that the formatting changes (enforced by our spotless settings), caused github to not recognize it as a simple file rename.

benwtrent · 2021-01-06T20:06:58Z

x-pack/plugin/src/test/resources/rest-api-spec/api/text_structure.find_structure.json

@@ -1,5 +1,5 @@
 {
-  "ml.find_file_structure":{
+  "text_structure.find_structure":{


opted to not keep the old yaml definition (with deprecation) as the API was experimental.

benwtrent · 2021-01-06T20:08:14Z

.../text-structure/src/main/java/org/elasticsearch/xpack/textstructure/TextStructurePlugin.java

+        ScalingExecutorBuilder utility = new ScalingExecutorBuilder(
+            UTILITY_THREAD_POOL_NAME,
+            1,
+            256,


This number has no significance. I am willing to reduce as it does seem rather high. I am not sure how many of these calls we expect to run in parallel on the same node. Maybe 16?

Yes, maybe this plugin shouldn't have its own thread pool at all, but reuse the "generic" one from core. Several other plugins do this, for example monitoring, ilm, slm, and parts of security. Since we expect this API to be pretty infrequently called I think it's reasonable to do the same here.

@droberts195 reusing generic seems OK with me. Will take a look.

elasticmachine · 2021-01-06T21:24:40Z

Pinging @elastic/ml-core (:ml)

droberts195

As you pointed out, the main outstanding decision is which thread pool to get the threads from.

The REST spec still says it's experimental, but this can be changed at the same time as the docs.

droberts195 · 2021-01-07T14:04:25Z

.../text-structure/src/main/java/org/elasticsearch/xpack/textstructure/TextStructurePlugin.java

+        ScalingExecutorBuilder utility = new ScalingExecutorBuilder(
+            UTILITY_THREAD_POOL_NAME,
+            1,
+            256,


Yes, maybe this plugin shouldn't have its own thread pool at all, but reuse the "generic" one from core. Several other plugins do this, for example monitoring, ilm, slm, and parts of security. Since we expect this API to be pretty infrequently called I think it's reasonable to do the same here.

benwtrent · 2021-01-07T15:48:26Z

@elasticmachine update branch

…plugin

benwtrent · 2021-01-07T17:45:00Z

run elasticsearch-ci/default-distro

droberts195

LGTM

This introduces a new `text-structure` plugin. This is the new home of the find file structure API. The old REST URL is still available but is deprecated. The new URL is: `_text_structure/find_structure`. All parameters and behavior are unchanged. Changes to the high-level REST client and docs will be in separate commit. related to: elastic#67001

…67251) * [ML] move find file structure to a new API endpoint (#67123) This introduces a new `text-structure` plugin. This is the new home of the find file structure API. The old REST URL is still available but is deprecated. The new URL is: `_text_structure/find_structure`. All parameters and behavior are unchanged. Changes to the high-level REST client and docs will be in separate commit. related to: #67001

benwtrent added >enhancement :ml Machine learning v8.0.0 v7.12.0 labels Jan 6, 2021

benwtrent force-pushed the feature/move-file-structure-finder-to-new-plugin branch from 6c64954 to 8288cb2 Compare January 6, 2021 20:04

benwtrent commented Jan 6, 2021

View reviewed changes

[ML] refactor file structure finder to its own plugin

168f930

benwtrent force-pushed the feature/move-file-structure-finder-to-new-plugin branch from 8288cb2 to 168f930 Compare January 6, 2021 20:29

benwtrent marked this pull request as ready for review January 6, 2021 21:24

benwtrent requested a review from droberts195 January 6, 2021 21:24

droberts195 reviewed Jan 7, 2021

View reviewed changes

move to use the generic thread pool

7f6abb4

benwtrent requested a review from droberts195 January 7, 2021 15:34

Merge branch 'master' into feature/move-file-structure-finder-to-new-…

8c0d773

…plugin

droberts195 approved these changes Jan 7, 2021

View reviewed changes

benwtrent merged commit af179ab into elastic:master Jan 11, 2021

benwtrent deleted the feature/move-file-structure-finder-to-new-plugin branch January 11, 2021 13:56

benwtrent mentioned this pull request Jan 11, 2021

[7.x] [ML] move find file structure to a new API endpoint (#67123) #67251

Merged

peteharverson mentioned this pull request Jan 12, 2021

[ML] Switch file data visualizer to use new text-structure plugin for find file structure API elastic/kibana#88010

Closed

spalger mentioned this pull request Feb 16, 2021

[ml] remove use of deprecated /_ml/find_file_structure API elastic/kibana#91569

Closed

jgowdyelastic mentioned this pull request Feb 18, 2021

[ML] Switching to new find file structure endpoint elastic/kibana#91802

Merged

1 task

stevejgordon mentioned this pull request Feb 22, 2021

7.12.0 Meta Ticket elastic/elasticsearch-net#5337

Closed

34 tasks

joegallo mentioned this pull request Mar 2, 2021

Remove the _ml/find-file-structure docs #69823

Merged

jgowdyelastic mentioned this pull request Mar 5, 2021

ES client : use the new type definitions elastic/kibana#83808

Merged

jakelandis added v8.0.0-alpha1 and removed v8.0.0 labels Jul 26, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ML] move find file structure to a new API endpoint #67123

[ML] move find file structure to a new API endpoint #67123

benwtrent commented Jan 6, 2021

benwtrent left a comment

benwtrent Jan 6, 2021

benwtrent Jan 6, 2021

benwtrent Jan 6, 2021

benwtrent Jan 6, 2021

benwtrent Jan 6, 2021

droberts195 Jan 7, 2021

benwtrent Jan 7, 2021

elasticmachine commented Jan 6, 2021

droberts195 left a comment

droberts195 Jan 7, 2021

benwtrent commented Jan 7, 2021

benwtrent commented Jan 7, 2021

droberts195 left a comment

[ML] move find file structure to a new API endpoint #67123

[ML] move find file structure to a new API endpoint #67123

Conversation

benwtrent commented Jan 6, 2021

benwtrent left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elasticmachine commented Jan 6, 2021

droberts195 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benwtrent commented Jan 7, 2021

benwtrent commented Jan 7, 2021

droberts195 left a comment

Choose a reason for hiding this comment