-
Notifications
You must be signed in to change notification settings - Fork 25.1k
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
Deprecate freeze index API #72618
Deprecate freeze index API #72618
Conversation
@elasticmachine update branch |
@@ -1,5 +1,8 @@ | |||
--- | |||
"Basic": | |||
- skip: | |||
features: [ "allowed_warnings" ] |
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.
Should this be "warnings" instead of "allowed_warnings" ? The difference is that if you only allow, then the warning can be missing and still pass the test. "warnings" is more like "require_warnings"
@elasticmachine update branch |
Pinging @elastic/es-core-features (Team:Core/Features) |
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 a comment, I think we should deprecate both endpoints, even if /_unfreeze
won't be removed until 9.0. Since the message doesn't supply a version this should be okay I think.
What do you think?
Route.builder(POST, "/{index}/_freeze") | ||
.deprecated(DEPRECATION_WARNING, DEPRECATION_VERSION) | ||
.build(), | ||
Route.builder(POST, "/{index}/_unfreeze").build() |
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.
We should also deprecate this endpoint, because it will also technically be going away. If we don't show the message a user could hit this a bunch but not know it's going away.
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 was operating off the deprecation schedule listed in the meta issue in which unfreeze
deprecation happens in 8.0. I'm open to deprecating it earlier though I'd like to get buy-in from at least some others involved in the meta issue.
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.
@zuketo, could you weigh in on the question of deprecating the unfreeze
endpoint now versus in 8.0?
Edit: scratch that. We'll sort it out among the team.
@elasticmachine update branch |
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 assuming CI is happy, thanks for discussing :)
Thanks, @dakrone! |
* master: (1643 commits) Make DataStreamsSnapshotsIT resilient to failures because of local time. (elastic#73516) Upgrade netty to 4.1.63 (elastic#73011) [DOCS] Create a new page for dissect content in scripting docs (elastic#73437) Deprecate freeze index API (elastic#72618) [DOCS] Remove 'closed data stream' reference [DOCS] Update alias references (elastic#73427) [DOCS] Create a new page for grok content in scripting docs (elastic#73118) Remove dependency on azure shadowjar since it's no longer required [DOCS] Update backport policy for known issues (elastic#73489) Shadowed dependencies should be hidden from pom dependencies (elastic#73467) Disable transitive dependencies when resolving bwc JDBC driver artifact (elastic#73448) Print full JVM implementation version at start of build (elastic#73439) [DOCS] Update snapshot/restore for data stream aliases (elastic#73438) Upgrade Azure SDK and Jackson (elastic#72833) (elastic#72995) [DOCS] Fix typo (elastic#73337) (elastic#73474) [DOCS] Fix typo (elastic#73444) (elastic#73472) [DOCS] Update alias security for data stream aliases (elastic#73436) Fix Bug with Concurrent Snapshot and Index Delete (elastic#73456) [DOCS] Move common scripting use cases up a level (elastic#73445) Add more validation for data stream aliases. (elastic#73416) ...
Relates to #70192