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

[Obs AI Assistant] fix knowledge base installation state #206130

Merged
merged 12 commits into from
Jan 15, 2025

Conversation

neptunian
Copy link
Contributor

@neptunian neptunian commented Jan 9, 2025

Resolves #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 [Obs AI Assistant] Fix knowledge base install status #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

@neptunian neptunian added release_note:skip Skip the PR/issue when compiling release notes Team:Obs AI Assistant Observability AI Assistant labels Jan 9, 2025
setIsPopoverOpen(false);

await knowledgeBase.install().then(() => {
setCheckForInstallStatus(false);
Copy link
Contributor Author

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.

@neptunian neptunian marked this pull request as ready for review January 10, 2025 20:05
@neptunian neptunian requested a review from a team as a code owner January 10, 2025 20:05
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ai-assistant (Team:Obs AI Assistant)

@neptunian neptunian added release_note:fix backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) and removed release_note:skip Skip the PR/issue when compiling release notes labels Jan 10, 2025
@neptunian neptunian requested a review from a team as a code owner January 13, 2025 19:16
@@ -100,69 +117,72 @@ export function WelcomeMessageKnowledgeBase({
</>
) : null}

{connectors.connectors?.length ? (
Copy link
Contributor Author

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.

@neptunian neptunian added backport:version Backport to applied version labels v8.18.0 v9.0.0 v8.17.1 and removed backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Jan 13, 2025
expect(screen.getByText('Install Knowledge base', { exact: false })).toBeInTheDocument();
expect(screen.getByText('Inspect issues', { exact: false })).toBeInTheDocument();
});
});
Copy link
Member

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!

neptunian and others added 4 commits January 14, 2025 07:10
…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]>
@elasticmachine
Copy link
Contributor

💛 Build succeeded, but was flaky

Failed CI Steps

Test Failures

  • [job] [logs] FTR Configs #76 / visualize app visual builder Time Series Elastic charts should display correct chart data, label names and area colors for sum aggregation when split by terms

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
observabilityAIAssistantApp 297.8KB 297.6KB -182.0B
observabilityAiAssistantManagement 100.2KB 100.2KB +8.0B
searchAssistant 163.2KB 163.0KB -182.0B
total -356.0B

History

@neptunian neptunian requested a review from sorenlouv January 14, 2025 18:47
Copy link
Member

@sphilipse sphilipse left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@neptunian neptunian added ci:project-deploy-elasticsearch Create an Elasticsearch Serverless project ci:project-deploy-observability Create an Observability project labels Jan 15, 2025
Copy link
Contributor

🤖 GitHub comments

Expand to view the GitHub comments

Just comment with:

  • /oblt-deploy : Deploy a Kibana instance using the Observability test environments.
  • run docs-build : Re-trigger the docs validation. (use unformatted text in the comment!)

@neptunian
Copy link
Contributor Author

/oblt-deploy

@neptunian neptunian added the ci:cloud-deploy Create or update a Cloud deployment label Jan 15, 2025
@neptunian neptunian merged commit 06526fe into elastic:main Jan 15, 2025
14 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.17, 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 15, 2025
)

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)
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
8.17 Backport failed because of merge conflicts
8.x

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

Manual backport

To create the backport manually run:

node scripts/backport --pr 206130

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jan 15, 2025
…) (#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]>
@neptunian
Copy link
Contributor Author

💚 All backports created successfully

Status Branch Result
8.17

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

Questions ?

Please refer to the Backport tool documentation

neptunian added a commit to neptunian/kibana that referenced this pull request Jan 15, 2025
)

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
neptunian added a commit that referenced this pull request Jan 15, 2025
) (#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-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:version Backport to applied version labels ci:cloud-deploy Create or update a Cloud deployment ci:project-deploy-elasticsearch Create an Elasticsearch Serverless project ci:project-deploy-observability Create an Observability project release_note:fix Team:Obs AI Assistant Observability AI Assistant v8.17.1 v8.18.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Obs AI Assistant] Fix knowledge base install status
6 participants