-
Notifications
You must be signed in to change notification settings - Fork 25k
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
GET data stream API returns additional information #59128
Conversation
This adds the data stream's index template, the configured ILM policy (if any) and the health status of the data stream to the GET _data_stream response.
Pinging @elastic/es-core-features (:Core/Features/Data streams) |
@elasticmachine update branch |
Added WIP as the HLRC still needs some work |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left one comment about REST tests that I think should be addressed. LGTM, otherwise.
Edit: Just saw you marked it WIP. I'll take another look at it when you've completed it.
- match: { data_streams.0.name: simple-data-stream1 } | ||
- match: { data_streams.0.timestamp_field.name: '@timestamp' } | ||
- match: { data_streams.0.generation: 1 } | ||
- length: { data_streams.0.indices: 1 } | ||
- match: { data_streams.0.indices.0.index_name: '.ds-simple-data-stream1-000001' } | ||
- match: { data_streams.1.name: simple-data-stream2 } | ||
- match: { data_streams.1.timestamp_field.name: '@timestamp2' } | ||
- match: { data_streams.0.generation: 1 } | ||
- length: { data_streams.1.indices: 1 } | ||
- match: { data_streams.1.indices.0.index_name: '.ds-simple-data-stream2-000001' } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that at least one of these REST tests should be updated with the full list of fields that are returned from the get data stream API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Dan, pushed a match for status
and template
(the lifecycle policy is tested in TimeseriesDataStreamIT
as it's an xpack setting)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks, @andreidan!
Restoring a data stream from a snapshot could install a data stream that doesn't match any composable templates. This makes the index template field in the `GET _data_stream` response optional.
This adds the data stream's index template, the configured ILM policy (if any) and the health status of the data stream to the GET _data_stream response. Restoring a data stream from a snapshot could install a data stream that doesn't match any composable templates. This also makes the `template` field in the `GET _data_stream` response optional. (cherry picked from commit 0d9c98a) Signed-off-by: Andrei Dan <[email protected]>
@@ -119,11 +214,13 @@ public void writeTo(StreamOutput out) throws IOException { | |||
|
|||
@Override | |||
public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { | |||
builder.startArray(); | |||
for (DataStream dataStream : dataStreams) { | |||
builder.startObject(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why was this top level data_streams
field introduced? I don't think that this is needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It did make it easier to verify in REST tests that the number of returned data streams was as expected with a - length: { data_streams: 0 }
directive. I don't think the length directive otherwise provides a way to refer to the root level of a response.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the - length:
directive should be improved to handle this? Now the response format is changed, because a test directive can't handle something.
I also don't see length directive being used on the data_streams
object, so maybe just keep using:
- match: { $body: [] }
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those are fair points. 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Went of the top array field to be consistent with component and composable templates eg.
Line 146 in 3515909
builder.startArray(COMPONENT_TEMPLATES.getPreferredName()); |
…9177) * GET data stream API returns additional information (#59128) This adds the data stream's index template, the configured ILM policy (if any) and the health status of the data stream to the GET _data_stream response. Restoring a data stream from a snapshot could install a data stream that doesn't match any composable templates. This also makes the `template` field in the `GET _data_stream` response optional. (cherry picked from commit 0d9c98a) Signed-off-by: Andrei Dan <[email protected]>
Updates docs and snippets for changes made to the get data stream API with PR #59128.
This adds the data stream's index template, the configured ILM policy
(if any) and the health status of the data stream to the
GET _data_stream
response.
Relates to #58316
Relates to #53100
The proposed response looks like this: