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

[Security GenAI] Remove assistantBedrockChat feature flag #195461

Merged
merged 4 commits into from
Oct 8, 2024

Conversation

stephmilovic
Copy link
Contributor

@stephmilovic stephmilovic commented Oct 8, 2024

Summary

The assistantBedrockChat feature flag was never properly implemented. It was set to true by default, with the assumption that adding it to kibana.dev.yml would switch it to false. However, we've since learned feature flags can only go from false to true. Therefore, when assistantBedrockChat was introduced in 8.15 it was always set to true. We are keeping the truthy paths in place and removing the flag in this PR.

  • Removes assistantBedrockChat feature flag
  • Removes code paths for ActionsClientSimpleChatModel from x-pack/plugins/elastic_assistant
  • Adds comments and tests for clarity in streaming handlers in default_assistant_graph/helpers.ts

@stephmilovic stephmilovic added release_note:fix v9.0.0 Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) Team:Security Generative AI Security Generative AI v8.16.0 labels Oct 8, 2024
@stephmilovic stephmilovic requested review from a team as code owners October 8, 2024 16:00
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@stephmilovic
Copy link
Contributor Author

@elasticmachine merge upstream

@elasticmachine
Copy link
Contributor

⏳ Build in-progress, with failures

Failed CI Steps

Test Failures

  • [job] [logs] Serverless Osquery Cypress Tests #1 / Alert Event Details - Cases Case sees osquery results from last action and add to a case sees osquery results from last action and add to a case

History

@stephmilovic
Copy link
Contributor Author

@elasticmachine merge upstream

@@ -164,41 +154,6 @@ export const streamGraph = async ({
}
}
}
if (event.name === 'ActionsClientSimpleChatModel') {
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️

Copy link
Contributor

@patrykkopycinski patrykkopycinski left a comment

Choose a reason for hiding this comment

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

Thank you @stephmilovic!

@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
securitySolution 20.6MB 20.6MB -120.0B

Page load bundle

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

id before after diff
securitySolution 88.3KB 88.3KB -24.0B

History

@stephmilovic stephmilovic merged commit 38d0bdd into elastic:main Oct 8, 2024
41 checks passed
@kibanamachine
Copy link
Contributor

Starting backport for target branches: 8.x

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

@kibanamachine
Copy link
Contributor

💔 All backports failed

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

Manual backport

To create the backport manually run:

node scripts/backport --pr 195461

Questions ?

Please refer to the Backport tool documentation

@stephmilovic
Copy link
Contributor Author

💚 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

stephmilovic added a commit that referenced this pull request Oct 9, 2024
…95461) (#195516)

# Backport

This will backport the following commits from `main` to `8.x`:
- [[Security GenAI] Remove `assistantBedrockChat` feature flag
(#195461)](#195461)

<!--- Backport version: 8.9.8 -->

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

<!--BACKPORT [{"author":{"name":"Steph
Milovic","email":"[email protected]"},"sourceCommit":{"committedDate":"2024-10-08T22:31:05Z","message":"[Security
GenAI] Remove `assistantBedrockChat` feature flag
(#195461)","sha":"38d0bdd3deeb72ca4e502361fb0df1f68fa7f443","branchLabelMapping":{"^v9.0.0$":"main","^v8.16.0$":"8.x","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["release_note:fix","v9.0.0","Team:
SecuritySolution","backport:prev-minor","Team:Security Generative
AI","v8.16.0"],"number":195461,"url":"https://github.com/elastic/kibana/pull/195461","mergeCommit":{"message":"[Security
GenAI] Remove `assistantBedrockChat` feature flag
(#195461)","sha":"38d0bdd3deeb72ca4e502361fb0df1f68fa7f443"}},"sourceBranch":"main","suggestedTargetBranches":["8.x"],"targetPullRequestStates":[{"branch":"main","label":"v9.0.0","labelRegex":"^v9.0.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/195461","number":195461,"mergeCommit":{"message":"[Security
GenAI] Remove `assistantBedrockChat` feature flag
(#195461)","sha":"38d0bdd3deeb72ca4e502361fb0df1f68fa7f443"}},{"branch":"8.x","label":"v8.16.0","labelRegex":"^v8.16.0$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->

Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) release_note:fix Team:Security Generative AI Security Generative AI Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. v8.16.0 v9.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants