Initial Service Bus crate #2863

RickWinter · 2025-08-05T04:00:10Z

ServiceBus crate

Initial ServiceBus crate

Create the initial structure for the ServiceBus crate. Includes tests, docs, and the basic clients:

Client
Sender
Receiver
Message Types
Error handling

Copilot

Pull Request Overview

This PR introduces the initial ServiceBus crate for Azure SDK for Rust, providing a comprehensive implementation for Azure Service Bus messaging capabilities. The implementation includes core clients, message types, authentication methods, and extensive test coverage.

Initial ServiceBus crate with Client, Sender, and Receiver components
Comprehensive test suite covering authentication, batching, scheduling, and topic/subscription scenarios
Azure DevOps CI pipeline configuration and test infrastructure setup

Reviewed Changes

Copilot reviewed 40 out of 42 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
ci.yml	Azure DevOps pipeline configuration for ServiceBus CI/CD
test-resources.bicep	Bicep template for provisioning test infrastructure resources
test-resources-post.ps1	Post-deployment script for retrieving connection strings
test-resources-pre.ps1	Pre-deployment setup script placeholder
tests/README.md	Comprehensive documentation for running live tests
tests/common/mod.rs	Common test utilities and environment variable helpers
tests/servicebus_*.rs	Extensive test suite covering all major ServiceBus functionality

Comments suppressed due to low confidence (3)

sdk/servicebus/azure_messaging_servicebus/tests/servicebus_client.rs

github-actions · 2025-08-05T17:54:28Z

API Change Check

APIView identified API level changes in this PR and created the following API reviews

azure_messaging_servicebus

sdk/servicebus/ci.yml

sdk/servicebus/.dict.txt

heaths · 2025-08-05T20:14:46Z

sdk/servicebus/azure_messaging_servicebus/test-resources.bicep

Test resource files are generally in the service directory so that any crates therein share resources, as is most often common. If any HTTP recorded tests, the assets.json should go in the same directory.

heaths · 2025-08-05T20:15:40Z

sdk/servicebus/azure_messaging_servicebus/test-resources.bicep

