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

Fix spurious validation errors for non-Azure-Storage backends #2015

Merged
merged 3 commits into from
Jan 31, 2022

Conversation

sebastianburckhardt
Copy link
Collaborator

The constructor of AzureStorageDurabilityProviderFactory performs validation on the options, even if this provider is not the provider that is actually selected for use.

This causes problems with spurious validation exceptions, e.g. with the Netherite provider if the latter uses more than 16 partitions.

This fix performs a check in the constructor of AzureStorageDurabilityProviderFactory to see if it is actually the selected provider, and skips the constructor if not. This avoids spurious errors caused by the configuration not being appropriate for this provider.

Issue describing the changes in this PR

resolves #2013

Pull request checklist

  • My changes do not require documentation changes
  • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
  • My changes do not require any extra work to be leveraged by OutOfProc SDKs

…Factory if the provider is not configured for use.
Copy link
Member

@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.

LGTM!

@cgillum
Copy link
Member

cgillum commented Jan 27, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@sebastianburckhardt
Copy link
Collaborator Author

Some tests are consistently failing. Investigating now.

It looks like the problem is that some tests are using custom provider types, such as AzureStorageShortenedTimerDurabilityProviderFactory, so my code is overly simplistic (I cannot simply check whether the storage provider is the default provider or not).

@sebastianburckhardt sebastianburckhardt merged commit 0c0cc87 into dev Jan 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot set PartitionCount larger than 16 for Netherite
2 participants