Skip to content

Refactored RecoverableConnection #2670

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

Merged

Conversation

LarryOsterman
Copy link
Member

@LarryOsterman LarryOsterman commented Jun 5, 2025

Refactored the RecoverableConnection struct to split out the individual recoverable types into their own modules to simplify and concentrate the logic throughout the recoverable code logic.

There's not a lot of actual code changing here, mostly this is about moving code around to isolate individual implementations of Recoverable types. Also renamed all the types from AzureXxxClient to RecoverableXxx to make it clearer what's going on.

Bonus: Changed jitter parameters to token refresher to be Duration structs instead of number of seconds - that allows tests to use extremely fine grained jitter amounts.

@LarryOsterman LarryOsterman marked this pull request as ready for review June 13, 2025 16:29
@Copilot Copilot AI review requested due to automatic review settings June 13, 2025 16:29
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Refactor the RecoverableConnection logic by splitting out sender, receiver, management, and CBS handling into their own modules, and update all imports and references accordingly.

  • Split the monolithic recoverable_connection module into recoverable/sender, recoverable/receiver, recoverable/management, and recoverable/cbs
  • Updated imports across producer, consumer, common, management, and authorizer modules to use the new recoverable module
  • Renamed connection_manager to recoverable_connection for clarity in consumer

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
sdk/eventhubs/azure_messaging_eventhubs/src/producer/mod.rs Switch to RecoverableSender from AmqpSenderClient and update import
sdk/eventhubs/azure_messaging_eventhubs/src/consumer/mod.rs Rename connection_manager to recoverable_connection and import from new module
sdk/eventhubs/azure_messaging_eventhubs/src/consumer/event_receiver.rs Update RecoverableConnection import path
sdk/eventhubs/azure_messaging_eventhubs/src/common/recoverable/sender.rs Added RecoverableSender implementation with retry logic
sdk/eventhubs/azure_messaging_eventhubs/src/common/recoverable/receiver.rs Added RecoverableReceiver implementation with retry and timeout
sdk/eventhubs/azure_messaging_eventhubs/src/common/recoverable/management.rs Extracted RecoverableManagementClient for management APIs
sdk/eventhubs/azure_messaging_eventhubs/src/common/recoverable/cbs.rs Extracted RecoverableClaimsBasedSecurity for CBS authorization
sdk/eventhubs/azure_messaging_eventhubs/src/common/recoverable/mod.rs Consolidated recoverable submodules and re-export types
sdk/eventhubs/azure_messaging_eventhubs/src/common/mod.rs Replace recoverable_connection module with recoverable and adjust exports
sdk/eventhubs/azure_messaging_eventhubs/src/common/management.rs Updated RecoverableConnection import path
sdk/eventhubs/azure_messaging_eventhubs/src/common/authorizer.rs Updated RecoverableConnection import and refined jitter types
Comments suppressed due to low confidence (3)

sdk/eventhubs/azure_messaging_eventhubs/src/common/recoverable/sender.rs:130

  • [nitpick] Update the unimplemented message to reference RecoverableSender instead of AmqpSenderClient to match the surrounding API naming.
unimplemented!("AmqpSenderClient does not support send_ref operation");

sdk/eventhubs/azure_messaging_eventhubs/src/common/recoverable/receiver.rs:76

  • [nitpick] Change the unimplemented message to refer to RecoverableReceiver rather than AmqpReceiverClient for consistency.
unimplemented!("AmqpReceiverClient does not support attach operation");

sdk/eventhubs/azure_messaging_eventhubs/src/common/recoverable/management.rs:130

  • [nitpick] Update this unimplemented message to mention RecoverableManagementClient instead of AmqpManagementClient to align with the struct name.
unimplemented!("AmqpManagementClient does not support attach operation");

@LarryOsterman LarryOsterman requested a review from heaths June 16, 2025 20:59
@LarryOsterman LarryOsterman merged commit 0a29a27 into Azure:main Jun 16, 2025
18 checks passed
@LarryOsterman LarryOsterman deleted the larryo/update_token_refresh_jitter branch June 16, 2025 21:43
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.

2 participants