Expose global retention settings via data stream lifecycle API #112210
Conversation
gmarouli
added
>enhancement
:Data Management/Data streams
|
Documentation preview: |
|
Hi @gmarouli, I've created a changelog YAML for you. |
|
@elasticmachine update branch |
|
@elasticmachine update branch |
|
I will double check the docs failure if it happens again. |
elasticsearchmachine
added
the
Team:Data Management
|
Pinging @elastic/es-data-management (Team:Data Management) |
| public Iterator<ToXContent> toXContentChunked(ToXContent.Params outerParams) { | ||
| return Iterators.concat(Iterators.single((builder, params) -> { | ||
| builder.startObject(); | ||
| if (globalRetention != null) { |
There was a problem hiding this comment.
Do we not want to show empty global configuration if none is configured? I'm trying to think of something analogous in an API we have (to see if it does this, or shows empty) but haven't thought of anything yet.
There was a problem hiding this comment.
I have been going back and forth on this too. In the same when we have no data streams we show an empty list. "data_streams": []. But because this is an object and not a list, I wasn't sure how to approach it.
There was a problem hiding this comment.
Do you think that having global_retention: {} is more descriptive? Like showing that, yes we do support it and it's non set?
There was a problem hiding this comment.
I changed the implementation to reflect this option.
There was a problem hiding this comment.
I'm also on the fence. I have no strong feeling either way. But maybe it's nice having it always present -- that way you know it's not set, rather than that you're just not looking for it in the right place.
There was a problem hiding this comment.
Maybe @dakrone has an opinion or knows of some relevant precedent.
There was a problem hiding this comment.
One argument against this is that now we need to update every yaml rest test that returns a data stream to require the data_stream_global_retention, don't we? Otherwise backwards compatibility test runs will fail.
There was a problem hiding this comment.
Yes, we do not have that many though. I will double check to ensure they are correct.
There was a problem hiding this comment.
While checking on the global retention in the existing get lifecycle tests, I noticed..... they were not even running :(. I fixed that in 15fa8e1.
I also decided to merge the two files into one. It seems to me that since the global retention is part of the response it should be part of the basic tests.
|
@elasticmachine update branch |
|
@elasticmachine update branch |
gmarouli
added
the
auto-merge-without-approval
…ic#112210) In this PR we expose the global retention via the `GET _data_stream/{target}/_lifecycle` API. Since the global retention is a main feature of the data stream lifecycle we chose to expose it by default. ``` GET /_data_stream/my-data-stream/_lifecycle { "global_retention": { "default_retention": "7d", "max_retention": "365d" }, "data_streams": [...] } ```
In this PR we expose the global retention via the
GET _data_stream/{target}/_lifecycleAPI.Since the global retention is a main feature of the data stream lifecycle we chose to expose it by default.