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

Remove the index type from internal watcher indexes #39761

Merged
merged 3 commits into from
Mar 8, 2019

Conversation

jakelandis
Copy link
Contributor

This commit removes the "doc" type from watcher internal indexes.
The template still carries the "_doc" type since that is needed for
the internal representation.

This impacts the .watches, .triggered-watches, and .watch-history indexes.

External consumers do not need any changes since all external calls
go through the _watcher API, and should not interact with the the .index directly.

Relates #38637

This commit removes the "doc" type from watcher internal indexes.
The template still carries the "_doc" type since that is needed for
the internal representation.

This impacts the .watches, .triggered-watches, and .watch-history indexes.

External consumers do not need any changes since all external calls
go through the _watcher API, and should not interact with the the .index directly.

Relates elastic#38637
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features

@@ -29,7 +29,7 @@

public static final String TYPE = "index";

@Nullable final String docType;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The changes to this file (IndexAction) were missed in #37594. I added it here to help us find what to remove when we completely get rid of types. Apologizes for conflating the two issues.

Copy link
Contributor

@jpountz jpountz left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -69,7 +69,7 @@ public void testWatchNotFound() {
String watchId = "my_watch_id";
doAnswer(invocation -> {
ActionListener<GetResponse> listener = (ActionListener<GetResponse>) invocation.getArguments()[1];
listener.onResponse(new GetResponse(new GetResult(Watch.INDEX, Watch.DOC_TYPE, watchId, UNASSIGNED_SEQ_NO, 0, -1, false,
listener.onResponse(new GetResponse(new GetResult(Watch.INDEX, null, watchId, UNASSIGNED_SEQ_NO, 0, -1, false,
Copy link
Contributor

Choose a reason for hiding this comment

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

the response would have _doc as a type in practice

Copy link
Contributor

Choose a reason for hiding this comment

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

++ to changing that to use default mapping instead

Copy link
Contributor

@hub-cap hub-cap left a comment

Choose a reason for hiding this comment

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

Pending just the one nit that @jpountz mentioned, im also approving this.

@@ -69,7 +69,7 @@ public void testWatchNotFound() {
String watchId = "my_watch_id";
doAnswer(invocation -> {
ActionListener<GetResponse> listener = (ActionListener<GetResponse>) invocation.getArguments()[1];
listener.onResponse(new GetResponse(new GetResult(Watch.INDEX, Watch.DOC_TYPE, watchId, UNASSIGNED_SEQ_NO, 0, -1, false,
listener.onResponse(new GetResponse(new GetResult(Watch.INDEX, null, watchId, UNASSIGNED_SEQ_NO, 0, -1, false,
Copy link
Contributor

Choose a reason for hiding this comment

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

++ to changing that to use default mapping instead

@jakelandis
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/1

1 similar comment
@jakelandis
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/1

@jakelandis jakelandis merged commit 4f5ca36 into elastic:master Mar 8, 2019
jakelandis added a commit to jakelandis/elasticsearch that referenced this pull request Mar 8, 2019
This commit removes the "doc" type from watcher internal indexes.
The template still carries the "_doc" type since that is needed for
the internal representation.

This impacts the .watches, .triggered-watches, and .watch-history indexes.

External consumers do not need any changes since all external calls
go through the _watcher API, and should not interact with the the .index directly.

Relates elastic#38637
jakelandis added a commit to jakelandis/elasticsearch that referenced this pull request Mar 8, 2019
This commit removes the "doc" type from watcher internal indexes.
The template still carries the "_doc" type since that is needed for
the internal representation.

This impacts the .watches, .triggered-watches, and .watch-history indexes.

External consumers do not need any changes since all external calls
go through the _watcher API, and should not interact with the the .index directly.

Relates elastic#38637
jakelandis added a commit that referenced this pull request Mar 8, 2019
This commit removes the "doc" type from watcher internal indexes.
The template still carries the "_doc" type since that is needed for
the internal representation.

This impacts the .watches, .triggered-watches, and .watch-history indexes.

External consumers do not need any changes since all external calls
go through the _watcher API, and should not interact with the the .index directly.

Relates #38637
jakelandis added a commit that referenced this pull request Mar 8, 2019
This commit removes the "doc" type from watcher internal indexes.
The template still carries the "_doc" type since that is needed for
the internal representation.

This impacts the .watches, .triggered-watches, and .watch-history indexes.

External consumers do not need any changes since all external calls
go through the _watcher API, and should not interact with the the .index directly.

Relates #38637
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.

None yet

5 participants