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

fixed concurrency and added unit tests #1376

Merged
merged 2 commits into from
Feb 12, 2019
Merged

Conversation

johnataylor
Copy link
Member

fixes #1292

  • removed caching of container handle
  • removed concurrency from Delete, Read and Write calls
  • simplified code it use straight forward loops instead of Linq and Lambdas
  • fixed conditional creation of AccessCondition (the original code was non sense but probably harmless)
  • removed calls to IfExists for container on Delete and Read (which are coded to be conditional anyhow)
  • added unit tests

This should most likely fix the non-nonsensical ETag exception because this code was incorrect. At least this code is now correct and that should presumably make the exception go away, at least it's a start. Anyhow I could not reproduce this exact problem either way.

I also made a good start on adding unit tests, I'm sure we could always add more.

{
var blobName = GetBlobName(key);
var blobReference = blobContainer.GetBlobReference(blobName);
await blobReference.DeleteIfExistsAsync(cancellationToken).ConfigureAwait(false);
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to use of Cancellation Token here.

Should you be checking between iterations to see if it was cancelled, then bail out? Or does that happen automatically through Exception handling?

@cleemullins
Copy link
Contributor

Build error:

2019-02-12T19:37:41.9675561Z Failed   TestBlobStorageWriteRead
2019-02-12T19:37:41.9702014Z Error Message:
2019-02-12T19:37:41.9702191Z  Test method Microsoft.Bot.Builder.Azure.Tests.AzureBlobStorageTests.TestBlobStorageWriteRead threw exception: 
2019-02-12T19:37:41.9704722Z Microsoft.WindowsAzure.Storage.StorageException: No connection could be made because the target machine actively refused it ---> System.Net.Http.HttpRequestException: No connection could be made because the target machine actively refused it ---> System.Net.Sockets.SocketException: No connection could be made because the target machine actively refused it
2019-02-12T19:37:41.9704849Z Stack Trace:
2019-02-12T19:37:41.9704899Z     at System.Net.Http.ConnectHelper.ConnectAsync(String host, Int32 port, CancellationToken cancellationToken)
2019-02-12T19:37:41.9704985Z --- End of inner exception stack trace ---
2019-02-12T19:37:41.9705033Z     at System.Net.Http.ConnectHelper.ConnectAsync(String host, Int32 port, CancellationToken cancellationToken)
2019-02-12T19:37:41.9705077Z    at System.Threading.Tasks.ValueTask`1.get_Result()
2019-02-12T19:37:41.9705146Z    at System.Net.Http.HttpConnectionPool.CreateConnectionAsync(HttpRequestMessage request, CancellationToken cancellationToken)
2019-02-12T19:37:41.9705540Z    at System.Threading.Tasks.ValueTask`1.get_Result()
2019-02-12T19:37:41.9705601Z    at System.Net.Http.HttpConnectionPool.WaitForCreatedConnectionAsync(ValueTask`1 creationTask)
2019-02-12T19:37:41.9705644Z    at System.Threading.Tasks.ValueTask`1.get_Result()
2019-02-12T19:37:41.9705693Z    at System.Net.Http.HttpConnectionPool.SendWithRetryAsync(HttpRequestMessage request, Boolean doRequestAuth, CancellationToken cancellationToken)
2019-02-12T19:37:41.9705761Z    at System.Net.Http.RedirectHandler.SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
2019-02-12T19:37:41.9705910Z    at System.Net.Http.HttpClient.FinishSendAsyncUnbuffered(Task`1 sendTask, HttpRequestMessage request, CancellationTokenSource cts, Boolean disposeCts)
2019-02-12T19:37:41.9705982Z    at Microsoft.WindowsAzure.Storage.Core.Executor.Executor.ExecuteAsync[T](RESTCommand`1 cmd, IRetryPolicy policy, OperationContext operationContext, CancellationToken token) in C:\Program Files (x86)\Jenkins\workspace\dotnet-split-pr-master\Lib\ClassLibraryCommon\Core\Executor\Executor.cs:line 93
2019-02-12T19:37:41.9706059Z --- End of inner exception stack trace ---
2019-02-12T19:37:41.9706124Z     at Microsoft.WindowsAzure.Storage.Core.Executor.Executor.ExecuteAsync[T](RESTCommand`1 cmd, IRetryPolicy policy, OperationContext operationContext, CancellationToken token) in C:\Program Files (x86)\Jenkins\workspace\dotnet-split-pr-master\Lib\ClassLibraryCommon\Core\Executor\Executor.cs:line 256
2019-02-12T19:37:41.9706230Z    at Microsoft.WindowsAzure.Storage.Blob.CloudBlobContainer.CreateAsync(BlobContainerPublicAccessType accessType, BlobRequestOptions options, OperationContext operationContext, CancellationToken cancellationToken) in C:\Program Files (x86)\Jenkins\workspace\dotnet-split-pr-master\Lib\ClassLibraryCommon\Blob\CloudBlobContainer.cs:line 179
2019-02-12T19:37:41.9706343Z    at Microsoft.WindowsAzure.Storage.Blob.CloudBlobContainer.CreateIfNotExistsAsync(BlobContainerPublicAccessType accessType, BlobRequestOptions options, OperationContext operationContext, CancellationToken cancellationToken) in C:\Program Files (x86)\Jenkins\workspace\dotnet-split-pr-master\Lib\ClassLibraryCommon\Blob\CloudBlobContainer.cs:line 383
2019-02-12T19:37:41.9706422Z    at Microsoft.Bot.Builder.Azure.AzureBlobStorage.WriteAsync(IDictionary`2 changes, CancellationToken cancellationToken) in D:\a\1\s\libraries\Microsoft.Bot.Builder.Azure\AzureBlobStorage.cs:line 160
2019-02-12T19:37:41.9706500Z    at Microsoft.Bot.Builder.Azure.Tests.AzureBlobStorageTests.TestBlobStorageWriteRead() in D:\a\1\s\tests\Microsoft.Bot.Builder.Azure.Tests\AzureBlobStorageTests.cs:line 41

@cleemullins cleemullins merged commit c9462f9 into master Feb 12, 2019
@cleemullins cleemullins deleted the johtaylo/issue1292 branch February 12, 2019 21:53
@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 48999

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 10 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+1.7%) to 74.772%

Files with Coverage Reduction New Missed Lines %
/libraries/Microsoft.Bot.Builder.Azure/AzureBlobStorage.cs 10 73.08%
Totals Coverage Status
Change from base Build 48899: 1.7%
Covered Lines: 4022
Relevant Lines: 5379

💛 - Coveralls

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.

Blob Storage State Http Conditional Header Not Met Exception
3 participants