-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Console] Add Kibana APIs Support #128562
Conversation
@jen-huang @legrego @kobelb @jfsiii @wylieconlon I made some changes as per the comments on #99102. Could you please review this functionality again and provide your feedback? Thank you in advance! |
@elasticmachine merge upstream |
1 similar comment
@elasticmachine merge upstream |
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.
I did a quick pass (test only, no code review), and my initial concerns all look resolved - nice work! I tested in a couple of different spaces, and it appears that this is respecting the current space, which is perfect.
I briefly tested "Copy as cURL", and that doesn't seem to copy anything to my clipboard.
One nit: It would be nice if we could support both GET kbn:api/...
and GET kbn:/api/...
. Adding the leading /
results in a 404
response.
@legrego thank you for testing this out! I updated the PR with your suggestion for supporting leading |
Pinging @elastic/platform-deployment-management (Team:Deployment Management) |
0a1ba2d
to
72581f5
Compare
src/plugins/console/public/application/models/sense_editor/sense_editor.ts
Outdated
Show resolved
Hide resolved
src/plugins/console/public/application/models/sense_editor/sense_editor.ts
Outdated
Show resolved
Hide resolved
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.
Latest changes lgtm, tested locally. Thanks a ton for working on this @mibragimov! Left a tiny nit, but feel free to ignore it 🚀
Thanks for the review @yuliacech @sabarasaba ! |
💛 Build succeeded, but was flakyTest Failures
Metrics [docs]Async chunks
History
To update your PR or re-run it, just comment with: cc @mibragimov |
This was reverted with f41dc1f. on-merge CI stated failing at https://buildkite.com/elastic/kibana-on-merge/builds/15267 |
This reverts commit f41dc1f.
Fixed and reinstated in #130816 |
Liked the idea, but what about large "clustered" systems and Cross cluster search? how would you define "kbn" for another instance of Elastic/Kibana? Should it have provision to define by IP/endpoint URL/port too?
|
Thanks for the suggestion @getkub! In the past we've discussed and decided against enabling Console to send requests to different clusters. Relevant issues: |
* Add support for Kibana API requests * Fix failing tests * Support leading / and minor refactor * Resolve conflicts * Update send_request.test file * Refactor * Add functional test * Address comments * Fix typo * Resolve conflicts and refactor error handling * [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix' * Address comments * Resolve merge conflicts * Rename KIBANA_API_KEYWORD Co-authored-by: Muhammad Ibragimov <[email protected]> Co-authored-by: kibanamachine <[email protected]>
This reverts commit 502a00b.
* Revert "Revert "[Console] Add Kibana APIs Support (elastic#128562)"" This reverts commit f41dc1f. * fix functional test
Closes #49330
This PR continues the work from #99102.
Completed tasks
Testing
To test this out, enter a request against a Kibana API, but prefix the path with
kbn:
, to help the Console API differentiate between ES API and Kibana API requests. For example:_Originally posted by @cjcenizal in #99102
Release Note
Dev Tools Console now supports sending requests to Kibana APIs: prepend any Kibana API endpoint with
kbn:
and send the request via Console, for example,GET kbn:/api/index_management/indices
.Screenshots