AMQP 1.0 Support for Service Bus SDK #1185
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Hi @minghuaw, are you still using the service bus test resource I set up for you? |
No. I am currently only using the ones I created under my own SB namespace |
18 tasks
…ure-sdk-for-rust into servicebus_over_amqp
|
@minghuaw, thank you for continuing to update this. ❤️ |
|
We really like seeing this work, but it is not something those of us volunteering can take on. It is probably best to create a new git repo for |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a partial fullfillment of service bus SDK using the AMQP 1.0 protocol (#643). The overall design follows that of the dotnet SDK.
What's added
The following key public types are added in this PR.
ServiceBusClientServiceBusSenderServiceBusReceiverServiceBusSessionReceiverServiceBusRuleManagerServiceBusMessageBatchThis PR also changes the minimum required rust version to 1.65 due to the use of generic associate types.
What's removed
The old
reqwestbased HTTP client (QueueClient,SubscriptionReceiver,TopicClient, andTopicSender) and the corresponding dependencies/examples/tests are removed.What's not implemented yet
ServiceBusProcessorServiceBusSessionProcessorServiceBusAdministrationClientTesting
Mock testing (using the
mockallcrate) has been applied to some unit tests but not implemented for integration testing, and integration testing relies heavily on live testing. Because of this, the environment variables listed below must be set in order to runcargo test. For testing on a local machine, it is recommended to put these environment variables in a.envfile that is placed at the root of this crate (ie.azure-sdk-for-rust/sdk/messaging_servicebus/.env).SERVICE_BUS_CONNECTION_STRING- full connection string to a service bus namespace. It should look like"Endpoint=sb://<your-namespace>.servicebus.windows.net/;SharedAccessKeyName=<your-policy>;SharedAccessKey=<your-key>"SERVICE_BUS_NAMESPACE-"<your-namespace>.servicebus.windows.net"SERVICE_BUS_SAS_KEY_NAME- shared access key name, for example"RootManageSharedAccessKey"SERVICE_BUS_SAS_KEY- shared access key value. This is essentially the field"<your-key>"in the full connection stringSERVICE_BUS_QUEUE- a queue that does NOT have session enabledSERVICE_BUS_SESSION_QUEUE- a queue that has session enabledSERVICE_BUS_TOPIC- a topic whose subscriptions will be used for non-session topic/subscription testingSERVICE_BUS_SUBSCRIPTION- a subscription ofSERVICE_BUS_TOPIC, and it does NOT have session enabledSERVICE_BUS_SESSION_TOPIC- a topic whose subscriptions will be used for session-enabled topic/subscription testingSERVICE_BUS_SESSION_SUBSCRIPTION- a subscription ofSERVICE_BUS_SESSION_TOPIC, and it must have session enabledSERVICE_BUS_RULE_FILTER_TEST_TOPIC- a topic whose subscription will be used to test get/delete/create rule filters. It is recommended to have a separate topic that is different fromSERVICE_BUS_TOPICandSERVICE_BUS_SESSION_TOPICto avoid accidentally affecting the subscription testing with the rule filters.SERVICE_BUS_RULE_FILTER_TEST_SUBSCRIPTION- a subscription ofSERVICE_BUS_RULE_FILTER_TEST_TOPICthat will be used for get/delete/create rule filter testing. It is recommended to have a separate topic/subscription that is different fromSERVICE_BUS_TOPIC/SERVICE_BUS_SUBSCRIPTIONandSERVICE_BUS_SESSION_TOPIC/SERVICE_BUS_SESSION_SUBSCRIPTIONto avoid accidentally affecting the subscription testing with the rule filters.Most integration test cases assume an empty queue/subscription at the beginning of the test and should leave an empty queue/subscription at the end of the test. So it is important to make sure only one live test is running for the testing queue/topic/subscription.
There is one long test in the integration tests (
fn send_to_queue_every_minute_for_two_hour()inmessaging_servicebus/tests/long_tests.rs) that is ignored in the usualcargo test. This test can be explicitly executed withcargo test --test long_tests -- --ignored --exact --nocapturewhich will also periodically print out message sent and message received. This test could also be used for testing recovery against network interruptions.I can provide my testing namespace/queue/topic/subscription (actually a copy of my
.envfile) in an email if necessary.Documentation
The current implementation sets a rust doc flag
"docsrs"to conditionally import types for intra-doc-links. To test the doc locally, the following command could be used. Please note that a nightly toolchain is needed (the nightly toolchain will used for the doc only, the crate itself doesn't need nightly at all)cargo +nightly doc --no-deps --open -Z unstable-options --config "build.rustdocflags=[\"--cfg\", \"docsrs\"]"Most of the documentations are ported from the dotnet SDK. More detailed explanation and/or examples might be necessary
Limitations and discussions
I have not got the time to review whether the APIs are cancel safe, but the send and recv method provided by the upstream AMQP crate have been reviewed and should be cancel safe.
There are a few design decisions that, I think, could be discussed
AzureNamedKeyCredentialandAzureSasCredentialshould be moved to theazure_corecrate (which is the way that is implemented in the dotnet sdk)?ServiceBusRetryPolicyandBasicRetryPolicy) essentially mimic what the dotnet sdk does. However, there is another retry policy trait in theazure_corecrate (azure_core::RetryPolicy), which is not exactly the same. Should this implementation move to useazure_core::RetryPolicyinstead?There is a known problem in the current implementation. Sometimes, upon certian network interruptions (eg. switching from one wireless network to another), the sending operation may need to wait for
ServiceBusRetryOptions::delay(default to 1 minute) before it starts to try to recover the connection and retry the sending operation. Thetests/long_tests.rshas been used for testing network interruptions by manually disconnecting/reconnecting or switching the network.Another problem is logging. Both
logandtracingare popular in the rust ecosystem. Would it make sense to use a feature to allow user to choose which logging crate to use? (this will likely be in the next PR). Ths current use oflogalso definitely has room for improvement.