azure-sdk-for-cpp
azure-sdk-for-cpp copied to clipboard
Azure
`BlobLeaseClient` is missing a `GetUrl` getter
Is your feature request related to a problem? Please describe.
I need to know the URL of the BlobLeaseClient to produce good error message from my utility functions.
For convenience, a direct link to the header: https://github.com/Azure/azure-sdk-for-cpp/blob/main/sdk/storage/azure-storage-blobs/inc/azure/storage/blobs/blob_lease_client.hpp
Describe the solution you'd like
BlobLeaseClient should have a GetUrl getter.
blob_lease_client.hpp:
std::string BlobLeaseClient::GetUrl() const
{
if (m_blobClient.HasValue())
{
return m_blobClient->GetUrl();
}
else if (m_blobContainerClient.HasValue())
{
return m_blobContainerClient->GetUrl();
}
else
{
AZURE_UNREACHABLE_CODE();
}
}
blob_lease_client.cpp:
/**
* @brief Get's the primary URL endpoint of the blob or container.
*
* @return A URL of a blob or container.
*/
std::string GetUrl() const;
Describe alternatives you've considered
Passing a string as parameter everywhere in my code.
Information Checklist Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report
- [x] Description Added
- [x] Expected solution specified
Lease client is a convenient method for customers to do lease operations, unlike BlobClient, BlobContainerClient or BlobServiceClient, lease client doesn't have a corresponding real resource at server side. I think maybe we shouldn't have GetUrl for it.
@LarryOsterman What do you think?
The C++ guidelines require that service clients support a GetUrl method. The same does not apply to non-service clients.
How is a LeaseClient instantiated? With a connection string/url? Or is it instantiated with a BlobClient? Or from a BlobClient?
If it's constructed with a connection string or URL it makes sense for there to be a GetUrl method. If it's constructed from or with a BlobClient, it doesn't make sense for there to be a GetUrl method.
One thing you might consider is having an ostream inserter for LeaseClient which generates a meaningful string for diagnostics. That would resolve the customer concern without having to add a method which doesn't necessarily make sense. You can think of an ostream inserter as being logically similar to the C# "ToString()" method.
At the moment I can't generate a meaningful string through the the public interface of BlobLeaseClient because there isn't a single public member function that can tell me anything about the resource the BlobLeaseClient targets.
GetUrl might be a bad name for it, but I believe there should be a way to ask for a string that identifies the resource receiving the lease operations that BlobLeaseClient can trigger.
These are the private data members of BlobLeaseClient
private:
Azure::Nullable<BlobClient> m_blobClient;
Azure::Nullable<BlobContainerClient> m_blobContainerClient;
std::mutex m_mutex;
std::string m_leaseId;
It can be instantiated by passing either BlobClient or BlobContainerClient because blobs and containers can be leased. Once you give a client to the BlobLeaseClient you can never recover information about them anymore, you would have to keep a copy of the client and not loose track of the client/lease-client pairing.
@Jinming-Hu We should expose at a minimum the LeaseId. I'm mixed on whether it makes sense to expose the URL of the targeted object. Other language SDKs do expose both of these values. see: https://learn.microsoft.com/en-us/dotnet/api/azure.storage.blobs.specialized.blobleaseclient?view=azure-dotnet
@RickWinter there is GetLeaseId already. It's only the target of the lease that is missing.
3 of the 4 P0 languages (.Net, Java, JavaScript) have a Uri property, Python does not. Go doesn't appear to have a BlobLeaseClient object.
I don't feel strongly about this, to be honest, but my take is that we should follow the pattern of the majority of the P0 SDK clients here.
