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.

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

[ML] File upload: Adds deployment initialization step #198446

Merged

Conversation

jgowdyelastic
Copy link
Member

@jgowdyelastic jgowdyelastic commented Oct 30, 2024

Fixes #196696

When adding a semantic text field, we now have an additional step in the file uploading process which calls inference for the selected inference endpoint.
The response of the inference call is ignored and a poll is started to check to see of the model has been deployed by check to see if num_allocations > 0
Any errors returned from the inference call will stop the upload, unless they are timeout errors which are ignored.

2024-11-05.15-52-49.2024-11-05.15_53_40.mp4

@jgowdyelastic jgowdyelastic marked this pull request as ready for review November 5, 2024 16:17
@jgowdyelastic jgowdyelastic requested a review from a team as a code owner November 5, 2024 16:17
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

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.

Tested and LGTM

}
if (dataViewCreatedStatus === IMPORT_STATUS.COMPLETE) {
completedStep = 5;
completedStep = 6;
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid bumping a completed step for each case in case of similar changes, did you consider defining them as a collection? name might be used for conditions later, without a need of bumping the index

interface ImportProgressStep {
  name: string;
  title: string; // Not sure if that would work, but we could also assign titles to each step 
  isCompleted: () => boolean; // or just assigning a boolean to the prop directly 
}
const steps: ImportProgressStep = [
  {
     name: 'initStep',
     isCompleted: () => reading === true &&
    readStatus === IMPORT_STATUS.INCOMPLETE &&
    parseJSONStatus === IMPORT_STATUS.INCOMPLETE
  },
  ...
]

Copy link
Member Author

@jgowdyelastic jgowdyelastic Nov 11, 2024

Choose a reason for hiding this comment

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

I agree having numbered steps here is not nice at all. This code is old and is going to be replaced in the upcoming file upload redesign. So I didn't want to get into refactoring it now.

path: '/internal/data_visualizer/inference/{inferenceId}',
access: 'internal',
options: {
tags: ['access:fileUpload:analyzeFile'],
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need an additional tag for inference?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think it's needed as es will be checking permissions.
The tags here are just for feature enablement.

@elasticmachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dataVisualizer 732 733 +1

Async chunks

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

id before after diff
dataVisualizer 615.9KB 618.3KB +2.4KB

History

cc @jgowdyelastic

Copy link
Contributor

@darnautov darnautov left a comment

Choose a reason for hiding this comment

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

LGTM

@jgowdyelastic jgowdyelastic merged commit fa6d8ee into elastic:main Nov 12, 2024
24 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11794932613

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 12, 2024
Fixes elastic#196696

When adding a semantic text field, we now have an additional step in the
file uploading process which calls inference for the selected inference
endpoint.
The response of the inference call is ignored and a poll is started to
check to see of the model has been deployed by check to see if
`num_allocations > 0`
Any errors returned from the inference call will stop the upload, unless
they are timeout errors which are ignored.

https://github.com/user-attachments/assets/382ce565-3b4b-47a3-a081-d79c15aa462f
(cherry picked from commit fa6d8ee)
@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 Nov 12, 2024
#199741)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[ML] File upload adding deployment initialization step
(#198446)](#198446)

<!--- 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-11-12T10:02:26Z","message":"[ML]
File upload adding deployment initialization step (#198446)\n\nFixes
https://github.com/elastic/kibana/issues/196696\r\n\r\nWhen adding a
semantic text field, we now have an additional step in the\r\nfile
uploading process which calls inference for the selected
inference\r\nendpoint.\r\nThe response of the inference call is ignored
and a poll is started to\r\ncheck to see of the model has been deployed
by check to see if\r\n`num_allocations > 0`\r\nAny errors returned from
the inference call will stop the upload, unless\r\nthey are timeout
errors which are
ignored.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/382ce565-3b4b-47a3-a081-d79c15aa462f","sha":"fa6d8ee9e0f6fcaa0280fbf5ab60217f9f28279f","branchLabelMapping":{"^v9.0.0$":"main","^v8.17.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:enhancement",":ml","Feature:File
and Index Data Viz","Feature:File
Upload","v9.0.0","backport:version","v8.17.0"],"title":"[ML] File upload
adding deployment initialization
step","number":198446,"url":"https://github.com/elastic/kibana/pull/198446","mergeCommit":{"message":"[ML]
File upload adding deployment initialization step (#198446)\n\nFixes
https://github.com/elastic/kibana/issues/196696\r\n\r\nWhen adding a
semantic text field, we now have an additional step in the\r\nfile
uploading process which calls inference for the selected
inference\r\nendpoint.\r\nThe response of the inference call is ignored
and a poll is started to\r\ncheck to see of the model has been deployed
by check to see if\r\n`num_allocations > 0`\r\nAny errors returned from
the inference call will stop the upload, unless\r\nthey are timeout
errors which are
ignored.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/382ce565-3b4b-47a3-a081-d79c15aa462f","sha":"fa6d8ee9e0f6fcaa0280fbf5ab60217f9f28279f"}},"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/198446","number":198446,"mergeCommit":{"message":"[ML]
File upload adding deployment initialization step (#198446)\n\nFixes
https://github.com/elastic/kibana/issues/196696\r\n\r\nWhen adding a
semantic text field, we now have an additional step in the\r\nfile
uploading process which calls inference for the selected
inference\r\nendpoint.\r\nThe response of the inference call is ignored
and a poll is started to\r\ncheck to see of the model has been deployed
by check to see if\r\n`num_allocations > 0`\r\nAny errors returned from
the inference call will stop the upload, unless\r\nthey are timeout
errors which are
ignored.\r\n\r\n\r\nhttps://github.com/user-attachments/assets/382ce565-3b4b-47a3-a081-d79c15aa462f","sha":"fa6d8ee9e0f6fcaa0280fbf5ab60217f9f28279f"}},{"branch":"8.x","label":"v8.17.0","branchLabelMappingKey":"^v8.17.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: James Gowdy <[email protected]>
tkajtoch pushed a commit to tkajtoch/kibana that referenced this pull request Nov 12, 2024
Fixes elastic#196696

When adding a semantic text field, we now have an additional step in the
file uploading process which calls inference for the selected inference
endpoint.
The response of the inference call is ignored and a poll is started to
check to see of the model has been deployed by check to see if
`num_allocations > 0`
Any errors returned from the inference call will stop the upload, unless
they are timeout errors which are ignored.


https://github.com/user-attachments/assets/382ce565-3b4b-47a3-a081-d79c15aa462f
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Nov 18, 2024
Fixes elastic#196696

When adding a semantic text field, we now have an additional step in the
file uploading process which calls inference for the selected inference
endpoint.
The response of the inference call is ignored and a poll is started to
check to see of the model has been deployed by check to see if
`num_allocations > 0`
Any errors returned from the inference call will stop the upload, unless
they are timeout errors which are ignored.


https://github.com/user-attachments/assets/382ce565-3b4b-47a3-a081-d79c15aa462f
@peteharverson peteharverson changed the title [ML] File upload adding deployment initialization step [ML] File upload: Adds deployment initialization step Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ML] File data visualizer wait for model to be deployed while uploading a file using a semantic text field
5 participants