-
Notifications
You must be signed in to change notification settings - Fork 25.2k
Remove endpoint for freezing indices #77273
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 endpoint for freezing indices #77273
Conversation
…hermann/elasticsearch into 70192_remove_freeze_endpoint
@@ -1533,24 +1530,6 @@ public void testAnalyze() throws Exception { | |||
assertNotNull(detailsResponse.detail()); | |||
} | |||
|
|||
public void testFreezeAndUnfreeze() throws IOException { |
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.
@dakrone, one of consequences of removing the freeze endpoint is that many tests for unfreeze depend on it. Do you have any concerns about removing tests that exercise the unfreeze endpoint?
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.
Not for these tests specifically, I think it's fine to change the client tests
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.
@jrodewig, it looks like docs for the ES clients in other repos are failing because they reference the now-removed freeze index API? If you can confirm that, I will open PRs in the client repos to remove those references unless there is a better way of doing that.
Pinging @elastic/clients-team (Team:Clients) |
Pinging @elastic/es-data-management (Team:Data Management) |
@danhermann - in prior discussions we decided to let REST API compatibility return a more meaningful error (see #70192 (comment)) if trying to freeze an index. Is that something that should be part of this PR, or a different on e ? cc @joegallo |
Thanks, @jakelandis. I'm happy to add that. Is there any documentation on how that works when an endpoint is completely removed as in this case? |
@elasticmachine update branch |
merge conflict between base and head |
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 for this update @danhermann. I pushed a commit to add some missing redirects. Once the merge conflict is resolved, that should fix the docs CI test.
Other than that, I'd add some more context to the unfreeze API docs. Since we'll still be supporting previously frozen indices (but not freezing new indices), I want to avoid confusion with the frozen data tier.
I'll take another look after that's addressed.
docs/reference/search/search-your-data/search-your-data.asciidoc
Outdated
Show resolved
Hide resolved
docs/reference/search/search-your-data/long-running-searches.asciidoc
Outdated
Show resolved
Hide resolved
Co-authored-by: James Rodewig <[email protected]>
Co-authored-by: James Rodewig <[email protected]>
Co-authored-by: James Rodewig <[email protected]>
…sciidoc Co-authored-by: James Rodewig <[email protected]>
Thanks, @jrodewig. I resolved the merge conflict and incorporated your doc suggestions. I think there is still a problem with the docs build, but I'm not sure what it is. |
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.
Docs LGTM. I pushed one small change to relocate the deprecation admon, but everything else looks good. The docs CI should also be good now.
Thanks @danhermann!
|
||
<<freeze-index-api, Freezes>> an index. | ||
|
||
deprecated[7.x,"The ILM Freeze action was deprecated in 7.x and will be treated as a no-op in 8.0+."] |
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.
Didn't think of this until I saw a related email... should we leave these docs in until the ILM action is actually removed?
That's a good question. I'm not sure there's a lot of value in documenting a no-op action. What do you think @dakrone? |
On one hand, it's nice to be able to see what something in a policy for the lifetime of 8.x does (since it won't be removed until 9.0). On the other hand, we have versioned API docs available, so I am okay to remove it also, since a user can go back to 7.x docs and see what it does. So, I think it's okay to remove, since a user can go back to any 7.x docs and see what it does. Does that sound reasonable to you too @jrodewig? |
That seems reasonable to me. Thanks for the discussion, @danhermann @dakrone. |
@elasticmachine update branch |
Closed in favor of #78918 |
Relates to #70192