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
[Synthetics] Global params Public APIs #169669
Conversation
Documentation preview: |
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
Pinging @elastic/uptime (Team:uptime) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Trying to follow the docs, I can't interact with the API. It looks like the original ~/internal/synthetics/params
path is still in place to edit the params. This should correspond to the docs, if I'm understanding the intention here correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary
Functionality LGTM - I have some concerns about the docs, a couple are nit but I think we need to have another pass on these to make sure we're giving people the best explanation of using the API that we can give.
Concerns
There are comments detailing the specific places in the diff I think we can improve.
- The docs for Edit do not seem to reflect the reality of using the API for editing params. It specifies that we can include a param's ID in the path, and it shows this in an example. When I try that I get a 404. If I include the ID in the body of the request, the API behaves in the expected manner.
- Let's include example request code for
POST
andDELETE
like we have for thePUT
route. - (Optional) I think it might be worth mentioning that
kbn-xsrf
header is required? That can be a real head scratcher as it's specific to Kibana, unlike something likeContent-Type
header which is standard. If we don't include it in other docs though, I guess we don't have to here.
Testing log
- created some params, example:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!!
💛 Build succeeded, but was flaky
Failed CI StepsMetrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: |
Summary
fixes #167048
Part of #169547
Global params Public APIs !!
Local testing
set this config to set default version, production already sets it.
server.versioned.versionResolution: oldest
Following API have been made public
Docs can be tested locally by
node scripts/docs --open