-
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
[Obs AI Assistant] fix knowledge base installation state #206130
[Obs AI Assistant] fix knowledge base installation state #206130
Conversation
setIsPopoverOpen(false); | ||
|
||
await knowledgeBase.install().then(() => { | ||
setCheckForInstallStatus(false); |
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.
don't stop checking kb status after /setup
has returned. model may not be ready.
x-pack/platform/packages/shared/kbn-ai-assistant/src/chat/welcome_message_knowledge_base.tsx
Outdated
Show resolved
Hide resolved
Pinging @elastic/obs-ai-assistant (Team:Obs AI Assistant) |
@@ -100,69 +117,72 @@ export function WelcomeMessageKnowledgeBase({ | |||
</> | |||
) : null} | |||
|
|||
{connectors.connectors?.length ? ( |
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.
moved check to parent. what case would we want to mount this component before setting up a connector? can't think of one.
...ared/observability_solution/observability_ai_assistant/server/routes/knowledge_base/route.ts
Show resolved
Hide resolved
x-pack/platform/packages/shared/kbn-ai-assistant/src/chat/welcome_message_knowledge_base.tsx
Outdated
Show resolved
Hide resolved
...k/platform/packages/shared/kbn-ai-assistant/src/chat/welcome_message_knowledge_base.test.tsx
Outdated
Show resolved
Hide resolved
...k/platform/packages/shared/kbn-ai-assistant/src/chat/welcome_message_knowledge_base.test.tsx
Outdated
Show resolved
Hide resolved
...k/platform/packages/shared/kbn-ai-assistant/src/chat/welcome_message_knowledge_base.test.tsx
Outdated
Show resolved
Hide resolved
expect(screen.getByText('Install Knowledge base', { exact: false })).toBeInTheDocument(); | ||
expect(screen.getByText('Inspect issues', { exact: false })).toBeInTheDocument(); | ||
}); | ||
}); |
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.
Thanks for adding a very thorough test!
…ome_message_knowledge_base.test.tsx Co-authored-by: Søren Louv-Jansen <[email protected]>
…ome_message_knowledge_base.test.tsx Co-authored-by: Søren Louv-Jansen <[email protected]>
…ome_message_knowledge_base.test.tsx Co-authored-by: Søren Louv-Jansen <[email protected]>
…ome_message_knowledge_base.tsx Co-authored-by: Søren Louv-Jansen <[email protected]>
💛 Build succeeded, but was flaky
Failed CI StepsTest Failures
Metrics [docs]Async chunks
History
|
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!
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
/oblt-deploy |
Starting backport for target branches: 8.17, 8.x |
) Resolves elastic#205970 Updates logic to account for knowledge base `/setup` no longer polling for model readiness before returning. - Currently we only poll `/status` if user manually installs the knowledge base. In cases where we auto installed, such as after successfully setting up a connector, we depended on `/setup` to poll internally. Since the latter was removed, we need to always poll `/status`, otherwise user could potentially be in the state where `setup` has finished (considered installed) but `status` still reports not ready and we show the install message again (see screenshots in elastic#205970) - Currently if an install is in progress and user closes the flyout, the progress state is lost. These changes should continue to reflect the installation progress in the UI. - Renames variables and adds comments for easier readability - adds unit test to component that handles the install UI state, `WelcomeMessageKnowledgeBase` --------- Co-authored-by: Søren Louv-Jansen <[email protected]> (cherry picked from commit 06526fe)
💔 Some backports could not be created
Note: Successful backport PRs will be merged automatically after passing CI. Manual backportTo create the backport manually run:
Questions ?Please refer to the Backport tool documentation |
…) (#206777) # Backport This will backport the following commits from `main` to `8.x`: - [[Obs AI Assistant] fix knowledge base installation state (#206130)](#206130) <!--- Backport version: 9.4.3 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport) <!--BACKPORT [{"author":{"name":"Sandra G","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-15T14:38:07Z","message":"[Obs AI Assistant] fix knowledge base installation state (#206130)\n\nResolves https://github.com/elastic/kibana/issues/205970\r\n\r\nUpdates logic to account for knowledge base `/setup` no longer polling\r\nfor model readiness before returning.\r\n\r\n- Currently we only poll `/status` if user manually installs the\r\nknowledge base. In cases where we auto installed, such as after\r\nsuccessfully setting up a connector, we depended on `/setup` to poll\r\ninternally. Since the latter was removed, we need to always poll\r\n`/status`, otherwise user could potentially be in the state where\r\n`setup` has finished (considered installed) but `status` still reports\r\nnot ready and we show the install message again (see screenshots in\r\nhttps://github.com//issues/205970)\r\n- Currently if an install is in progress and user closes the flyout, the\r\nprogress state is lost. These changes should continue to reflect the\r\ninstallation progress in the UI.\r\n- Renames variables and adds comments for easier readability\r\n- adds unit test to component that handles the install UI state,\r\n`WelcomeMessageKnowledgeBase`\r\n\r\n---------\r\n\r\nCo-authored-by: Søren Louv-Jansen <[email protected]>","sha":"06526fe928ac98580e6fe02768b3c403184694f1","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","ci:cloud-deploy","Team:Obs AI Assistant","ci:project-deploy-elasticsearch","ci:project-deploy-observability","backport:version","v8.18.0","v8.17.1"],"title":"[Obs AI Assistant] fix knowledge base installation state","number":206130,"url":"https://github.com/elastic/kibana/pull/206130","mergeCommit":{"message":"[Obs AI Assistant] fix knowledge base installation state (#206130)\n\nResolves https://github.com/elastic/kibana/issues/205970\r\n\r\nUpdates logic to account for knowledge base `/setup` no longer polling\r\nfor model readiness before returning.\r\n\r\n- Currently we only poll `/status` if user manually installs the\r\nknowledge base. In cases where we auto installed, such as after\r\nsuccessfully setting up a connector, we depended on `/setup` to poll\r\ninternally. Since the latter was removed, we need to always poll\r\n`/status`, otherwise user could potentially be in the state where\r\n`setup` has finished (considered installed) but `status` still reports\r\nnot ready and we show the install message again (see screenshots in\r\nhttps://github.com//issues/205970)\r\n- Currently if an install is in progress and user closes the flyout, the\r\nprogress state is lost. These changes should continue to reflect the\r\ninstallation progress in the UI.\r\n- Renames variables and adds comments for easier readability\r\n- adds unit test to component that handles the install UI state,\r\n`WelcomeMessageKnowledgeBase`\r\n\r\n---------\r\n\r\nCo-authored-by: Søren Louv-Jansen <[email protected]>","sha":"06526fe928ac98580e6fe02768b3c403184694f1"}},"sourceBranch":"main","suggestedTargetBranches":["8.x","8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/206130","number":206130,"mergeCommit":{"message":"[Obs AI Assistant] fix knowledge base installation state (#206130)\n\nResolves https://github.com/elastic/kibana/issues/205970\r\n\r\nUpdates logic to account for knowledge base `/setup` no longer polling\r\nfor model readiness before returning.\r\n\r\n- Currently we only poll `/status` if user manually installs the\r\nknowledge base. In cases where we auto installed, such as after\r\nsuccessfully setting up a connector, we depended on `/setup` to poll\r\ninternally. Since the latter was removed, we need to always poll\r\n`/status`, otherwise user could potentially be in the state where\r\n`setup` has finished (considered installed) but `status` still reports\r\nnot ready and we show the install message again (see screenshots in\r\nhttps://github.com//issues/205970)\r\n- Currently if an install is in progress and user closes the flyout, the\r\nprogress state is lost. These changes should continue to reflect the\r\ninstallation progress in the UI.\r\n- Renames variables and adds comments for easier readability\r\n- adds unit test to component that handles the install UI state,\r\n`WelcomeMessageKnowledgeBase`\r\n\r\n---------\r\n\r\nCo-authored-by: Søren Louv-Jansen <[email protected]>","sha":"06526fe928ac98580e6fe02768b3c403184694f1"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.17","label":"v8.17.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT--> Co-authored-by: Sandra G <[email protected]>
💚 All backports created successfully
Note: Successful backport PRs will be merged automatically after passing CI. Questions ?Please refer to the Backport tool documentation |
) Resolves elastic#205970 Updates logic to account for knowledge base `/setup` no longer polling for model readiness before returning. - Currently we only poll `/status` if user manually installs the knowledge base. In cases where we auto installed, such as after successfully setting up a connector, we depended on `/setup` to poll internally. Since the latter was removed, we need to always poll `/status`, otherwise user could potentially be in the state where `setup` has finished (considered installed) but `status` still reports not ready and we show the install message again (see screenshots in elastic#205970) - Currently if an install is in progress and user closes the flyout, the progress state is lost. These changes should continue to reflect the installation progress in the UI. - Renames variables and adds comments for easier readability - adds unit test to component that handles the install UI state, `WelcomeMessageKnowledgeBase` --------- Co-authored-by: Søren Louv-Jansen <[email protected]> (cherry picked from commit 06526fe) # Conflicts: # x-pack/packages/kbn-ai-assistant/src/chat/welcome_message_knowledge_base.test.tsx # x-pack/plugins/observability_solution/observability_ai_assistant/server/routes/knowledge_base/route.ts
) (#206837) # Backport This will backport the following commits from `main` to `8.17`: - [[Obs AI Assistant] fix knowledge base installation state (#206130)](#206130) <!--- Backport version: 9.6.4 --> ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sorenlouv/backport) <!--BACKPORT [{"author":{"name":"Sandra G","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-15T14:38:07Z","message":"[Obs AI Assistant] fix knowledge base installation state (#206130)\n\nResolves https://github.com/elastic/kibana/issues/205970\r\n\r\nUpdates logic to account for knowledge base `/setup` no longer polling\r\nfor model readiness before returning.\r\n\r\n- Currently we only poll `/status` if user manually installs the\r\nknowledge base. In cases where we auto installed, such as after\r\nsuccessfully setting up a connector, we depended on `/setup` to poll\r\ninternally. Since the latter was removed, we need to always poll\r\n`/status`, otherwise user could potentially be in the state where\r\n`setup` has finished (considered installed) but `status` still reports\r\nnot ready and we show the install message again (see screenshots in\r\nhttps://github.com//issues/205970)\r\n- Currently if an install is in progress and user closes the flyout, the\r\nprogress state is lost. These changes should continue to reflect the\r\ninstallation progress in the UI.\r\n- Renames variables and adds comments for easier readability\r\n- adds unit test to component that handles the install UI state,\r\n`WelcomeMessageKnowledgeBase`\r\n\r\n---------\r\n\r\nCo-authored-by: Søren Louv-Jansen <[email protected]>","sha":"06526fe928ac98580e6fe02768b3c403184694f1","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","ci:cloud-deploy","Team:Obs AI Assistant","ci:project-deploy-elasticsearch","ci:project-deploy-observability","backport:version","v8.18.0","v8.17.1"],"title":"[Obs AI Assistant] fix knowledge base installation state","number":206130,"url":"https://github.com/elastic/kibana/pull/206130","mergeCommit":{"message":"[Obs AI Assistant] fix knowledge base installation state (#206130)\n\nResolves https://github.com/elastic/kibana/issues/205970\r\n\r\nUpdates logic to account for knowledge base `/setup` no longer polling\r\nfor model readiness before returning.\r\n\r\n- Currently we only poll `/status` if user manually installs the\r\nknowledge base. In cases where we auto installed, such as after\r\nsuccessfully setting up a connector, we depended on `/setup` to poll\r\ninternally. Since the latter was removed, we need to always poll\r\n`/status`, otherwise user could potentially be in the state where\r\n`setup` has finished (considered installed) but `status` still reports\r\nnot ready and we show the install message again (see screenshots in\r\nhttps://github.com//issues/205970)\r\n- Currently if an install is in progress and user closes the flyout, the\r\nprogress state is lost. These changes should continue to reflect the\r\ninstallation progress in the UI.\r\n- Renames variables and adds comments for easier readability\r\n- adds unit test to component that handles the install UI state,\r\n`WelcomeMessageKnowledgeBase`\r\n\r\n---------\r\n\r\nCo-authored-by: Søren Louv-Jansen <[email protected]>","sha":"06526fe928ac98580e6fe02768b3c403184694f1"}},"sourceBranch":"main","suggestedTargetBranches":["8.17"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/206130","number":206130,"mergeCommit":{"message":"[Obs AI Assistant] fix knowledge base installation state (#206130)\n\nResolves https://github.com/elastic/kibana/issues/205970\r\n\r\nUpdates logic to account for knowledge base `/setup` no longer polling\r\nfor model readiness before returning.\r\n\r\n- Currently we only poll `/status` if user manually installs the\r\nknowledge base. In cases where we auto installed, such as after\r\nsuccessfully setting up a connector, we depended on `/setup` to poll\r\ninternally. Since the latter was removed, we need to always poll\r\n`/status`, otherwise user could potentially be in the state where\r\n`setup` has finished (considered installed) but `status` still reports\r\nnot ready and we show the install message again (see screenshots in\r\nhttps://github.com//issues/205970)\r\n- Currently if an install is in progress and user closes the flyout, the\r\nprogress state is lost. These changes should continue to reflect the\r\ninstallation progress in the UI.\r\n- Renames variables and adds comments for easier readability\r\n- adds unit test to component that handles the install UI state,\r\n`WelcomeMessageKnowledgeBase`\r\n\r\n---------\r\n\r\nCo-authored-by: Søren Louv-Jansen <[email protected]>","sha":"06526fe928ac98580e6fe02768b3c403184694f1"}},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"url":"https://github.com/elastic/kibana/pull/206777","number":206777,"state":"OPEN"},{"branch":"8.17","label":"v8.17.1","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}] BACKPORT-->
Resolves #205970
Updates logic to account for knowledge base
/setup
no longer polling for model readiness before returning./status
if user manually installs the knowledge base. In cases where we auto installed, such as after successfully setting up a connector, we depended on/setup
to poll internally. Since the latter was removed, we need to always poll/status
, otherwise user could potentially be in the state wheresetup
has finished (considered installed) butstatus
still reports not ready and we show the install message again (see screenshots in [Obs AI Assistant] Fix knowledge base install status #205970)WelcomeMessageKnowledgeBase