azure-sdk-for-go
azure-sdk-for-go copied to clipboard
Azure
azcontainerregistry: delegate token caching
azcontainerregistry: decouple token exchange from request specifics
I would like to propose a generally-useful token cache in a future commit. Managing the refresh tokens and exchanging them for access tokens does not need to rely specifically on the authentication policy or the specific client request being made, so this commit decouples the two concerns, thereby enabling the future refactor to be simpler.
Signed-off-by: Steve Kuznetsov [email protected]
azcontainerregistry: delegate token caching
Mechanically refactor token caching, retrieval and exchange behavior from the authentication policy into a dedicated object, allowing us to separate token management concerns from Azure client policy concerns.
Signed-off-by: Steve Kuznetsov [email protected]
azcontainerregistry: move code
This commit simply moves code from one location to another, broken out from other commits for ease of review.
Signed-off-by: Steve Kuznetsov [email protected]
Builds on top of #23270
Proving out a separation of concerns that could solve https://github.com/Azure/azure-sdk-for-go/issues/20571
Thank you for your contribution @stevekuznetsov! We will review the pull request and get back to you soon.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
I don't see anything wrong with this, however I also don't see a reason to make these changes, because they don't affect client behavior or the module's public API. Can you please describe the scenario motivating this? I take it you want to replace the ACR token cache of a BlobClient or Client instance with a custom implementation. Why is the current internal caching function not sufficient?
@chlowell were you aware of the conversation happening on https://github.com/Azure/azure-sdk-for-go/issues/20571? Sorry if I was not clear - I 100% agree with you that the changes here as they are will not be user-visible, but their value comes from the context of that issue, specifically what I laid out in this comment.
Hi @stevekuznetsov. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.
Would be great to do it but not sure if maintainers want this feature.
Hi @stevekuznetsov. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.
Hi @stevekuznetsov. Thank you for your contribution. Since there hasn't been recent engagement, we're going to close this out. Feel free to respond with a comment containing /reopen if you'd like to continue working on these changes. Please be sure to use the command to reopen or remove the no-recent-activity label; otherwise, this is likely to be closed again with the next cleanup pass.