Added support for Bedrock API keys #983
Conversation
|
Thanks for the PR, @andykuszyk.
|
Ah right, I didnt realise that! I'll test Bedrock access using |
|
> 1. All gptel-backend structs including gptel-bedrock already provide a `:key` field, there is no need to add `:api-key`.
Ah right, I didnt realise that! I'll test Bedrock access using `:key` and see how it goes 👍
I doubt it will work, since AFAICT the auth functionality isn't implemented for :key. I was commention only on the name of the field. The authors of gptel-bedrock (pinged above) might have more information for us.
|
You're right, it doesn't work as-is: 
|
gptel-bedrock.el
Outdated
| @@ -628,11 +637,12 @@ STREAM - Whether to use streaming responses or not." | |||
| :header nil ; x-amz-security-token is set in curl-args if needed | |||
| :models (gptel--process-models models) | |||
| :model-region model-region | |||
| :api-key api-key | |||
There was a problem hiding this comment.
I think this just needs to be replaced with :key api-key to use the default gptel field. But can you call this bedrock-api-key so it's clear what it refers to?
There was a problem hiding this comment.
@felipeochoa I've refactored this PR based on your comment, and changed the name of this parameter 👍
|
I think it would be better to not touch Better would be to handle this directly in (defun gptel-bedrock--curl-args (region backend)
"Generate the curl arguments to get a bedrock request signed for use in REGION."
(let* ((api-key (gptel-backend-key backend))
(auth-args (if api-key (new-code) (existing-code))))
(nconc etc.))) |
[Bedrock API keys](https://docs.aws.amazon.com/bedrock/latest/userguide/api-keys.html) offer a convenient alternative to authenticating with AWS to use Bedrock. This commit introduces an optional `:bedrock-api-key` parameter for providing an API key, which takes precedence over other authentication methods.
|
@felipeochoa @karthink I've re-worked this PR based on your feedback (thanks for your advice 🙏 🙂 ). I've also tested this locally with both a Bedrock API key and an AWS profile for authentication, and it seems to work fine for both 👍 |
|
@andykuszyk I can merge this with a minimal review once @felipeochoa or @akssri give us the go-ahead. |
| @@ -635,7 +642,9 @@ parameters (as plist keys) and values supported by the API." | |||
| (gptel--make-bedrock | |||
| :name name | |||
| :host host | |||
| :header nil ; x-amz-security-token is set in curl-args if needed | |||
There was a problem hiding this comment.
Why not set the bearer token in curl-args as well to keep the logic centralized? See https://github.com/karthink/gptel/pull/1007/files as an example
There was a problem hiding this comment.
Good idea 👍
I guess we could allow the API key to be set as either an elisp parameter, or as an environment variable?
Bedrock API keys offer a convenient alternative to authenticating with AWS to use Bedrock. This commit introduces an optional
:api-keyparameter for providing an API key, which takes precedence over other authentication methods.