-
Notifications
You must be signed in to change notification settings - Fork 412
feat(bedrock): support AWS_BEARER_TOKEN_BEDROCK env var #1080
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
base: main
Are you sure you want to change the base?
feat(bedrock): support AWS_BEARER_TOKEN_BEDROCK env var #1080
Conversation
src/anthropic/lib/bedrock/_auth.py
Outdated
| profile: str | None, | ||
| data: str | None, | ||
| ) -> dict[str, str]: | ||
| bedrock_bearer = os.getenv("AWS_BEARER_TOKEN_BEDROCK") |
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
Fixes #1079