+    zoneRedundant: false
+  }
+
+  resource rootSharedAccessKey 'AuthorizationRules@2022-10-01-preview' = {


Why specify the api-version? Child resources default to the same api-version as the parent.

I don't recall where I found the example for this. I followed the example, and this is how it was done.

The example is unnecessarily complex. I'd drop them in a separate PR. Let's exhibit best practices. I mean, this api-version is even older than the parent resource. I've never once overridden the api-version. It's possible, but unnecessary in the vast majority of cases.

heaths · 2025-08-05T20:18:02Z

sdk/servicebus/azure_messaging_servicebus/test-resources-post.ps1

+Write-Host "Retrieving Service Bus connection strings..."
+
+try {
+  $connectionString = az servicebus namespace authorization-rule keys list `


Why use the Azure CLI in Azure PowerShell instead of using Azure PowerShell, which shares the same auth? If this is copied from other languages, sure: I designed these to be sharable. But I'd seriously consider all languages fix their scripts to not require that Azure CLI just happened to be auth'd as the same user and same sub as Azure PowerShell. That's just asking for trouble. See sdk/keyvault's post-script for an example of how I did it across languages for post-provisioning scripts.

sdk/servicebus/azure_messaging_servicebus/src/receiver.rs

sdk/servicebus/azure_messaging_servicebus/src/sender.rs

sdk/servicebus/azure_messaging_servicebus/Cargo.toml

sdk/servicebus/azure_messaging_servicebus/LIVE_TESTS.md

LarryOsterman · 2025-08-05T19:44:59Z

sdk/servicebus/azure_messaging_servicebus/LIVE_TESTS.md

+- Azure Service Bus namespace with Standard tier
+- Test queue named `testqueue`  
+- Test topic named `testtopic` with subscription `testsubscription`
+- Shared access keys for testing (RootManageSharedAccessKey, ListenOnly, SendOnly)


We should not support shared access keys in Rust.

sdk/servicebus/azure_messaging_servicebus/src/sender.rs

LarryOsterman · 2025-08-06T21:30:34Z

sdk/servicebus/azure_messaging_servicebus/tests/common/mod.rs

+static INIT_LOGGING: std::sync::Once = std::sync::Once::new();
+
+#[allow(dead_code)]
+pub fn setup() {


If you use #[recorded::test(live)], you get tracing for free, in addition, you can use recording.credential() and avoid having to configure a test credential - that way it works in both pipelines and locally.

Initial ServiceBus crate Create the initial structure for the ServiceBus crate. Includes tests, docs, and the basic clients: - Client - Sender - Receiver - Message Types - Error handling

heaths

Would liek to see all the client and method options fixed, at least. Added a few more nits, but those could be done in a subsequent PR.

heaths · 2025-08-12T23:05:35Z

sdk/servicebus/azure_messaging_servicebus/src/client.rs

+impl ServiceBusClient {
+    /// Creates a helper function to build AmqpConnectionOptions from ServiceBusClientOptions.
+    fn build_connection_options(
+        options: &ServiceBusClientOptions,


This must be Option<ServiceBusClientOptions> per guidelines. This is for consistency across all Azure SDK for Rust crates including EH. This was discussed to death in early design discussions and agreed upon by all party members.

Client and method options should be Cloneable and, as explained offline (where you also said you wanted to even document a recommended file layout for consistency), we decided not to hide allocations. We end up calling unwrap_or_default() which would require allocation in the golden path so, like parameters, we don't do that.

heaths · 2025-08-12T23:06:39Z

sdk/servicebus/azure_messaging_servicebus/src/client.rs

+    pub(crate) async fn new_internal(
+        fully_qualified_namespace: &str,
+        credential: Arc<dyn TokenCredential>,
+        options: ServiceBusClientOptions,


Related to the previous comment, you're not even consistent: there it was a reference, here it's a value. And this would be required, so now everyone has to pass a ServiceBusClientOptions::default() instead of None. Please follow guidelines for the best developer experience.

heaths · 2025-08-12T23:07:00Z

sdk/servicebus/azure_messaging_servicebus/src/client.rs

+    pub async fn create_receiver(
+        &self,
+        queue_name: &str,
+        options: Option<CreateReceiverOptions>,


Yeah like this, but yet another variation of inconsistency.

heaths · 2025-08-12T23:12:12Z

sdk/servicebus/ci.yml

+pr:
+  branches:
+    include:
+      - main
+      - feature/*
+      - release/*
+      - hotfix/*
+  paths:
+    include:
+      - sdk/servicebus/


Not used AFAIK. All pull requests go through eng/pipelines/pullrequest.yml or something like that.

heaths · 2025-08-12T23:13:00Z

sdk/servicebus/azure_messaging_servicebus/Cargo.toml

+async-lock.workspace = true
+async-stream.workspace = true
+async-trait.workspace = true
+azure_core.workspace = true


You want to disable default features here. See EH for an example.

heaths · 2025-08-12T23:36:12Z

sdk/servicebus/azure_messaging_servicebus/src/client.rs

+    AmqpValue,
+};
+use std::sync::Arc;
+use url::Url;


Nit: though rust-analyzer probably did this, try to export from azure_core only in case we ever redefine these. That said, we'd just go through and fix them anyway but doing so now helps avoid future potential problems e.g., Url, Uuid, Method, et. al.(?).

heaths · 2025-08-12T23:37:17Z

sdk/servicebus/azure_messaging_servicebus/src/models/mod.rs

+#[non_exhaustive]
+pub enum EntityState {
+    /// The entity is active and can send/receive messages.
+    #[serde(rename = "Active")]


Nit: all these renames are unnecessary if you just use #[serde(rename_all = "PascalCase")] similar to what you did on line 31 below.

heaths · 2025-08-12T23:38:58Z

sdk/servicebus/azure_messaging_servicebus/src/client.rs

+    ///     Ok(())
+    /// }
+    /// ```
+    pub fn builder() -> ServiceBusClientBuilder {


Client builders should go in clients. Seems we don't really have a guideline that spells it out, but I could add one. But I would hope it would be obvious enough: client builds in clients, model builders (unlikely, but possible) in models.

heaths · 2025-08-12T23:40:02Z

sdk/servicebus/azure_messaging_servicebus/src/client.rs

+    pub async fn create_sender(
+        &self,
+        queue_or_topic_name: &str,
+        _options: Option<CreateSenderOptions>,


If you're getting dead_code or unused_variables on parameters, it means your public methods aren't being exported publicly correctly. This is a warning side you shouldn't just suppress by prefacing it with "_".

heaths · 2025-08-12T23:41:27Z

sdk/servicebus/azure_messaging_servicebus/src/client.rs

+    /// #[tokio::main]
+    /// async fn main() -> Result<(), Box<dyn std::error::Error>> {


Examples don't typically show boiler plate. Hide like we do in other methods. Preface these with a # and unindent the following lines. Don't forget to hide the final Ok(()) and } with a # as well. That's not only idiomatic, it's consistent throughout our codebase (or should be).

Copilot AI review requested due to automatic review settings August 5, 2025 04:00

RickWinter requested review from heaths, ronniegeraghty and LarryOsterman as code owners August 5, 2025 04:00

Copilot AI reviewed Aug 5, 2025

View reviewed changes

heaths changed the title ~~ServiceBus crate~~ Initial Service Bus crate Aug 5, 2025

heaths requested changes Aug 5, 2025

View reviewed changes

heaths marked this pull request as draft August 6, 2025 20:06

LarryOsterman requested changes Aug 6, 2025

View reviewed changes

RickWinter force-pushed the service_bus branch 8 times, most recently from 535a370 to a1be98a Compare August 11, 2025 22:49

RickWinter marked this pull request as ready for review August 11, 2025 23:02

RickWinter force-pushed the service_bus branch 2 times, most recently from de16b27 to d7ff1de Compare August 12, 2025 21:23

ServiceBus crate

5acc786

Initial ServiceBus crate Create the initial structure for the ServiceBus crate. Includes tests, docs, and the basic clients: - Client - Sender - Receiver - Message Types - Error handling

RickWinter force-pushed the service_bus branch from d7ff1de to 5acc786 Compare August 12, 2025 22:31

heaths requested changes Aug 12, 2025

View reviewed changes

		/// #[tokio::main]
		/// async fn main() -> Result<(), Box<dyn std::error::Error>> {

Initial Service Bus crate #2863

Are you sure you want to change the base?

Initial Service Bus crate #2863

Uh oh!

Conversation

RickWinter commented Aug 5, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Aug 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

API Change Check

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

heaths Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

heaths left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Aug 5, 2025 •

edited

Loading

heaths Aug 12, 2025 •

edited

Loading