Skip to content

[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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

vincenttran-msft
Copy link
Member

@vincenttran-msft vincenttran-msft commented Jun 3, 2025

This PR introduces the following changes:

  • Regenerates against main branch in feature/blob-tsp
  • Initial implementation of get_account_info for BlobClient, ContainerClient, and BlobServiceClient
  • Initial implementation of set_tags and get_tags for BlobClient

Accompanying TypeSpec changes (merge before merging this):
Azure/azure-rest-api-specs#35044

@github-actions github-actions bot added the Storage Storage Service (Queues, Blobs, Files) label Jun 3, 2025
@vincenttran-msft vincenttran-msft marked this pull request as ready for review June 3, 2025 01:11
@Copilot Copilot AI review requested due to automatic review settings June 3, 2025 01:11
Copy link
Contributor

@Copilot Copilot AI left a 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. Add use 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)

Copy link
Member

@heaths heaths left a 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 {
Copy link
Member

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>.

Copy link
Member Author

@vincenttran-msft vincenttran-msft Jun 4, 2025

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?

Copy link
Member

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.

Copy link

github-actions bot commented Jun 4, 2025

API Change Check

APIView identified API level changes in this PR and created the following API reviews

azure_storage_blob

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Storage Storage Service (Queues, Blobs, Files)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants