Skip to content

Deprecate freeze index API#72618

Merged
danhermann merged 23 commits intoelastic:masterfrom
danhermann:70192_deprecate_freeze_endpoint
May 27, 2021
Merged

Deprecate freeze index API#72618
danhermann merged 23 commits intoelastic:masterfrom
danhermann:70192_deprecate_freeze_endpoint

Conversation

@danhermann
Copy link
Copy Markdown
Contributor

Relates to #70192

@danhermann danhermann added :Data Management/Indices APIs DO NOT USE. Use ":Distributed/Indices APIs" or ":StorageEngine/Templates" instead. >deprecation v8.0.0 v7.14.0 labels May 3, 2021
@danhermann
Copy link
Copy Markdown
Contributor Author

@elasticmachine update branch

---
"Basic":
- skip:
features: [ "allowed_warnings" ]
Copy link
Copy Markdown
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
Copy Markdown
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 (obsolete) DO NOT USE. This team no longer exists. label May 12, 2021
@elasticmachine
Copy link
Copy Markdown
Collaborator

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

@dakrone dakrone self-requested a review May 13, 2021 15:24
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Contributor Author

@elasticmachine update branch

Copy link
Copy Markdown
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
Copy Markdown
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 DO NOT USE. Use ":Distributed/Indices APIs" or ":StorageEngine/Templates" instead. >deprecation Team:Data Management (obsolete) DO NOT USE. This team no longer exists. v7.14.0 v8.0.0-alpha1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants