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.

Sign up for GitHub

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

Jump to bottom

[ML] Fix file upload with no ingest pipeline #193744

Merged
merged 5 commits into from
Sep 25, 2024
Merged

[ML] Fix file upload with no ingest pipeline #193744

merged 5 commits into from
Sep 25, 2024

Conversation

jgowdyelastic
Copy link
Member

@jgowdyelastic jgowdyelastic commented Sep 23, 2024 •
edited by kibanamachine
Loading

With some datasets the find structure api will not generate an ingest pipeline. A recent change to how we catch and display errors during file upload means an upload with no pipeline now produces an error which aborts the upload.
Previously all pipeline creation errors were ignored and hidden from the user.

This PR changes changes the file upload endpoint to allow it to receive no ingest pipeline and also changes the UI to not display the pipeline creation step during upload.

This file can be used to test the fix.
https://github.com/elastic/eland/blob/main/tests/flights.json.gz

@jgowdyelastic
Copy link
Member Author

/ci

@jgowdyelastic jgowdyelastic marked this pull request as ready for review September 24, 2024 11:52
@jgowdyelastic jgowdyelastic requested review from a team as code owners September 24, 2024 11:52
@jgowdyelastic jgowdyelastic self-assigned this Sep 25, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dataVisualizer 726.5KB 726.6KB +93.0B
fileUpload 951.4KB 951.4KB +18.0B
total +111.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
fileUpload 14.8KB 14.8KB +8.0B

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @jgowdyelastic

@peteharverson peteharverson mentioned this pull request Sep 25, 2024
Closed
23 tasks
peteharverson
peteharverson approved these changes Sep 25, 2024
View reviewed changes
Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Tested with the flights data set and confirmed it is uploaded successfully with this change. Great to see the new functional test.

walterra
walterra approved these changes Sep 25, 2024
View reviewed changes
Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code LGTM. 👍

nreese
nreese approved these changes Sep 25, 2024
View reviewed changes
Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

kibana-presentation changes LGTM
code review only

@jgowdyelastic jgowdyelastic merged commit ee1a147 into elastic:main Sep 25, 2024
19 of 21 checks passed
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 25, 2024
@jgowdyelastic
[ML] Fix file upload with no ingest pipeline (elastic#193744)
1291765 
With some datasets the find structure api will not generate an ingest
pipeline. A recent
[change](elastic#186956) to how we catch
and display errors during file upload means an upload with no pipeline
now produces an error which aborts the upload.
Previously all pipeline creation errors were ignored and hidden from the
user.

This PR changes changes the file upload endpoint to allow it to receive
no ingest pipeline and also changes the UI to not display the pipeline
creation step during upload.

This file can be used to test the fix.
https://github.com/elastic/eland/blob/main/tests/flights.json.gz

(cherry picked from commit ee1a147)
@kibanamachine kibanamachine mentioned this pull request Sep 25, 2024
Merged
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
✅ 8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Sep 25, 2024
@kibanamachine @jgowdyelastic
[8.x] [ML] Fix file upload with no ingest pipeline (#193744) (#194019)
603f51f 
# Backport

This will backport the following commits from `main` to `8.x`:
- [[ML] Fix file upload with no ingest pipeline
(#193744)](#193744)

<!--- Backport version: 9.4.3 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sqren/backport)

<!--BACKPORT [{"author":{"name":"James
Gowdy","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-09-25T14:30:30Z","message":"[ML]
Fix file upload with no ingest pipeline (#193744)\n\nWith some datasets
the find structure api will not generate an ingest\r\npipeline. A
recent\r\n[change](#186956) to how
we catch\r\nand display errors during file upload means an upload with
no pipeline\r\nnow produces an error which aborts the
upload.\r\nPreviously all pipeline creation errors were ignored and
hidden from the\r\nuser.\r\n\r\nThis PR changes changes the file upload
endpoint to allow it to receive\r\nno ingest pipeline and also changes
the UI to not display the pipeline\r\ncreation step during
upload.\r\n\r\nThis file can be used to test the
fix.\r\nhttps://github.com/elastic/eland/blob/main/tests/flights.json.gz","sha":"ee1a147baca52dca5703663d35b66e7c44f3b676","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix",":ml","Feature:File
and Index Data Viz","Feature:File
Upload","v9.0.0","v8.16.0"],"title":"[ML] Fix file upload with no ingest
pipeline","number":193744,"url":"https://github.com/elastic/kibana/pull/193744","mergeCommit":{"message":"[ML]
Fix file upload with no ingest pipeline (#193744)\n\nWith some datasets
the find structure api will not generate an ingest\r\npipeline. A
recent\r\n[change](#186956) to how
we catch\r\nand display errors during file upload means an upload with
no pipeline\r\nnow produces an error which aborts the
upload.\r\nPreviously all pipeline creation errors were ignored and
hidden from the\r\nuser.\r\n\r\nThis PR changes changes the file upload
endpoint to allow it to receive\r\nno ingest pipeline and also changes
the UI to not display the pipeline\r\ncreation step during
upload.\r\n\r\nThis file can be used to test the
fix.\r\nhttps://github.com/elastic/eland/blob/main/tests/flights.json.gz","sha":"ee1a147baca52dca5703663d35b66e7c44f3b676"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/193744","number":193744,"mergeCommit":{"message":"[ML]
Fix file upload with no ingest pipeline (#193744)\n\nWith some datasets
the find structure api will not generate an ingest\r\npipeline. A
recent\r\n[change](#186956) to how
we catch\r\nand display errors during file upload means an upload with
no pipeline\r\nnow produces an error which aborts the
upload.\r\nPreviously all pipeline creation errors were ignored and
hidden from the\r\nuser.\r\n\r\nThis PR changes changes the file upload
endpoint to allow it to receive\r\nno ingest pipeline and also changes
the UI to not display the pipeline\r\ncreation step during
upload.\r\n\r\nThis file can be used to test the
fix.\r\nhttps://github.com/elastic/eland/blob/main/tests/flights.json.gz","sha":"ee1a147baca52dca5703663d35b66e7c44f3b676"}},{"branch":"8.x","label":"v8.16.0","branchLabelMappingKey":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: James Gowdy <[email protected]>
@pquentin pquentin mentioned this pull request Sep 26, 2024
Open
@jgowdyelastic jgowdyelastic mentioned this pull request Sep 27, 2024
Merged
jgowdyelastic added a commit that referenced this pull request Sep 27, 2024
@jgowdyelastic
[ML] Fix file upload when pipline is used (#194273)
6fd1913 
Fixes bug introduced in #193744

If a file is uploaded with an ingest pipeline, the pipeline is created,
but not used when uploading the data.

Adds tests to check that the data exists in the index and the expected
fields exist and are populated.
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Sep 27, 2024
@jgowdyelastic
[ML] Fix file upload when pipline is used (elastic#194273)
3c1cc85 
Fixes bug introduced in elastic#193744

If a file is uploaded with an ingest pipeline, the pipeline is created,
but not used when uploading the data.

Adds tests to check that the data exists in the index and the expected
fields exist and are populated.

(cherry picked from commit 6fd1913)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peteharverson peteharverson peteharverson approved these changes

@nreese nreese nreese approved these changes

@walterra walterra walterra approved these changes

Assignees

@jgowdyelastic jgowdyelastic

Labels
Feature:File and Index Data Viz ML file and index data visualizer Feature:File Upload :ml release_note:fix v8.16.0 v9.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@jgowdyelastic @elasticmachine @kibana-ci @kibanamachine @walterra @nreese @peteharverson