Using Azure.Storage.Blobs for storage access #10018
Conversation
Shorter HttpStatusCode checks.
ETag tracking.
| var ex = await Assert.ThrowsAsync<StorageException>(() => file.DownloadToStreamAsync(destination)); | ||
| Assert.Contains("timeout", ex.Message); | ||
| } | ||
|
|
||
| [BlobStorageFact] | ||
| public async Task OpenWriteAsyncReturnsWritableStream() |
There was a problem hiding this comment.
I don't know where these integration tests are run. If they are not run anywhere, it might be worth setting up to run them locally (I think there is some connection string setup) or deleting them. Perhaps they'll catch a bug if they are run.
There was a problem hiding this comment.
Removed dependency on old SDK in those tests, ran/fixed some, removed one that no longer passes due to the way new SDK works, verified that we don't really rely on the behavior the test checks.
| _blobClient = new Lazy<BlobServiceClient>(CreateBlobServiceClient); | ||
| } | ||
|
|
||
| public static CloudBlobClientWrapper UsingMsi(string storageConnectionString, string clientId = null) |
There was a problem hiding this comment.
is storageConnectString is necessary if we use MSI? storageConnectionString with "SharedAccessSignature=" will work?
There was a problem hiding this comment.
Technically, we only need the host name. But I'm not sure yet how drastic I want to go with configuration changes that would require. For MSI, I'd guess, a trivial connection string like BlobEndpoint=https://storageaccount.blob.core.windows.net might be enough, I haven't tested it though.
There was a problem hiding this comment.
Looks like not working, I could test it.
There was a problem hiding this comment.
probably can't. it threw exception: No valid combination of account information found.
There was a problem hiding this comment.
I think at least we could remove the SAS in connection string when using MSI
Progress on https://github.com/NuGet/Engineering/issues/5335
Built on top of #9981 (draft PR), so all the SDK isolation work done there is present here.
Some choices made:
MaximumExecutionTimesupport that existed inWindowsAzure.Storageis dropped. Consumers would have to produce cancellation tokens with time limits if needed;There are table wrappers using old SDK left that I haven't touched yet. Those are going to be addressed separately.
I commented out some tests and need some input on what to do with them.
The code was deployed to dev and passed E2E and functional tests.
There likely going to be more changes as this update is propagated to other repositories.