dotnet / orleans

Cloud Native application framework for .NET
https://docs.microsoft.com/dotnet/orleans
MIT License
10.02k stars 2.02k forks source link

Update Azure Storage libraries #5479

Closed seniorquico closed 4 years ago

seniorquico commented 5 years ago

We keep experiencing the issue described in #5003. The fix in the upstream repo was rejected as the Azure Table storage service/API/SDKs were being transferred from the Azure Storage team to the Cosmos DB team.

The Cosmos DB team has released a new SDK for Azure Table storage with support for .NET Standard. Interestingly, the team chose to release this under a new package ID, Microsoft.Azure.Cosmos.Table, as opposed to updating their existing Microsoft.Azure.CosmosDB.Table package.

Additionally, the Azure Storage SDK has since been broken out into separate packages. We now have:

The new Common library contains, CloudStorageAccount, StorageException, retry policies, etc..

I took a stab at updating the package references in the Orleans solution, hoping this will bring a fix to our issue. Unfortunately, I encountered several problems.

First, the new, refactored Azure Storage SDKs have picked up a transitive dependency on Microsoft.Azure.KeyVault.Core with a specified version of ">=1.0.0". NuGet restores the lowest applicable version, 1.0.0. Unfortunately, that particular version does not provide a library under a TFM for .NET Standard. This causes NuGet errors like the following:

error NU1701: Package 'Microsoft.Azure.KeyVault.Core 1.0.0' was restored using '.NETFramework,Version=v4.6.1' instead of the project target framework '.NETStandard,Version=v2.0'. This package may not be fully compatible with your project.

I had to promote Microsoft.Azure.KeyVault.Core to an explicit dependency to get an updated version with a library under a TFM for .NET Standard. Is there another option besides adding an explicit dependency? If not, is it preferred to specify the oldest or newest version in the new, explicit dependency?

Next, while the Microsoft.Azure.Cosmos.Table library keeps a mostly consistent API with prior versions of WindowsAzure.Storage, all of the "common" classes were duplicated and the namespaces updated. This is problematic, for example, in shared classes like AzureStorageUtils where CloudStorageAccount and StorageException now exist in both the Microsoft.Azure.Cosmos.Table and Microsoft.WindowsAzure.Storage namespaces and lead to ambiguous references. I noticed that the Microsoft.Azure.CosmosDB.Table package (which only supports NetFx) was recently updated to take a dependency on Microsoft.Azure.Storage.Common (thus reusing CloudStorageAccount, StorageException, etc.). I reached out to the Cosmos DB team to see if a similar future is in store for Microsoft.Azure.Cosmos.Table.

Next, the classes NameValidator and StorageErrorCodeStrings were changed to an internal visibility in Microsoft.Azure.Cosmos.Table (they remain public in Microsoft.Azure.Storage.Common).

Finally, even though the NuGet package indicates Microsoft.Azure.Cosmos.Table is released under an MIT license, the source code is not yet available. I also reached out to the Cosmos DB team for guidance.

So, how to proceed? Should the Blob/Queue code get separated from the Table code? That sounds undesirable if there's some confirmation the Microsoft.Azure.Cosmos.Table library will end up taking on a Microsoft.Azure.Storage.Common dependency. While waiting for a Cosmos DB team response, the refactored Azure Storage SDKs are getting new bug fixes for Blob and Queue. WindowsAzure.Storage appears to be frozen.

Vhab commented 5 years ago

Is Microsoft.Azure.Cosmos.Table able to talk to existing Azure Storage Table accounts? The non-cosmos kind.

Edit: Ended up validating this for a related thing. Yes it does.

jason-bragg commented 5 years ago

@seniorquico, thanks for letting us know about these upgrade issues.

I'll look into this as time allows, but my initial impression is that it looks like the direction their going is for Microsoft.Azure.Cosmos.Table to subsume table features from WindowsAzure.Storage, so we should try to move to it as best we can. If there are issues, we may want to hold off until the new packages are more stable.

Please let us know what the cosmos team comes back with. Thanks

seniorquico commented 5 years ago

Finally received word from the Cosmos team:

