Migrate DurableTask.ServiceBus ServiceBus SDK from Microsoft.Azure.ServiceBus to Azure.Messaging.ServiceBus#1014
Conversation
cgillum
left a comment
cgillum
left a comment
There was a problem hiding this comment.
A couple administrative comments (haven't looked at the core implementation quite yet):
|
/azp run |
|
EDIT: Azure Pipelines run triggered successfully. |
src/DurableTask.ServiceBus/Common/Abstraction/ServiceBusAbstraction.cs
Outdated
Show resolved
Hide resolved
src/DurableTask.ServiceBus/Common/Abstraction/ServiceBusAbstraction.cs
Outdated
Show resolved
Hide resolved
src/DurableTask.ServiceBus/Common/Abstraction/ServiceBusAbstraction.cs
Outdated
Show resolved
Hide resolved
|
/azp run |
|
Commenter does not have sufficient privileges for PR 1014 in repo Azure/durabletask |
| public async Task SetStateAsync(byte[] sessionState) | ||
| { | ||
| await this.session.SetStateAsync(sessionState); | ||
| if (sessionState == null) |
There was a problem hiding this comment.
this looks a bit weird, but it prevents InvalidArgumentExceptions and retains compatibility
|
/azp run |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
/azp run |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
/azp run |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
There's now a successful pipeline run associated with this PR.
I think there are some details about the GitHub<->Azure DevOps integration that I'm not familiar with. It looks like this is being blocked from merging based on a previous run. |
|
/azp run |
|
EDIT: Azure Pipelines run created successfully. |
cgillum
left a comment
cgillum
left a comment
There was a problem hiding this comment.
Everything looks good from my perspective. I confirmed that the CI is executing both the Azure Storage and Service Bus tests in a distributed way. I think we should be good to merge this now.
cgillum
left a comment
cgillum
left a comment
There was a problem hiding this comment.
Ah, I think I mispoke. It seems the tests are actually still just running the net462 variants, according to the logs:
Source filter: \bin\Debug\net462\DurableTask.AzureStorage.Tests.dll,\bin\Debug\net462\DurableTask.ServiceBus.Tests.dll,\bin\Debug\net462\DurableTask.Core.Tests.dll,\bin\Debug\net462\DurableTask.Emulator.Tests.dll,!**\obj**
We'll need to update the pipeline to run the .NET Core variant of these libraries.
|
/azp run |
| EDIT: Azure Pipelines run created successfully. |
|
I manually ran the |
3 participants
Validation strategies for .NET Core version
It has come to my attention that the .NET Core version of this library needs additional validation. This section describes validations performed manually.
Leveraging a real world prod service
Without revealing much about our internal design, my team uses DTF ServiceBus for running workflows in a .NET Core project. I've locally sideloaded this new version into a worker service running in a dev environment that consumes real world application orchestrations.
The tasks and orchestrations consumed by the worker running with the sideloaded library did not produce unhandled exceptions and did not behave in any way that suggested issues. Additionally, the worker was running in concert with other worker instances that were running the current stable version without Azure.Messaging.ServiceBus; I consider this a testament to its compatibility.
Running DurableTask.ServiceBus.Tests project
I also set up live ServiceBus and AzureStorage resources in my own Azure subscription and added their connections strings to the project. This allowed me to run the DurableTask.ServiceBus.Tests(netcoreapp3.1) project against real world resources and validate those scenarios. I am attaching a VSTest overview screenshot as proof of these results:
NOTE: All of the failing tests in this image are ALSO failing in the previous stable version (when I switch to
mainbranch).