feat(bedrock): support AWS_BEARER_TOKEN_BEDROCK env var - #1080
feat(bedrock): support AWS_BEARER_TOKEN_BEDROCK env var#1080davidmhewitt wants to merge 4 commits into
Conversation
| profile: str | None, | ||
| data: str | None, | ||
| ) -> dict[str, str]: | ||
| bedrock_bearer = os.getenv("AWS_BEARER_TOKEN_BEDROCK") |
There was a problem hiding this comment.
I think adding api_key as a client argument and defaulting to that environment variable would be better than only supporting it through the environment variable
There was a problem hiding this comment.
Sure, I've added it as the last argument to to the client constructor to preserve the existing ordering. Let me know if that makes sense, given the _strict_response_validation arg (and associated comment) above it.
|
LGTM! Note that it also needs ruff format. Could you please run/commit it? |
RobertCraigie
left a comment
There was a problem hiding this comment.
Do you know why the AWS naming around this functionality is inconsistent? i.e. why is the env var "bearer token" but the actual value is an "api key"?
Not sure! I would guess they called it API key on the user-facing docs side to be consistent with other LLM providers that use that terminology. Then someone implementing the feature in the SDK decided to name the env var in the way that describes the way the token is transported 🤷♂️ I'd be happy to rename all the variables/params to one or the other here if it helps |
8c2088d to
1beed86
Compare
|
@davidmhewitt thank you for contributing this! I would love to see this merged. |
|
Thanks a lot @davidmhewitt. This is a very needed feature. Hope it will be merged soon |
|
Thanks! Would also love to see this merged. |
|
@RobertCraigie Friendly ping on this! We could also rename |
|
Thanks! Could it be approved and merged? :) EDIT: for anyone that might stumble on this PR, it's now supported. anthropic-sdk-python/src/anthropic/lib/bedrock/_client.py Lines 136 to 162 in 717f90b |
|
Thank you for the PR - this was already addressed on |
Fixes #1079