remove old doc placeholder and migrate ilm docs to top-level by talevy · Pull Request #34615 · elastic/elasticsearch

talevy · 2018-10-18T18:53:18Z

Looks like we are restructuring the docs, this migrates ILM docs outside of the x-pack doc structure.

This is related to the initiative to move x-pack cleanup to ESRestTestCase #34530

elasticmachine · 2018-10-18T18:53:20Z

Pinging @elastic/es-core-infra

colings86

LGTM but I'd like @debadair to look over the docs changes to make sure the new structure is in line with the way the docs will be structured going forward

talevy · 2018-10-19T16:16:03Z

thanks for the review @colings86

AthenaEryma · 2018-10-19T16:48:00Z

LGTM as well - and the extra cleanup of ILM will be nice.

nik9000 · 2018-10-19T21:22:23Z

test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java

    }

+    private static void removePoliciesFromAllIndexes() throws IOException {
+        Response response = adminClient().performRequest(new Request("GET", "/_all"));


I think we'll usually have wiped the indices already. Do we need to do this in that case?

oh, you're right. this is unnecessary

nik9000 · 2018-10-19T21:22:45Z

test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java

+            try {
+                adminClient().performRequest(new Request("DELETE", indexName + "/_ilm/"));
+            } catch (Exception e) {
+                // ok


OK because we're racing? Maybe we should add the ignore parameter to the request in that case.

We're really only ok with 404s here, right?

nik9000 · 2018-10-19T21:23:24Z

test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java

+            Response response = adminClient().performRequest(new Request("GET", "/_ilm"));
+            policies = ESRestTestCase.entityAsMap(response);
+        } catch (Exception e) {
+            return;


This seems a bit broad.

nik9000 · 2018-10-19T21:24:00Z

test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java

+
+        try {
+            Response response = adminClient().performRequest(new Request("GET", "/_ilm"));
+            policies = ESRestTestCase.entityAsMap(response);


You are already in ESRestTestCase so you don't need the to reference the class.

nik9000 · 2018-10-19T21:24:11Z

test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java

+            try {
+                adminClient().performRequest(new Request("DELETE", "/_ilm/" + policyName));
+            } catch (Exception e) {
+                // ok


Same comment as before about why this is ok.

talevy · 2018-10-22T18:13:39Z

thank you for the review @nik9000. I will clean up the exception handling and the likes. I will also be pending merging this PR until #34657 goes in so that I can properly modify the version checks for this to work when run in 6.x branches

…-docs

talevy · 2018-10-25T19:54:10Z

test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java

+        try {
+            Response response = adminClient().performRequest(new Request("GET", "/_ilm"));
+            policies = entityAsMap(response);
+        } catch (ResponseException e) {


this is caught here because of #34881

Maybe add a comment referencing the issue and your intention to remove this once it is fixed. That'd be nice for someone reading the code after the fact I think.

talevy · 2018-10-25T21:04:38Z

#34657 was merged into master, so I will begin work to update and version-guard the deletion of policies to version of ES that contain ILM

…-docs

This reverts commit 870655e.

talevy · 2018-10-25T21:47:20Z

I will use 870655e for the backport to 6.x

nik9000

LGTM

nik9000 · 2018-10-25T21:49:41Z

test/framework/src/main/java/org/elasticsearch/test/rest/ESRestTestCase.java

+        try {
+            Response response = adminClient().performRequest(new Request("GET", "/_ilm"));
+            policies = entityAsMap(response);
+        } catch (ResponseException e) {


Maybe add a comment referencing the issue and your intention to remove this once it is fixed. That'd be nice for someone reading the code after the fact I think.

…-docs

talevy · 2018-10-25T23:18:11Z

thanks for the review @nik9000, the other PR was merged, so I will just remove the GET _ilm exception catching

talevy · 2018-10-26T04:17:19Z

test this please

we are restructuring the docs, this migrates ILM docs outside of the x-pack doc structure.

talevy added >docs General docs changes :Data Management/ILM+SLM DO NOT USE. Use ":StorageEngine/ILM" or ":Distributed Coordination/SLM" instead. labels Oct 18, 2018

talevy requested a review from debadair October 18, 2018 18:53

talevy force-pushed the ilm-move-docs branch from bd4b19f to 8fbc187 Compare October 18, 2018 19:05

talevy added 3 commits October 18, 2018 12:43

remove old doc placeholder and migrate ilm docs to top-level

3a7f1f4

add xpack-basic role

a83a464

migrate cleanup of ILM to ESRestTestCase

5f3541f

related elastic#34530.

talevy force-pushed the ilm-move-docs branch from 98e7820 to 5f3541f Compare October 18, 2018 21:55

talevy requested review from AthenaEryma, colings86 and nik9000 October 18, 2018 21:58

colings86 reviewed Oct 19, 2018

View reviewed changes

nik9000 requested changes Oct 19, 2018

View reviewed changes

talevy added 6 commits October 25, 2018 10:40

Merge remote-tracking branch 'upstream/index-lifecycle' into ilm-move…

a3c851a

…-docs

move new start-stop ilm docs to new dir

d8278fa

cleanup exception handling in policyremoval

798abab

fix get_status snippet

03defff

Merge remote-tracking branch 'upstream/index-lifecycle' into ilm-move…

71ac18b

…-docs

fix policy cleanup exception handling

a6b9af8

talevy commented Oct 25, 2018

View reviewed changes

add proper x-pack testenv to start-stop-ilm

3a28734

talevy added 2 commits October 25, 2018 14:36

Merge remote-tracking branch 'upstream/index-lifecycle' into ilm-move…

5bc8f27

…-docs

add version guard for cleanup with nodes < 6.6

870655e

talevy requested review from colings86 and nik9000 October 25, 2018 21:45

Revert "add version guard for cleanup with nodes < 6.6"

020171c

This reverts commit 870655e.

nik9000 approved these changes Oct 25, 2018

View reviewed changes

talevy added 2 commits October 25, 2018 15:29

Merge remote-tracking branch 'upstream/index-lifecycle' into ilm-move…

5fd6da2

…-docs

Merge remote-tracking branch 'upstream/index-lifecycle' into ilm-move…

4403e3f

…-docs

remove exception handling around GET _ilm

c64b5b7

add back exception handling for GET if ILM is not enabled

c3538ab

talevy merged commit e737ea7 into elastic:index-lifecycle Oct 26, 2018

talevy deleted the ilm-move-docs branch October 26, 2018 19:19

talevy added a commit that referenced this pull request Oct 26, 2018

remove old doc placeholder and migrate ilm docs to top-level (#34615)

7d101c6

we are restructuring the docs, this migrates ILM docs outside of the x-pack doc structure.

Conversation

talevy commented Oct 18, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticmachine commented Oct 18, 2018

Uh oh!

colings86 left a comment

Choose a reason for hiding this comment

Uh oh!

talevy commented Oct 19, 2018

Uh oh!

AthenaEryma commented Oct 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

talevy commented Oct 22, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

talevy commented Oct 25, 2018

Uh oh!

talevy commented Oct 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nik9000 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

talevy commented Oct 25, 2018

Uh oh!

talevy commented Oct 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

talevy commented Oct 18, 2018 •

edited

Loading

talevy commented Oct 25, 2018 •

edited

Loading