Skip to content
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

Adding Storage Backend Selection in host.json #1702

Merged
merged 11 commits into from
Mar 12, 2021
Merged

Conversation

bachuv
Copy link
Collaborator

@bachuv bachuv commented Mar 1, 2021

These changes allow customers to configure which storage backend they want their app to use (e.g. Azure Storage, MicrosoftSQL, or Netherite) with a new type attribute under durableTask/storageProvider in host.json (example below). The extension will register all storage backend services and then choose which one to use in the DurableTaskExtension constructor. Adding the ability to register all backends and then choose a storage backend allows us to deploy all storage backend packages through extension bundles.

The main changes in this PR are allowing the extension to register a collection of services through dependency injection, adding a Name property in IDurabilityProviderFactory, and choosing the DurabilityProvider by comparing a customers configured type with the Name property in the available IDurabilityProviderFactory implementations.

{
  "version": "2.0",
  "extensions": {
    "durableTask": {
      "storageProvider": {
        "type": "MicrosoftSQL",
        "connectionStringName": "SQLDB_Connection"
      }
    }
  }
}

Resolves #1666

Pull request checklist

  • My changes do not require documentation changes
    • Otherwise: Documentation PR is ready to merge and referenced in pending_docs.md
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • I have added all required tests (Unit tests, E2E tests)

@bachuv bachuv added this to the v2.4.2 milestone Mar 1, 2021
@bachuv bachuv self-assigned this Mar 1, 2021
Copy link
Collaborator

@cgillum cgillum left a 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! Just a few nitpicks from my POV.

src/WebJobs.Extensions.DurableTask/DurableTaskExtension.cs Outdated Show resolved Hide resolved
test/Common/DurableClientBaseTests.cs Outdated Show resolved Hide resolved
test/Common/DurableTaskLifeCycleNotificationTest.cs Outdated Show resolved Hide resolved
test/Common/DurableTaskLifeCycleNotificationTest.cs Outdated Show resolved Hide resolved
test/Common/DurableTaskLifeCycleNotificationTest.cs Outdated Show resolved Hide resolved
test/Common/DurableTaskLifeCycleNotificationTest.cs Outdated Show resolved Hide resolved
test/Common/HttpApiHandlerTests.cs Outdated Show resolved Hide resolved
test/FunctionsV1/PlatformSpecificHelpers.FunctionsV1.cs Outdated Show resolved Hide resolved
test/FunctionsV2/DurableTaskListenerTests.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@ConnorMcMahon ConnorMcMahon left a comment

Choose a reason for hiding this comment

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

A few nits, but overall looks great!

Copy link
Collaborator

@cgillum cgillum left a comment

Choose a reason for hiding this comment

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

This looks good to me. Just a couple non-blocking comments.

Copy link
Contributor

@ConnorMcMahon ConnorMcMahon left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Contributor

@ConnorMcMahon ConnorMcMahon left a comment

Choose a reason for hiding this comment

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

I think the current tests need some tweaking.

test/FunctionsV2/StorageProviderSelectionTests.cs Outdated Show resolved Hide resolved
Copy link
Contributor

@ConnorMcMahon ConnorMcMahon left a comment

Choose a reason for hiding this comment

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

The PR content is solid at this point! Just address some of the stylistic and naming suggestions before merging.

test/Common/TestHelpers.cs Outdated Show resolved Hide resolved
test/Common/TestHelpers.cs Outdated Show resolved Hide resolved
test/FunctionsV2/StorageProviderSelectionTests.cs Outdated Show resolved Hide resolved
test/FunctionsV2/StorageProviderSelectionTests.cs Outdated Show resolved Hide resolved
test/FunctionsV2/StorageProviderSelectionTests.cs Outdated Show resolved Hide resolved
test/FunctionsV2/StorageProviderSelectionTests.cs Outdated Show resolved Hide resolved
@bachuv bachuv merged commit 56e9c45 into dev Mar 12, 2021
@bachuv bachuv deleted the vabachu/backendconfig branch March 12, 2021 02:19
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.

Ability to select alternate storage backend in host.json
4 participants