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

feat(bedrock): New embedding model config #665

Merged
merged 14 commits into from
Sep 4, 2024

Conversation

aws-rafams
Copy link
Contributor

@aws-rafams aws-rafams commented Aug 27, 2024

Fixes #657

Also addressing #492


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the project license.

@krokoko krokoko self-requested a review August 27, 2024 19:54
krokoko
krokoko previously approved these changes Aug 27, 2024
@krokoko krokoko self-requested a review August 27, 2024 19:57
@krokoko
Copy link
Collaborator

krokoko commented Aug 27, 2024

Failing build

@aws-rafams
Copy link
Contributor Author

aws-rafams commented Aug 27, 2024

Failing build

I see it fails because of an assert on the integ test snapshot. It did work locally. I was having similar problems and that's why I had to comment out part of the code. Will look into it.

@aws-rafams
Copy link
Contributor Author

aws-rafams commented Aug 27, 2024

seems the assert fails due to some non-deterministic name generation and my local build succeeds due to some local caching, will continue to look into a possible solution

Copy link
Collaborator

@krokoko krokoko left a comment

Choose a reason for hiding this comment

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

Waiting for #663 to go in before merging this

@aws-rafams aws-rafams reopened this Sep 2, 2024
@aws-rafams aws-rafams requested a review from krokoko September 2, 2024 13:05
Copy link
Collaborator

@scottschreckengaust scottschreckengaust left a comment

Choose a reason for hiding this comment

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

LGTM

@krokoko krokoko merged commit 4dcf286 into awslabs:main Sep 4, 2024
13 of 15 checks passed
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.

(bedrock): Knowledge base BedrockEmbeddingModelConfiguration
4 participants