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

Get-templates APIs don't support lists #78989

Merged

Conversation

DaveCTurner
Copy link
Contributor

We document that GET /_index_template/... accepts a comma-separated
list of template names but in fact today this API accepts only a single
name or pattern. Likewise GET /_cat/templates/... (at least it didn't
until #78829 but that's not released yet). This commit fixes the docs to
indicate these APIs accept only a single template name and also adds
some extra validation to reject requests containing a , since such a
request cannot match any actual templates.

It also adjusts GET /_cat/templates to use the filtering built into
TransportGetComposableIndexTemplateAction rather than retrieving all
templates and then filtering them on the coordinating node.

We document that `GET /_index_template/...` accepts a comma-separated
list of template names but in fact today this API accepts only a single
name or pattern. Likewise `GET /_cat/templates/...` (at least it didn't
until elastic#78829 but that's not released yet). This commit fixes the docs to
indicate these APIs accept only a single template name and also adds
some extra validation to reject requests containing a `,` since such a
request cannot match any actual templates.

It also adjusts `GET /_cat/templates` to use the filtering built into
`TransportGetComposableIndexTemplateAction` rather than retrieving all
templates and then filtering them on the coordinating node.
@DaveCTurner
Copy link
Contributor Author

I'm in two minds about whether to call this a breaking change or not. It turns a 404 Not Found into a 400 Bad Request but only on requests that are definitely already broken, hardly seems worth documenting.

@DaveCTurner DaveCTurner requested a review from dakrone October 12, 2021 14:05
@DaveCTurner DaveCTurner marked this pull request as ready for review October 12, 2021 14:05
@elasticmachine elasticmachine added the Team:Data Management Meta label for data/management team label Oct 12, 2021
@elasticmachine
Copy link
Collaborator

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

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, thanks David

Comment on lines +97 to +98
version: " - 7.7.99"
reason: "index template v2 API unavailable before 7.8"
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for catching this

@sethmlarson sethmlarson added the Team:Clients Meta label for clients team label Oct 12, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/clients-team (Team:Clients)

Copy link
Contributor

@sethmlarson sethmlarson left a comment

Choose a reason for hiding this comment

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

LGTM from a clients perspective. This type is already captured as a non-list within the Elasticsearch specification as well.

@DaveCTurner DaveCTurner merged commit d2bb6eb into elastic:master Oct 13, 2021
@DaveCTurner DaveCTurner deleted the 2021-10-12-get-template-without-commas branch October 13, 2021 11:13
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Oct 13, 2021
We document that `GET /_index_template/...` accepts a comma-separated
list of template names but in fact today this API accepts only a single
name or pattern. Likewise `GET /_cat/templates/...` (at least it didn't
until elastic#78829 but that's not released yet). This commit fixes the docs to
indicate these APIs accept only a single template name and also adds
some extra validation to reject requests containing a `,` since such a
request cannot match any actual templates.

It also adjusts `GET /_cat/templates` to use the filtering built into
`TransportGetComposableIndexTemplateAction` rather than retrieving all
templates and then filtering them on the coordinating node.

Backport of elastic#78989
elasticsearchmachine pushed a commit that referenced this pull request Oct 13, 2021
We document that `GET /_index_template/...` accepts a comma-separated
list of template names but in fact today this API accepts only a single
name or pattern. Likewise `GET /_cat/templates/...` (at least it didn't
until #78829 but that's not released yet). This commit fixes the docs to
indicate these APIs accept only a single template name and also adds
some extra validation to reject requests containing a `,` since such a
request cannot match any actual templates.

It also adjusts `GET /_cat/templates` to use the filtering built into
`TransportGetComposableIndexTemplateAction` rather than retrieving all
templates and then filtering them on the coordinating node.

Backport of #78989
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>bug :Data Management/Indices APIs APIs to create and manage indices and templates Team:Clients Meta label for clients team Team:Data Management Meta label for data/management team v7.16.0 v8.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants