Skip to content

exists template needs a template name#25988

Merged
cbuescher merged 1 commit intoelastic:masterfrom
honzakral:api-fixes
Oct 27, 2017
Merged

exists template needs a template name#25988
cbuescher merged 1 commit intoelastic:masterfrom
honzakral:api-fixes

Conversation

@honzakral
Copy link
Copy Markdown
Contributor

@honzakral honzakral commented Jul 31, 2017

running curl -X HEAD localhost:9200/_template results in 405 Method Not Allowed

@honzakral honzakral added the :Core/Infra/REST API REST infrastructure and utilities label Jul 31, 2017
jasontedor
jasontedor requested changes Aug 1, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

The flat_settings parameter should remain, it is supported on HEAD template requests.

rest-api-spec/src/main/resources/rest-api-spec/api/indices.exists_template.json Outdated
Copy link
Copy Markdown
Member

@jasontedor jasontedor Aug 1, 2017

Choose a reason for hiding this comment

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

flat_settings is supported on HEAD requests; the behavior of GET and HEAD are aligned when both methods are supported.

rest-api-spec/src/main/resources/rest-api-spec/api/indices.exists_template.json Outdated
Copy link
Copy Markdown
Member

@jasontedor jasontedor Aug 1, 2017

Choose a reason for hiding this comment

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

This change is correct.

@honzakral
Copy link
Copy Markdown
Contributor Author

honzakral commented Aug 1, 2017

Thanks for the review, updated the commit. Out of curiosity, what does flat_settings do for HEAD request?

@jasontedor
Copy link
Copy Markdown
Member

jasontedor commented Aug 1, 2017

Out of curiosity, what does flat_settings do for HEAD request?

Per the HTTP specification, when an endpoint supports both the HEAD and GET methods, the response to a HEAD request should be the same as a GET request to the same endpoint (including the same parameters) except that the body should be empty (to be explicit: this means that the Content-Length header should be the same as if a HEAD request had been sent). The way that we implement HEAD requests on endpoints that support them is to treat the request as if it was a GET request, and then before sending the request to the client, strip the body from the response. This means that a parameter like flat_settings on a HEAD request will change the content of the reply that would have come if the method was a GET method, thus changing the content length, thus changing the value of the Content-Length header that is included in the reply to the HEAD request.

jasontedor
jasontedor approved these changes Aug 1, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@jasontedor jasontedor left a comment

Choose a reason for hiding this comment

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

LGTM.

@cbuescher
Copy link
Copy Markdown
Member

cbuescher commented Oct 27, 2017

@honzakral just going through older open PRs, seems like this one is ready to merge. Do you want to go ahead or should we do it?

@honzakral
Copy link
Copy Markdown
Contributor Author

honzakral commented Oct 27, 2017

@cbuescher, please go ahead, I am not up-to-date with all the necessary backporting policies and other steps.

Thank you!

@cbuescher cbuescher merged commit 1dd6aee into elastic:master Oct 27, 2017
@cbuescher
Copy link
Copy Markdown
Member

cbuescher commented Oct 27, 2017

@honzakral not entirely sure how rest-api-specs are used, but I suppose this change should be applied to all active branches down to 5.6?

@cbuescher cbuescher removed the >bug label Oct 27, 2017
@honzakral
Copy link
Copy Markdown
Contributor Author

honzakral commented Oct 27, 2017

I'd say yes, but not sure what is the current status of freezes/branch policies etc. That is why I try to stay away from merging patches to es myself since I am no longer familiar enough with the process...

cbuescher pushed a commit that referenced this pull request Oct 27, 2017
@honzakral @cbuescher
Exists template needs a template name (#25988)
4bacb60
cbuescher pushed a commit that referenced this pull request Oct 27, 2017
@honzakral @cbuescher
Exists template needs a template name (#25988)
119515d
cbuescher pushed a commit that referenced this pull request Oct 27, 2017
@honzakral @cbuescher
Exists template needs a template name (#25988)
dbc7f22
@olcbean olcbean mentioned this pull request Oct 28, 2017
Closed
@lcawl lcawl added v6.0.0-rc2 and removed v6.0.0 labels Oct 30, 2017
jasontedor added a commit to olcbean/elasticsearch that referenced this pull request Oct 30, 2017
@jasontedor
Merge branch 'master' into pr/23767
0744b44 
* master: (63 commits)
  [Docs] Fix note in bucket_selector
  [Docs] Fix indentation of examples (elastic#27168)
  [Docs] Clarify `span_not` query behavior for non-overlapping matches (elastic#27150)
  [Docs] Remove first person "I" from getting started (elastic#27155)
  [Docs] Correct link target for datatype murmur3 (elastic#27143)
  Fix division by zero in phrase suggester that causes assertion to fail
  Enable Docstats with totalSizeInBytes for 6.1.0
  Adds average document size to DocsStats (elastic#27117)
  Upgrade Painless from ANTLR 4.5.1-1 to  ANTLR 4.5.3. (elastic#27153)
  Exists template needs a template name (elastic#25988)
  [Tests] Fix occasional test failure due to two random values being the same
  Fix beidermorse phonetic token filter for unspecified `languageset` (elastic#27112)
  Fix max score tracking with field collapsing (elastic#27122)
  [Doc] Add Ingest CSV Processor Plugin to plugin as a community plugin (elastic#27105)
  Removed the beta tag from cross-cluster search
  fixed typo in ConstructingObjectParse (elastic#27129)
  Allow for the Painless Definition to have multiple instances (elastic#27096)
  Apply missing request options to the expand phase (elastic#27118)
  Only pull SegmentReader once in getSegmentInfo (elastic#27121)
  Fix BWC for discovery stats
  ...
@lcawl lcawl removed the v6.1.0 label Dec 12, 2017
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasontedor jasontedor jasontedor approved these changes

Assignees

No one assigned

Labels

>bug :Core/Infra/REST API REST infrastructure and utilities v5.6.4 v6.0.0-rc2 v7.0.0-beta1

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@honzakral @jasontedor @cbuescher @clintongormley @jimczi @lcawl