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

Improve support for temporary IAM credentials in chat_bedrock() #266

Merged
merged 1 commit into from
Jan 27, 2025

Conversation

atheriel
Copy link
Collaborator

@atheriel atheriel commented Jan 23, 2025

Many AWS IAM credentials expire, but previously we ignored this by looking up credentials only once, in chat_bedrock(). This commit introduces a caching layer that handles expiry and moves credential retrieval closer to request time, instead.

The design is almost identical to httr2's OAuth token caching mechanism, but I had to re-implement various pieces because not all of that API is exported.

(We could probably introduce a req_aws_credentials() function to httr2 itself that would handle this, but that might tie us too closely to the semantics of paws.common.)

Unit tests are included.

Closes #261.

@atheriel
Copy link
Collaborator Author

I haven't been able to a real test of this yet. Should maybe hold off on merging until then.

@hadley
Copy link
Member

hadley commented Jan 23, 2025

It does feel like maybe httr2 could provide some generic interface for this (i.e. some callback that determines if an error response was due to an out of date token, and another function that updates that token?)

@atheriel
Copy link
Collaborator Author

A lot of the pieces are already there with req_auth_sign() -- which isn't exported -- but I think much of the value is in implementing caching correctly, which that doesn't help with.

@hadley
Copy link
Member

hadley commented Jan 23, 2025

I wonder if you can abuse req_oauth() to do what you want?

@atheriel atheriel force-pushed the aws-credential-refresh branch from 6037b07 to 66d0137 Compare January 24, 2025 00:32
Many AWS IAM credentials expire, but previously we ignored this by
looking up credentials only once, in `chat_bedrock()`. This commit
introduces a caching layer that handles expiry and moves credential
retrieval closer to request time, instead.

The design is almost identical to httr2's OAuth token caching mechanism,
but I had to re-implement various pieces because not all of that API is
exported.

(We could probably introduce a `req_aws_credentials()` function to
`httr2` itself that would handle this, but that might tie us too closely
to the semantics of `paws.common`.)

Unit tests are included.

Closes #261.

Signed-off-by: Aaron Jacobs <[email protected]>
@atheriel atheriel force-pushed the aws-credential-refresh branch from 66d0137 to 1ce004a Compare January 27, 2025 17:52
@atheriel atheriel merged commit bdc63b1 into main Jan 27, 2025
11 checks passed
@atheriel atheriel deleted the aws-credential-refresh branch January 27, 2025 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

chat_bedrock() does not handle AWS credentials that expire
2 participants