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

[Obs ai assistant] - bug bulk import for knowledge base #205075

Merged
merged 5 commits into from
Jan 7, 2025
Merged

[Obs ai assistant] - bug bulk import for knowledge base #205075

merged 5 commits into from
Jan 7, 2025

Conversation

arturoliduena
Copy link
Contributor

@arturoliduena arturoliduena commented Dec 23, 2024 •
edited by kibanamachine
Loading

Closes #196096

Summary

Wrong successful message with error notification pop up on adding a wrong format file under bulk import for knowledge base
#196096

@arturoliduena arturoliduena requested a review from a team as a code owner December 23, 2024 08:25
@elasticmachine
Copy link
Contributor

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

dgieselaar
dgieselaar reviewed Dec 30, 2024
View reviewed changes
...ility_ai_assistant_management/public/routes/components/knowledge_base_bulk_import_flyout.tsx Outdated
@@ -54,6 +54,7 @@ export function KnowledgeBaseBulkImportFlyout({ onClose }: { onClose: () => void

try {
entries = elements.map((el) => JSON.parse(el));
mutateAsync({ entries }).then(onClose);
Copy link
Member

@dgieselaar dgieselaar Dec 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

you'd need to await this presumably? and I think you can remove the .then(onClose) (both here and in the catch block) and move it to below the try/catch block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dgieselaar, updated it to await mutateAsync and moved onClose outside the try/catch block.

@arturoliduena arturoliduena force-pushed the obs-ai-assistant-196096-bug-bulk-import-for-knowledge-base branch from 0fcc09a to 7848536 Compare January 3, 2025 15:00
sorenlouv
sorenlouv reviewed Jan 3, 2025
View reviewed changes
...ility_ai_assistant_management/public/routes/components/knowledge_base_bulk_import_flyout.tsx Outdated
Comment on lines 56 to 61
entries = elements.map((el) => JSON.parse(el));
await mutateAsync({ entries });
Copy link
Member

Choose a reason for hiding this comment

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

Afaict entries can be declared as const now:

Suggested change
entries = elements.map((el) => JSON.parse(el));
await mutateAsync({ entries });
const entries = elements.map((el) => JSON.parse(el));
await mutateAsync({ entries });

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the suggestion! You’re right, entries can be declared as const since it doesn’t get reassigned. I’ve updated the code to remove the let declaration and added the appropriate type annotation to ensure type safety.

arturoliduena and others added 4 commits January 7, 2025 08:50
@arturoliduena
[Obs ai assistant] - bug bulk import for knowledge base
9a6c2ee
@arturoliduena
Fix bulk import flow in Knowledge Base by awaiting async mutation and…
2456787 
… ensuring onClose is called correctly
@arturoliduena
Refactor entry parsing in Knowledge Base bulk import to declare entri…
bd7b560 
…es with proper type
@arturoliduena @sorenlouv
Update x-pack/solutions/observability/plugins/observability_ai_assist…
d6fe09e 
…ant_management/public/routes/components/knowledge_base_bulk_import_flyout.tsx - Move the type assertion to the JSON parsing and let the compiler handle the rest.

Co-authored-by: Søren Louv-Jansen <[email protected]>
@arturoliduena arturoliduena force-pushed the obs-ai-assistant-196096-bug-bulk-import-for-knowledge-base branch from 0753032 to d6fe09e Compare January 7, 2025 07:50
@elasticmachine
Copy link
Contributor

💚 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
observabilityAiAssistantManagement 97.6KB 97.6KB -1.0B

History

@arturoliduena arturoliduena merged commit 2c97ed3 into elastic:main Jan 7, 2025
8 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.16, 8.17, 8.x

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

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 7, 2025
@arturoliduena
[Obs ai assistant] - bug bulk import for knowledge base (elastic#205075)
23587a1 
Closes elastic#196096

## Summary

Wrong successful message with error notification pop up on adding a
wrong format file under bulk import for knowledge base
[elastic#196096](elastic#196096)

---------

Co-authored-by: Søren Louv-Jansen <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 2c97ed3)
@kibanamachine kibanamachine mentioned this pull request Jan 7, 2025
Merged
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jan 7, 2025
@arturoliduena
[Obs ai assistant] - bug bulk import for knowledge base (elastic#205075)
3a601f0 
Closes elastic#196096

## Summary

Wrong successful message with error notification pop up on adding a
wrong format file under bulk import for knowledge base
[elastic#196096](elastic#196096)

---------

Co-authored-by: Søren Louv-Jansen <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 2c97ed3)
@kibanamachine kibanamachine mentioned this pull request Jan 7, 2025
Merged
@kibanamachine
Copy link
Contributor

💔 Some backports could not be created

Status Branch Result
❌ 8.16 Backport failed because of merge conflicts
✅ 8.17
✅ 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 205075

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Jan 7, 2025
@kibanamachine @arturoliduena
[8.17] [Obs ai assistant] - bug bulk import for knowledge base (#205075…
b56ed4c 
…) (#205687)

# Backport

This will backport the following commits from `main` to `8.17`:
- [[Obs ai assistant] - bug bulk import for knowledge base
(#205075)](#205075)

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

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

<!--BACKPORT [{"author":{"name":"Arturo
Lidueña","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-07T09:44:16Z","message":"[Obs
ai assistant] - bug bulk import for knowledge base (#205075)\n\nCloses
#196096 \r\n\r\n## Summary\r\n\r\nWrong successful message with error
notification pop up on adding a\r\nwrong format file under bulk import
for knowledge
base\r\n[#196096](https://github.com/elastic/kibana/issues/196096)\r\n\r\n---------\r\n\r\nCo-authored-by:
Søren Louv-Jansen <[email protected]>\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"2c97ed3bfec8a99de931d85f50b1e940835bde47","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","Team:Obs
AI
Assistant","v8.16.0","backport:version","v8.17.0","v8.18.0"],"title":"[Obs
ai assistant] - bug bulk import for knowledge
base","number":205075,"url":"https://github.com/elastic/kibana/pull/205075","mergeCommit":{"message":"[Obs
ai assistant] - bug bulk import for knowledge base (#205075)\n\nCloses
#196096 \r\n\r\n## Summary\r\n\r\nWrong successful message with error
notification pop up on adding a\r\nwrong format file under bulk import
for knowledge
base\r\n[#196096](https://github.com/elastic/kibana/issues/196096)\r\n\r\n---------\r\n\r\nCo-authored-by:
Søren Louv-Jansen <[email protected]>\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"2c97ed3bfec8a99de931d85f50b1e940835bde47"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.17","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/205075","number":205075,"mergeCommit":{"message":"[Obs
ai assistant] - bug bulk import for knowledge base (#205075)\n\nCloses
#196096 \r\n\r\n## Summary\r\n\r\nWrong successful message with error
notification pop up on adding a\r\nwrong format file under bulk import
for knowledge
base\r\n[#196096](https://github.com/elastic/kibana/issues/196096)\r\n\r\n---------\r\n\r\nCo-authored-by:
Søren Louv-Jansen <[email protected]>\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"2c97ed3bfec8a99de931d85f50b1e940835bde47"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.17","label":"v8.17.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Arturo Lidueña <[email protected]>
kibanamachine added a commit that referenced this pull request Jan 7, 2025
@kibanamachine @arturoliduena
[8.x] [Obs ai assistant] - bug bulk import for knowledge base (#205075)…
def21fa 
… (#205688)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Obs ai assistant] - bug bulk import for knowledge base
(#205075)](#205075)

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

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

<!--BACKPORT [{"author":{"name":"Arturo
Lidueña","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-01-07T09:44:16Z","message":"[Obs
ai assistant] - bug bulk import for knowledge base (#205075)\n\nCloses
#196096 \r\n\r\n## Summary\r\n\r\nWrong successful message with error
notification pop up on adding a\r\nwrong format file under bulk import
for knowledge
base\r\n[#196096](https://github.com/elastic/kibana/issues/196096)\r\n\r\n---------\r\n\r\nCo-authored-by:
Søren Louv-Jansen <[email protected]>\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"2c97ed3bfec8a99de931d85f50b1e940835bde47","branchLabelMapping":{"^v9.0.0$":"main","^v8.18.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","Team:Obs
AI
Assistant","v8.16.0","backport:version","v8.17.0","v8.18.0"],"title":"[Obs
ai assistant] - bug bulk import for knowledge
base","number":205075,"url":"https://github.com/elastic/kibana/pull/205075","mergeCommit":{"message":"[Obs
ai assistant] - bug bulk import for knowledge base (#205075)\n\nCloses
#196096 \r\n\r\n## Summary\r\n\r\nWrong successful message with error
notification pop up on adding a\r\nwrong format file under bulk import
for knowledge
base\r\n[#196096](https://github.com/elastic/kibana/issues/196096)\r\n\r\n---------\r\n\r\nCo-authored-by:
Søren Louv-Jansen <[email protected]>\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"2c97ed3bfec8a99de931d85f50b1e940835bde47"}},"sourceBranch":"main","suggestedTargetBranches":["8.16","8.17","8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","branchLabelMappingKey":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/205075","number":205075,"mergeCommit":{"message":"[Obs
ai assistant] - bug bulk import for knowledge base (#205075)\n\nCloses
#196096 \r\n\r\n## Summary\r\n\r\nWrong successful message with error
notification pop up on adding a\r\nwrong format file under bulk import
for knowledge
base\r\n[#196096](https://github.com/elastic/kibana/issues/196096)\r\n\r\n---------\r\n\r\nCo-authored-by:
Søren Louv-Jansen <[email protected]>\r\nCo-authored-by: kibanamachine
<[email protected]>","sha":"2c97ed3bfec8a99de931d85f50b1e940835bde47"}},{"branch":"8.16","label":"v8.16.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.17","label":"v8.17.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"},{"branch":"8.x","label":"v8.18.0","branchLabelMappingKey":"^v8.18.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Arturo Lidueña <[email protected]>
kowalczyk-krzysztof pushed a commit to kowalczyk-krzysztof/kibana that referenced this pull request Jan 7, 2025
@arturoliduena @sorenlouv
@kibanamachine @kowalczyk-krzysztof
[Obs ai assistant] - bug bulk import for knowledge base (elastic#205075)
024f7ef 
Closes elastic#196096 

## Summary

Wrong successful message with error notification pop up on adding a
wrong format file under bulk import for knowledge base
[elastic#196096](elastic#196096)

---------

Co-authored-by: Søren Louv-Jansen <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Jan 13, 2025
@arturoliduena @sorenlouv
@kibanamachine @CAWilson94
[Obs ai assistant] - bug bulk import for knowledge base (elastic#205075)
57b1b31 
Closes elastic#196096 

## Summary

Wrong successful message with error notification pop up on adding a
wrong format file under bulk import for knowledge base
[elastic#196096](elastic#196096)

---------

Co-authored-by: Søren Louv-Jansen <[email protected]>
Co-authored-by: kibanamachine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dgieselaar dgieselaar dgieselaar left review comments

@sorenlouv sorenlouv sorenlouv approved these changes

Assignees
No one assigned
Labels
backport:version Backport to applied version labels release_note:fix Team:Obs AI Assistant Observability AI Assistant v8.16.0 v8.17.0 v8.17.1 v8.18.0 v9.0.0
Projects
None yet
Milestone
No milestone
5 participants
@arturoliduena @elasticmachine @kibanamachine @sorenlouv @dgieselaar