-
Notifications
You must be signed in to change notification settings - Fork 110
feat: Amazon Bedrock Embedding doc #204
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few small nits. Otherwise looks good.
docs/embeddings/amazon-bedrock.md
Outdated
</TabItem> | ||
<TabItem value="js" label="JavaScript"> | ||
|
||
To use Amazon Bedrock embedding API, you must have `@aws-sdk/client-bedrock-runtime` installed. To use: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chezou, do you think users can benefit from explaining how to supply the credentials e.g. export them as env vars or use aws cli to login?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's too much to explain AWS credential preparation stuff in detail, so I added an example with Access Key ID and Secret Access Key instead.
Also, added the link to more information on AWS credential handling.
7390e29
- Add boto3 installation - Add more JS authentication details
@tazarov I think I updated with enough information, but let me know if there is anything I can improve. |
@jeffchuber Can I ask you to review this PR? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good - pending the one nit, thanks!
docs/embeddings/amazon-bedrock.md
Outdated
const collection = await client.getCollection({name: "name", embeddingFunction: ef}) | ||
``` | ||
|
||
If you want to use other credentials, you need to install `@aws-sdk/credential-providers`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you provide an example or two of "other credentials"? thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, added db5ee3c
@jeffchuber Can you merge it when you have time? |
@jeffchuber @HammadB Can you merge this PR? |
@jeffchuber can we merge this PR? chroma-core/chroma#1675 is waiting for this one |
This is follow-up for chroma-core/chroma#1361
When I find time to contribute JS SDK, I will send another PR for document update as well.JS client is now ready for review chroma-core/chroma#1659
So, I added a JS document as well.