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

Roman/bugfix support bedrock embeddings #2650

Merged
merged 10 commits into from
Mar 21, 2024

Conversation

rbiseck3
Copy link
Contributor

Description

This PR resolved the following open issue: bug/bedrock-encoder-not-supported-in-ingest. To do so, the following changes were made:

  • All aws configs were added as input parameters to the CLI
  • These were mapped to the bedrock embedder when an embedder is generated via get_embedder
  • An ingest test was added to call the aws bedrock service
  • Requirements for boto were bumped because the first version to introduce the bedrock runtime, which is required to hit the bedrock service, was introduced in version 1.34.63, which was ahead of the version of boto pinned.

@potter-potter
Copy link
Contributor

potter-potter commented Mar 15, 2024

did you get changes in expected output when running the comparison test in the .sh? My embeddings are now 8 decimals instead of 17 decimals.

@rbiseck3 rbiseck3 force-pushed the roman/bugfix-support-bedrock-embeddings branch from 2f2a710 to 3f13302 Compare March 18, 2024 12:11
@potter-potter
Copy link
Contributor

Would love to see this running in test_ingest_src. Can you fix the lint and shfmt.

@potter-potter
Copy link
Contributor

test_ingest_src failed in CI

@rbiseck3
Copy link
Contributor Author

test_ingest_src failed in CI

CI needs to have updated AWS creds to have permissions to hit the bedrock model.

@potter-potter
Copy link
Contributor

potter-potter commented Mar 19, 2024

test_ingest_src failed in CI

CI needs to have updated AWS creds to have permissions to hit the bedrock model.

did you want to give us-east-2 a try for the region? locally I'm able to run bedrock on both local
platform and command line with same old creds. (though platform is defaulting to us-west-2 and actually not encoding. I think its ui issues. Actually probably because its not running this fix :) )

Copy link
Contributor

@potter-potter potter-potter left a comment

Choose a reason for hiding this comment

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

This looks good.

@rbiseck3 rbiseck3 force-pushed the roman/bugfix-support-bedrock-embeddings branch from 5968ad7 to f9c41ff Compare March 21, 2024 17:14
@rbiseck3 rbiseck3 added this pull request to the merge queue Mar 21, 2024
Merged via the queue into main with commit 4ff6a5b Mar 21, 2024
46 checks passed
@rbiseck3 rbiseck3 deleted the roman/bugfix-support-bedrock-embeddings branch March 21, 2024 18:54
kaaloo pushed a commit to inclusif/unstructured that referenced this pull request Apr 8, 2024
### Description
This PR resolved the following open issue:
[bug/bedrock-encoder-not-supported-in-ingest](https://github.com/Unstructured-IO/unstructured/issues/2319).
To do so, the following changes were made:
* All aws configs were added as input parameters to the CLI
* These were mapped to the bedrock embedder when an embedder is
generated via `get_embedder`
* An ingest test was added to call the aws bedrock service
* Requirements for boto were bumped because the first version to
introduce the bedrock runtime, which is required to hit the bedrock
service, was introduced in version `1.34.63`, which was ahead of the
version of boto pinned.

---------

Co-authored-by: ryannikolaidis <1208590+ryannikolaidis@users.noreply.github.com>
Co-authored-by: rbiseck3 <rbiseck3@users.noreply.github.com>
kaaloo pushed a commit to inclusif/unstructured that referenced this pull request Apr 8, 2024
### Description
This PR resolved the following open issue:
[bug/bedrock-encoder-not-supported-in-ingest](https://github.com/Unstructured-IO/unstructured/issues/2319).
To do so, the following changes were made:
* All aws configs were added as input parameters to the CLI
* These were mapped to the bedrock embedder when an embedder is
generated via `get_embedder`
* An ingest test was added to call the aws bedrock service
* Requirements for boto were bumped because the first version to
introduce the bedrock runtime, which is required to hit the bedrock
service, was introduced in version `1.34.63`, which was ahead of the
version of boto pinned.

---------

Co-authored-by: ryannikolaidis <1208590+ryannikolaidis@users.noreply.github.com>
Co-authored-by: rbiseck3 <rbiseck3@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants