-
Notifications
You must be signed in to change notification settings - Fork 285
Implement EventHubs token refresh #2476
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
Implement EventHubs token refresh #2476
Conversation
…-connection; removed credentials from authorize_path since they're not needed at that point
API change check APIView has identified API level changes in this PR and created following API reviews. |
…lementation; tightened up token_refresh test
…pset if they are not defined
sdk/eventhubs/azure_messaging_eventhubs/src/common/connection_manager.rs
Outdated
Show resolved
Hide resolved
Co-authored-by: Heath Stewart <[email protected]>
…ion logic more robust
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 implements Event Hubs token refresh support and updates the AMQP library to use async_trait for cleaner, asynchronous trait implementations. Key changes include:
- Refactoring AMQP sender, receiver, connection, and management APIs to use async_trait along with adding +Send bounds on parameters.
- Introducing an asynchronous task spawner in azure_core, with implementations for tokio, standard, and wasm targets.
- Minor dependency and error mapping updates in Cargo.toml and process modules.
Reviewed Changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
sdk/core/azure_core_amqp/src/sender.rs | Updated async API definitions and added +Send bounds on parameters. |
sdk/core/azure_core_amqp/src/receiver.rs | Converted API functions to async functions with appropriate +Send bounds. |
sdk/core/azure_core_amqp/src/noop.rs | Updated noop implementations with async_trait. |
sdk/core/azure_core_amqp/src/management.rs | Refactored management API to use async_trait and simplified method definitions. |
sdk/core/azure_core_amqp/src/fe2o3/*.rs | Applied async_trait refactoring and added Send bounds across session, sender, receiver, etc. |
sdk/core/azure_core_amqp/src/connection.rs | Transformed connection APIs to async functions with awaited inner calls. |
sdk/core/azure_core_amqp/src/cbs.rs | Updated cbs APIs to use async_trait. |
sdk/core/azure_core_amqp/Cargo.toml | Added async-trait as a workspace dependency. |
sdk/core/azure_core/src/task/* | Introduced new task spawner implementations and comprehensive tests for asynchronous tasks. |
sdk/core/azure_core/src/process/standard.rs | Simplified error mapping using io::Error::other. |
sdk/core/azure_core/src/lib.rs | Exposed the new task module. |
… which exposes a wait primitive
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.
Just a couple nits/tips. I'd like to see a "flattening" of the standard_spawn.rs
but I wouldn't hold up this PR for it. It made it a bit hard to follow because all of a sudden is felt like reading...gasp...VB and keeping prefaced whitespace in mind because important. 😉
Implement Event Hubs authorization token refresh.
Because AMQP is a connection-oriented protocol (unlike HTTP which is nominally a connectionless protocol), authorization tokens used by AMQP can expire while the connections are still active. To implement this, each EventHubs (and ServiceBus) client spawns a background task to refresh authorization tokens.
There are several elements to this PR:
async_trait
trait rather than implementing async traits manually. This uncovered several subtle bugs in the AMQP implementation (mostly theSend
trait being missing on certain types).ConnectionManager
type to add a token refresh task which is launched when a path is first authorized.ConnectionManager
refactoring, thecredential
field was moved from the service client objects into the connection manager and thecredential
field was removed from theauthorize_path
method.StdExecutor
type to simplify the error case.Note that the ConnectionManager has a number of
#[cfg(test)]
test hooks to enable unit testing the connection manager token refresh logic. That allows for in-isolation testing of the token refresh logic while not significantly impeding the functionality.