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

Cluster state and CRUD operations for data streams #53877

Merged
merged 9 commits into from
Mar 24, 2020

Conversation

danhermann
Copy link
Contributor

Adds data streams to cluster state and implements their create, delete, and get operations.

Relates to #53100.

@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-features (:Core/Features/Data streams)

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

Looks good. I left a number of comments.

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

Looking good. Left some test comments.

}

static List<DataStream> getDataStreams(ClusterState clusterState, Request request) {
Map<String, DataStream> dataStreams = clusterState.metaData().dataStreams();
Copy link
Member

Choose a reason for hiding this comment

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

unit test?
Also I think this should throw a resource not found exception if a specific data stream doesn't exist?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems there are different existing behaviors when attempting to retrieve a specific resource that does not exist. E.g., aliases and indices return a 404 and index templates and ingest pipelines return an empty list. I don't if those are intentional behavioral differences, but I can certainly return a 404 if that's appropriate for data streams.

Copy link
Member

Choose a reason for hiding this comment

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

I think that returning a 404 in case of requesting a specific data stream is appropriate.

@danhermann danhermann marked this pull request as ready for review March 23, 2020 17:58
@danhermann
Copy link
Contributor Author

@elasticmachine update branch

@martijnvg
Copy link
Member

@elasticmachine update branch

Copy link
Member

@martijnvg martijnvg left a comment

Choose a reason for hiding this comment

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

LGTM

@danhermann danhermann merged commit aed8ce7 into elastic:master Mar 24, 2020
@danhermann danhermann deleted the data_streams_crud branch March 24, 2020 11:08
danhermann added a commit to danhermann/elasticsearch that referenced this pull request Mar 24, 2020
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.

4 participants