-
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
Add failure handling for set processors in ML inference pipelines #144654
Add failure handling for set processors in ML inference pipelines #144654
Conversation
Also add a remove processor and text_classification and text_embedding types.
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 tried testing this and I get an error when trying to create the inference pipeline now. Not sure if it's something with my local setup or not though. Here is the error I am seeing:
[2022-11-14T08:56:43.494-06:00][ERROR][plugins.enterpriseSearch] An error occurred while resolving request to http://localhost:5602/internal/enterprise_search/indices/search-test-001/ml_inference/pipeline_processors: Enterprise Search encountered an error.
[2022-11-14T08:56:43.494-06:00][ERROR][plugins.enterpriseSearch] ResponseError: script_exception: [script_exception] Reason: compile error
at KibanaTransport.request (/Users/rodney/Code/elastic/kibana-code-reviews/node_modules/@elastic/transport/lib/Transport.js:476:27)
at runMicrotasks (<anonymous>)
at processTicksAndRejections (node:internal/process/task_queues:96:5)
x-pack/plugins/enterprise_search/common/ml_inference_pipeline/index.test.ts
Outdated
Show resolved
Hide resolved
@TattdCodeMonkey I was able to reproduce this error if the destination field had a hyphen/dash in the name, but it was fine if the field had an underscore or no hyphen/dash. I reproduced this on |
x-pack/plugins/enterprise_search/common/ml_inference_pipeline/index.ts
Outdated
Show resolved
Hide resolved
Should be fixed with the newest commit. |
Fields with dashes would not compile otherwise.
💚 Build Succeeded
Metrics [docs]Async chunks
Unknown metric groupsESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
FF was yesterday, so I added the backport label. This was approved by @efegurkan, but enough has changed since there that I'm looking for another review 👍 |
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, thanks for resolving the bug with hyphens too.
…astic#144654) ## Summary Also, add a `remove` processor and `text_classification` and `text_embedding` types. ### Checklist Delete any items that are not applicable to this PR. - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios (cherry picked from commit 8e81a7d)
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
…es (#144654) (#145472) # Backport This will backport the following commits from `main` to `8.6`: - [Add failure handling for set processors in ML inference pipelines (#144654)](#144654) <!--- Backport version: 8.9.7 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Brian McGue","email":"[email protected]"},"sourceCommit":{"committedDate":"2022-11-16T22:22:14Z","message":"Add failure handling for set processors in ML inference pipelines (#144654)\n\n## Summary\r\n\r\nAlso, add a `remove` processor and `text_classification` and\r\n`text_embedding` types.\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios","sha":"8e81a7d03eb62f116f836460ed8a3eaacfa04cbe","branchLabelMapping":{"^v8.7.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:skip","Team:EnterpriseSearch","v8.6.0","v8.7.0"],"number":144654,"url":"https://github.com/elastic/kibana/pull/144654","mergeCommit":{"message":"Add failure handling for set processors in ML inference pipelines (#144654)\n\n## Summary\r\n\r\nAlso, add a `remove` processor and `text_classification` and\r\n`text_embedding` types.\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios","sha":"8e81a7d03eb62f116f836460ed8a3eaacfa04cbe"}},"sourceBranch":"main","suggestedTargetBranches":["8.6"],"targetPullRequestStates":[{"branch":"8.6","label":"v8.6.0","labelRegex":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"main","label":"v8.7.0","labelRegex":"^v8.7.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/144654","number":144654,"mergeCommit":{"message":"Add failure handling for set processors in ML inference pipelines (#144654)\n\n## Summary\r\n\r\nAlso, add a `remove` processor and `text_classification` and\r\n`text_embedding` types.\r\n\r\n### Checklist\r\n\r\nDelete any items that are not applicable to this PR.\r\n\r\n- [x] [Unit or functional\r\ntests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)\r\nwere updated or added to match the most common scenarios","sha":"8e81a7d03eb62f116f836460ed8a3eaacfa04cbe"}}]}] BACKPORT--> Co-authored-by: Brian McGue <[email protected]>
Summary
Also, add a
remove
processor andtext_classification
andtext_embedding
types.Checklist
Delete any items that are not applicable to this PR.