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

Add Bedrock Cloud Model provider #21092

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

5herlocked
Copy link

Closes #16544

Release Notes:

  • Added support for Bedrock as an FM provider
  • Added rudimentary support for AWS credential management to Zed

@5herlocked
Copy link
Author

This still needs to be updated to the new method of managing models, going to spend the next couple of weeks migrating to it.

@5herlocked
Copy link
Author

5herlocked commented Nov 23, 2024

Zed maintainers, would appreciate some guidance on how the ZedPro things are setup and if it makes sense to pass the Bedrock models into it.

On a cursory reading of the feature, i don't believe it does -- however, i am open to working with the bedrock service team to make it a reality.

@maxdeviant maxdeviant changed the title feature: Bedrock Cloud Model provider Add Bedrock Cloud Model provider Nov 23, 2024
@rgbkrk
Copy link
Member

rgbkrk commented Nov 23, 2024

I don't think you need to pass through the bedrock models through the collab service, as Zed would need to run their own bedrock setup. I'd just do the API key / AWS credentials route.

@SomeoneToIgnore
Copy link
Contributor

Unfortunately, our CLA bot is broken, so I have to substitute for it for a while:

We require contributors to sign our Contributor License Agreement, and we don't have @5herlocked on file. You can sign our CLA at https://zed.dev/cla. Once you've signed, post a comment here that says '@cla-bot check'.

@5herlocked
Copy link
Author

Will get it done post a legal review of the docs (just want to make sure AWS doesn't have an issue with it)

@5herlocked
Copy link
Author

@cla-bot check

@notpeter notpeter added the cla-signed The user has signed the Contributor License Agreement label Nov 25, 2024
use thiserror::Error;

use aws_sdk_bedrockruntime as bedrock;
pub use aws_sdk_bedrockruntime as bedrock_client;
Copy link
Member

Choose a reason for hiding this comment

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

I see on https://docs.rs/aws-sdk-bedrockruntime/latest/aws_sdk_bedrockruntime/ that this requires tokio. Does this setup get around that to ensure it uses gpui's executors?

Copy link
Author

Choose a reason for hiding this comment

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

I'll be honest, didn't think about that -- but, I don't believe so.
All the uses of tokio in the Bedrock SDK are opaque, so I don't know if they can be gotten around to use the gpui executors.

Probably to get around i'd have to implement SigV4 + use the raw HTTP client although it defeats the point of the SDK.

Copy link
Member

@rgbkrk rgbkrk Nov 26, 2024

Choose a reason for hiding this comment

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

In a similar vein I've had to export just the serde types and then construct the API calls inside of Zed. At the very least I wish that Zed's HttpClient trait was exported so that it would be easier to build libraries that target GPUI.

Copy link
Author

@5herlocked 5herlocked Nov 26, 2024

Choose a reason for hiding this comment

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

Do you think this could be possible with: the SDK config builder

Based on the source code it seems to accept an http_client as part of the config constructor.

Will see if i can implement it to accept the GPUI HttpClient.

Copy link
Member

@rgbkrk rgbkrk Nov 27, 2024

Choose a reason for hiding this comment

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

Rust will only be happy with that if they're the same HttpClient trait between GPUI/Zed and the SDK config builder.

crates/collab/src/llm.rs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed The user has signed the Contributor License Agreement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support AWS Bedrock as an Assistant provider
4 participants