We retained most of the API surface from Azure Storage to reduce the learning curve / transition cost. However, when it comes to cross-cutting APIs such as OperationContext and StorageException, we find the need to fork the implementation due to diverging feature-sets and requirements. Hence, at this time we don't have plans to move Cosmos.Table SDK to depend on Storage.Common.

Reuse of the common classes appears to be out of the picture.

vmendi commented 5 years ago

@seniorquico What does it mean? Does it mean that it's going to be super difficult to migrate?

seniorquico commented 5 years ago

@vmendi It's not super difficult... just a chore to update everything. The biggest change is a refactor of the shared AzureStorageUtils class. I split it into two, one for blob & queue and one for table. Here's an updated branch on top of the latest Orleans master branch:

https://github.com/seniorquico/orleans/tree/update-azure-storage-sdks

The only issue remaining is (from original post):

Next, the classes NameValidator and StorageErrorCodeStrings were changed to an internal visibility in Microsoft.Azure.Cosmos.Table (they remain public in Microsoft.Azure.Storage.Common).

The Cosmos team indicated they'd help provide recommendations should any issues arise during migration. I just opened https://github.com/Azure/azure-cosmos-table-dotnet/issues/5 to request that guidance. After I get a response and fix this last issue, I can open a PR!

KSemenenko commented 5 years ago

I also have some conflict library for AzureStoreage. Do you plan to update them and use Microsoft.Azure.Cosmos.Table which supports both AzureTables and CosmosDB and thus add support for two types of storage in Orleans? @sergeybykov

seniorquico commented 5 years ago

The branch I referenced in the previous post has been updated to make use of the new Microsoft.Azure.* packages (instead of the obsolete Windows.AzureStorage package). With this change, one should be able to use Azure Table endpoints and Azure Cosmos endpoints using the Table API. The backend implementation is completely transparent to the storage provider.

As previously noted, I am waiting on Azure/azure-cosmos-table-dotnet#5 before wrapping up the changes and making a PR. An additional to-do item not previously noted is to update unit tests. In all, I don't think there's more than a couple hours of work left.

Really just waiting on a new version of the Microsoft.Azure.Cosmos.Table package. We're now going on 2 months without an update. At this point, my opinion is that Microsoft has little interest in supporting Azure Table storage. 😞

sergeybykov commented 5 years ago

Do you plan to update them and use Microsoft.Azure.Cosmos.Table which supports both AzureTables and CosmosDB and thus add support for two types of storage in Orleans?

We don't have explicit plans for this. We would happily take @seniorquico's PR for 3.0.

Vhab commented 5 years ago

At this point, my opinion is that Microsoft has little interest in supporting Azure Table storage. 😞

We share a similar frustration. Especially around the forced deprecation of the old library before the new one was ready, and refusal to accept PRs to fix https://github.com/dotnet/orleans/issues/5003 in the old library.

The fore mentioned issue is still affecting us regularly. Maybe it's an option to go forward with the workaround that is currently in place on your branch? With the intention to remove the workaround should the Cosmos library be updated.

sergeybykov commented 5 years ago

@seniorquico Did I get it right that your change is currently dependent on 1.0.4-preview version of Microsoft.Azure.Cosmos.Table?

seniorquico commented 5 years ago

@sergeybykov Yes. The latest 1.0.4-preview fixes the visibility of some types.

I just pushed an update to the branch in my fork that uses the updated 1.0.4-preview. The unit tests still need to be updated. Additionally, it appears there have been some recent changes in the src\Azure directory merged into the master branch. Looks like I'll also need to resolve some conflicts.

vmendi commented 4 years ago

1.0.5 is already out. How is this going? Can we help with anything?

seniorquico commented 4 years ago

@vmendi Great news! The tasks I listed above are still outstanding. Need to incorporate the latest master branch changes, and need to update the unit tests.

sergeybykov commented 4 years ago

@seniorquico I opened #6013 based on your https://github.com/seniorquico/orleans/tree/update-azure-storage-sdks to complete this effort for 3.0.0. Please take a look.

sergeybykov commented 4 years ago

Resolved via #6013.