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

Adds missing doc details for ML get calendar APIs #852

Merged
merged 5 commits into from
Nov 4, 2021
Merged

Adds missing doc details for ML get calendar APIs #852

merged 5 commits into from
Nov 4, 2021

Conversation

lcawl
Copy link
Contributor

@lcawl lcawl commented Oct 7, 2021

@sethmlarson
Copy link
Contributor

@lcawl We just recently added a gate on GET or HEAD with body under the assumption that all APIs in Elasticsearch with a GET+body also allowed POST, seems our assumption wasn't correct! We'll likely have to revert or make an exception to #831.

droberts195 added a commit to droberts195/elasticsearch that referenced this pull request Oct 11, 2021
The docs for the get scheduled events endpoint stated that the
arguments could be sent in a request body instead of in URL
arguments. The spec was inconsistent with this.

Additionally, GET endpoints that allow a request body need to
support POST too, for the benefit of clients that do not support
GETs with bodies.

This change adds the option to POST a get scheduled events
request and adjusts the spec to permit this.

Relates to elastic/elasticsearch-specification#852
@sethmlarson
Copy link
Contributor

This is blocked on elastic/elasticsearch#79271

@lcawl
Copy link
Contributor Author

lcawl commented Oct 18, 2021

@sethmlarson per your discussion in elastic/elasticsearch#78923, it seems like we'll need to add an exception ( re #831) for these APIs. Is that feasible?

@lcawl lcawl marked this pull request as ready for review November 3, 2021 17:26
@lcawl lcawl removed the on hold label Nov 3, 2021
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.

Left two comments about the request body of ml.get_calendars

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

@sethmlarson sethmlarson merged commit 67f2f14 into elastic:main Nov 4, 2021
@lcawl lcawl deleted the docs-get-calendars branch November 4, 2021 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants