Open DaleyKD opened 1 year ago
Hi @DaleyKD thanks for your interest in the library.
The implementation tries to clean up after itself, so currently we don’t support creating containers on the fly.
I also wanted to minimize the number of API calls on the main lock acquisition path, although in this case I suppose we could wait to catch the exception before creating the container.
What use case do you have that would make that desirable? As someone who is not an Azure expert, do you know if there is anything about container creation that would make it less desirable/harder to automate than automatic blob creation?
First, I totally misread the code path and see where you're automatically creating the blob, while I thought it was the container. My apologies.
The main reason is that I don't want to ask my customers to go in and manually create a container. I'd hope asking them to create the storage account would suffice. (If I DO have to ask them, it's not the end of the world.)
Creating a container isn't that bad. Let me see if I can find a code snippet from my other project.
Well, this is straight from MSFT and how they had us do election mutexes: https://github.com/mspnp/cloud-design-patterns/blob/master/leader-election/DistributedMutex/BlobLeaseManager.cs
At the very bottom, you see that they use a BlobContainerClient
to create the container.
I would kindly "argue" that the code should create the container if it doesn't exist, but not clean it up. Reason? In my code, we have MANY services doing leader election. We use the same container, but different blobs.
The main reason is that I don't want to ask my customers to go in and manually create a container. I'd hope asking them to create the storage account would suffice. (If I DO have to ask them, it's not the end of the world.)
Makes sense. Also fits with one of the goals of the library which is to "just work" without extra setup on the back-end.
Well, this is straight from MSFT and how they had us do election mutexes
Nice!
I would kindly "argue" that the code should create the container if it doesn't exist, but not clean it up. Reason? In my code, we have MANY services doing leader election. We use the same container, but different blobs.
I agree. Upon consideration I think this would be good behavior to add. In fact, it is very consistent with our FileDistributedLock
which lazily creates (but does not clean up) the lock file's directory.
Is this something you'd be interested in contributing?
Implementation notes:
CreateContainerIfNotExistsAsync
to BlobClientWrapper
. Should use this._blobClient.GetParentBlobContainerClient()
and should create the container with the same debugging metadata tag that AzureBlobLeaseDistributedLock
uses when creating blobs.AzureBlobLeaseDistributedLock.TryAcquireAsync
, catch the case where the container does not exist and in that condition create the container and then the blob before retrying.TestWrapperCreateContainerIfNotExists
to AzureBlobLeaseDistributedLockTest
similar to TestWrapperCreateIfNotExists
(but simpler since there aren't multiple types).TestThrowsIfContainerDoesNotExist
with one called TestCreatesContainerIfNeeded
that asserts the successful lock acquisition and that the container is not cleaned up when the lock is released. The test itself should clean up the container after the fact.Also, at risk of stating the obvious, in the short-term you should be able to work around this by simply creating the container yourself during process spinup or right before lock acquisition.
Here's what I have as a workaround:
var blobServiceClient = new BlobServiceClient(azureStorageConnectionString);
if (!blobServiceClient.GetBlobContainers().Any(x => x.Name.Equals(containerName)))
{
blobServiceClient.CreateBlobContainer(containerName);
}
services.AddAzureClients(builder =>
{
builder.AddClient<BlobContainerClient, BlobClientOptions>(options => new BlobContainerClient(azureStorageConnectionString, containerName));
});
Looking at the code, I thought it would attempt to create a new container if it came back as not found. However, an exception is thrown in my code.
Here's the error immediately kicked out: