-
Notifications
You must be signed in to change notification settings - Fork 288
[Storage] get_account_info
for all clients, set/get_tags
for BlobClient
#2661
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for retrieving account info (sku_name
, account_kind
) on all client types and implements set_tags
/get_tags
on BlobClient
, alongside updating tests and regenerating against the latest spec.
- Exposed new tag and account-info models in
src/lib.rs
and added client methods (get_account_info
,set_tags
,get_tags
) - Added corresponding integration tests for service, container, and blob clients
- Updated TSP location, assets tag, and test helper utility
Reviewed Changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
sdk/storage/azure_storage_blob_test/src/lib.rs | Added test_blob_tag_equality helper and import |
sdk/storage/azure_storage_blob/tsp-location.yaml | Bumped spec commit for regen |
sdk/storage/azure_storage_blob/tests/blob_service_client.rs | Added test_get_account_info for service client |
sdk/storage/azure_storage_blob/tests/blob_container_client.rs | Added test_get_account_info for container client |
sdk/storage/azure_storage_blob/tests/blob_client.rs | Added test_blob_tags and test_get_account_info |
sdk/storage/azure_storage_blob/src/lib.rs | Re-exported new tag and account-info types |
sdk/storage/azure_storage_blob/src/clients/blob_service_client.rs | Added get_account_info method |
sdk/storage/azure_storage_blob/src/clients/blob_container_client.rs | Added get_account_info method |
sdk/storage/azure_storage_blob/src/clients/blob_client.rs | Added set_tags , get_tags , and get_account_info |
sdk/storage/assets.json | Updated asset tag hash |
Comments suppressed due to low confidence (1)
sdk/storage/azure_storage_blob/tests/blob_client.rs:303
- The test references
RequestContent
but it is not imported. Adduse azure_core::RequestContent;
at the top of this file to resolve the undefined name.
blob_client
.set_tags(RequestContent::try_from(blob_tags.clone())?, None)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see nothing blocking.
create_test_blob(&blob_client).await?; | ||
|
||
// Set Tags with Tags Specified | ||
let blob_tag_1 = BlobTag { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this match what other languages do? It would be more idiomatic to have this as a HashMap<String, String>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In Python we get something like this:
blob_tags = {"tag1": "firsttag", "tag2": "secondtag", "tag3": "thirdtag"}
While .NET yields something like this:
IDictionary<string, string> tags = BuildTags();
BlobUploadOptions options = new BlobUploadOptions
{
Tags = tags
};
// Unraveling BuildTags()
public Dictionary<string, string> BuildTags()
=> new Dictionary<string, string>
{
{ "tagKey0", "tagValue0" },
{ "tagKey1", "tagValue1" }
};
So I could definitely see the case that we make this more user-friendly such that we take in a HashMap
and build the BlobTags
object for them. cc: @jalauzon-msft Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think both get and set tags should just work with HashMaps. I don't see a need for this extra type.
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
This PR introduces the following changes:
main
branch in feature/blob-tspget_account_info
forBlobClient
,ContainerClient
, andBlobServiceClient
set_tags
andget_tags
forBlobClient
Accompanying TypeSpec changes (merge before merging this):
Azure/azure-rest-api-specs#35044