-
Notifications
You must be signed in to change notification settings - Fork 283
Add GetTokenOptions #2629
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?
Add GetTokenOptions #2629
Conversation
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
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.
Pull Request Overview
This PR extends the TokenCredential
API to accept an optional GetTokenOptions
parameter and updates all existing credentials, cache logic, and examples to propagate it (using None
by default). It also refactors the token cache to pass options through to the fetch callback.
- Extend
TokenCredential::get_token
to acceptOption<GetTokenOptions>
and add theGetTokenOptions
type. - Update every credential implementation (and calling sites) to accept and forward the new
options
parameter. - Refactor
TokenCache
to accept and pass options to the provided callback function.
Reviewed Changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
sdk/core/azure_core/src/credentials.rs | Added GetTokenOptions and updated trait signature |
sdk/identity/azure_identity/src/credentials/cache.rs | Refactored cache to accept options and callback |
sdk/identity/azure_identity/src/credentials/imds_managed_identity_credentials.rs | Updated get_token signature and calls |
Comments suppressed due to low confidence (1)
sdk/identity/azure_identity/src/credentials/cache.rs:30
- [nitpick] No tests currently pass a non-
None
GetTokenOptions
through the cache. Consider adding a test case that supplies a realGetTokenOptions
to ensure it is correctly propagated to the callback.
pub(crate) async fn get_token<'a, C, F>(
impl TokenCache { | ||
pub(crate) fn new() -> Self { | ||
Self(RwLock::new(HashMap::new())) | ||
} | ||
|
||
pub(crate) async fn get_token( | ||
pub(crate) async fn get_token<'a, C, F>( |
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.
[nitpick] Consider renaming the callback
parameter to a more descriptive name, such as fetch_fn
, to clarify that it is responsible for fetching a fresh token when the cache misses or expires.
Copilot uses AI. Check for mistakes.
Closes #2566