Skip to content

Conversation

@joegallo
Copy link
Contributor

Follow up to #78918, replaces #74705

The squash merge commit in this middle of this PR is just #79859, you can ignore it here and review it there.

#78918 removed the _freeze endpoint entirely, which means we respond to _freeze requests with a 400 Bad Request and a response body like {"error":"no handler found for uri [/tweets-1/_freeze] and method [POST]"} -- that is, the standard Elasticsearch "you've got the wrong method and/or url" response.

That's definitely the right treatment for a normal request, but if the client specifies compatible-with=7 on their request, then we should have a better response -- I've gone with 410 Gone and:

{
  "error" : {
    "root_cause" : [
      {
        "type" : "unsupported_operation_exception",
        "reason" : "It is no longer possible to freeze indices, but existing frozen indices can still be unfrozen"
      }
    ],
    "type" : "unsupported_operation_exception",
    "reason" : "It is no longer possible to freeze indices, but existing frozen indices can still be unfrozen"
  },
  "status" : 410
}

We just always respond with a 400 bad request (even if you ask to
freeze an index that doesn't actually exist).
404 on an index that doesn't exist, 410 on one that does exist.
@joegallo joegallo added >non-issue :Data Management/Indices APIs APIs to create and manage indices and templates v8.0.0 labels Oct 26, 2021
@joegallo joegallo requested a review from jakelandis October 26, 2021 18:03
@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Oct 26, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-data-management (Team:Data Management)

On 7.16 it fails with a 400 bad request because we don't allow this
operation (semantically), on master it was failing with a 400 bad
request because we removed the freeze endpoint entirely, so it
happened to pass. Now that I have _freeze requests responding with 410
gone, this test is now failing, so we have to skip it.

This surprised me a little at first, but I really do think this is all
correct.
Copy link
Contributor

@jakelandis jakelandis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@joegallo joegallo merged commit a780594 into elastic:master Oct 26, 2021
@joegallo joegallo deleted the freeze-rest-compatibility branch October 26, 2021 21:12
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 >non-issue Team:Data Management Meta label for data/management team v8.0.0-beta1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants