[ILM] Replace legacy ES client with the new client #78416
Conversation
|
@elasticmachine merge upstream |
yuliacech
added
chore
Feature:ILM
release_note:skip
|
Pinging @elastic/es-ui (Team:Elasticsearch UI) |
There was a problem hiding this comment.
Nice job @yuliacech! Thanks for working on this. I did a quick smoke test of ILM and did not notice any regressions. I also assume the API integration tests would have caught any issues.
I left a comment in the code regarding the use of isEsError.
Also, per the migration docs, I think you can migrate the functional tests from using const client = getService('legacyEs'); to const client = getService('es');.
I also just want to confirm this work is intended for 7.10, given FF is today?
| return response.ok(); | ||
| } catch (e) { | ||
| if (lib.isEsError(e)) { | ||
| if (e instanceof ResponseError) { |
There was a problem hiding this comment.
I think there are other instances of lib.isEsError being used throughout the server code - do they need to be updated as well?
|
@elasticmachine merge upstream |
|
@elasticmachine merge upstream |
|
Hi @alisonelizabeth, |
There was a problem hiding this comment.
Latest changes LGTM. Thanks @yuliacech! I did not test again.
There are some changes in how the new client handles/returns ES errors, so further work is needed to update es_ui_shared.isEsError function (#79678).
This is not blocking, but I wonder if it's worth adding support for the way the new client handles errors soon rather than later, and rename the existing is_es_error to is_es_error_legacy (or something to that effect). Once everything is migrated, we could remove the legacy file. Otherwise, it seems like we potentially have to go back and replace if (e.name === 'ResponseError') with if (lib.isEsError(e)) throughout the code, rather than just updating the file path.
|
|
||
| const indexLifecycleDataEnricher = async ( | ||
| indicesList: IndexWithoutIlm[], | ||
| // TODO replace deprecated ES client after Index Management is updated |
There was a problem hiding this comment.
Thanks for adding this TODO! It may also be worth adding a comment to #73973 as a reminder to circle back around to this.
|
@elasticmachine merge upstream |
1 similar comment
|
@elasticmachine merge upstream |
💚 Build SucceededMetrics [docs]distributable file count
History
To update your PR or re-run it, just comment with: |
|
thank you for the review, @alisonelizabeth ! I agree about addressing |
* [ILM] Replace legacy ES client with the new client * [ILM] Fix type check errors * [ILM] Fix api integration test * [ILM] Update error handling and integration test for the new client * [ILM] Fix api integration test * [ILM] Fix api integration test * [ILM] Add a todo comment for Index Management plugin extension Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Summary
Fixes #77906.
This PR removes all usage of
LegacyAPICallerand replaces it withElasticsearchClientin ILM server code. There are some changes in how the new client handles/returns ES errors, so further work is needed to updatees_ui_shared.isEsErrorfunction (#79678).Todo
es_ui_shared.isEsErrorfunction [ES UI Shared] Update isEsError function for new ES client #79678