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

Deprecate freeze index API #72618

Merged
merged 23 commits into from
May 27, 2021

Conversation

danhermann
Copy link
Contributor

Relates to #70192

@danhermann danhermann added :Data Management/Indices APIs APIs to create and manage indices and templates >deprecation v8.0.0 v7.14.0 labels May 3, 2021
@danhermann
Copy link
Contributor Author

@elasticmachine update branch

@@ -1,5 +1,8 @@
---
"Basic":
- skip:
features: [ "allowed_warnings" ]
Copy link
Contributor

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"

@danhermann
Copy link
Contributor Author

@elasticmachine update branch

@danhermann danhermann marked this pull request as ready for review May 12, 2021 16:08
@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label May 12, 2021
@elasticmachine
Copy link
Collaborator

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

@dakrone dakrone self-requested a review May 13, 2021 15:24
Copy link
Member

@dakrone dakrone left a 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()
Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

@danhermann danhermann May 27, 2021

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.

@danhermann
Copy link
Contributor Author

@elasticmachine update branch

Copy link
Member

@dakrone dakrone left a 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 :)

@danhermann
Copy link
Contributor Author

Thanks, @dakrone!

@danhermann danhermann merged commit 40a029f into elastic:master May 27, 2021
@danhermann danhermann deleted the 70192_deprecate_freeze_endpoint branch May 27, 2021 20:14
limingnihao added a commit to limingnihao/elasticsearch that referenced this pull request May 28, 2021
* 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)
  ...
danhermann added a commit to danhermann/elasticsearch that referenced this pull request Jun 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Indices APIs APIs to create and manage indices and templates >deprecation Team:Data Management Meta label for data/management team v7.14.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants