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

[Storage] stage_block, commit_block_list for BlobClient, get_container_properties for ContainerClient, ServiceClient #2273

Merged
merged 15 commits into from
Mar 12, 2025

Conversation

vincenttran-msft
Copy link
Member

@vincenttran-msft vincenttran-msft commented Mar 4, 2025

This PR introduces the following:

BlobClient

  • stage_block
  • commit_block_list

ContainerClient

  • get_container_properties

ServiceClient

  • Initial introduction of ServiceClient
  • get_service_properties

This PR also moves to the standardized way to fetch randomized seeded names (random_string) and includes a helper utility for test setup.

@github-actions github-actions bot added the Storage Storage Service (Queues, Blobs, Files) label Mar 4, 2025
@vincenttran-msft vincenttran-msft changed the title [Storage] stage_block, commit_block_list for BlobClient, get_container_properties for ContainerClient, get_service_properties for ServiceClient [Storage] stage_block, commit_block_list for BlobClient, get_container_properties for ContainerClient, ServiceClient Mar 10, 2025
@vincenttran-msft vincenttran-msft requested a review from heaths March 11, 2025 04:30
@heaths
Copy link
Member

heaths commented Mar 11, 2025

You have some clippy lints to resolve as well. Look at the failed build. It's a good idea to run this locally e.g., cargo clippy -p azure_storage_blob so you can quickly iterate on lints.

@vincenttran-msft
Copy link
Member Author

You have some clippy lints to resolve as well. Look at the failed build. It's a good idea to run this locally e.g., cargo clippy -p azure_storage_blob so you can quickly iterate on lints.

Interesting, this doesn't manifest locally:

C:\azure-sdk-for-rust\sdk\storage>cargo clippy -p azure_storage_blob
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.75s

I also largely took that credential type hint that is problematic from Cosmos ex, so I am surprised Cosmos does not have the same error regarding redundant explicit links.

Regardless, I removed the duplicate explicit link and re-ran clippy locally (although I can't really trust the local output, as it didn't flag it even before the fix).

@heaths
Copy link
Member

heaths commented Mar 11, 2025

You have some clippy lints to resolve as well. Look at the failed build. It's a good idea to run this locally e.g., cargo clippy -p azure_storage_blob so you can quickly iterate on lints.

Interesting, this doesn't manifest locally:

C:\azure-sdk-for-rust\sdk\storage>cargo clippy -p azure_storage_blob
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.75s

I also largely took that credential type hint that is problematic from Cosmos ex, so I am surprised Cosmos does not have the same error regarding redundant explicit links.

Regardless, I removed the duplicate explicit link and re-ran clippy locally (although I can't really trust the local output, as it didn't flag it even before the fix).

If you look at the command in Azure Pipelines, it will use an explicit toolchain e.g., +stable or maybe +nightly. You need to use the same one. If you're using a different version (>= MSRV, as is required), the lints may not catch it (they get more and more pedantic with each subsequent toolchain version).

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 don't see the lib.rs change. We don't want to be exposing all those submodules to clients.

@heaths
Copy link
Member

heaths commented Mar 12, 2025

I was trying to check out your branch when I realized you're not working in a fork. Please use forks on GitHub: fork your repo, push your branch to your fork, and open a PR normally. This not only yields in faster pull times, but also makes other workflows easier and is the GitHub way. Using a single shared repo is how Azure Repos operates but isn't the norm.

@vincenttran-msft vincenttran-msft merged commit 7564641 into main Mar 12, 2025
17 checks passed
@vincenttran-msft vincenttran-msft deleted the vincenttran/remaining_mvp_functionality branch March 12, 2025 18:29
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.

4 participants