-
Notifications
You must be signed in to change notification settings - Fork 272
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
Register IPlatformInformationService in AddDurableClientFactory() #1727
Conversation
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.
Looks good to me, thank you for cleaning up my post-merge mess 😄 !
Left a non-blocking comment
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.
Approved with suggestions.
public async Task DurableClient_AzureStorage_SuccessfulSetup() | ||
{ | ||
string orchestratorName = nameof(TestOrchestrations.SayHelloInline); | ||
using (ITestHost host = TestHelpers.GetJobHost( |
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.
In the long run, we may be able to simplify this test (and the additional helper methods we added) by making a generic .NET Core host that calls builder.Services.AddDurableClientFactory();
. That is more inline with what end customers would be doing, and then you don't need all of the changes to TestHelpers to accommodate wiring this into the JobHost.
In the interest of time and the code freeze today, I am fine postponing this.
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.
Looks great!
These changes incorporate the changes from #1706 and register
IPlatformInformationService
inAddDurableClientFactory()
. Without registeringIPlatformInformationService
, creating a DurableClient with Azure Storage fails because it can't find an implementation ofIPlatformInformationService
inAzureStorageDurabilityProviderFactory
. This PR also includes a test in DurableClientBaseTests to help us catch if any new services need to be registered inAddDurableClientFactory()
in the future.Pull request checklist
pending_docs.md
release_notes.md