[Storage] set_legal_hold for BlobClient
#3335
Conversation
…ount type, update azure_core to allow for playback-only mcaro
github-actions
bot
added
Azure.Core
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
vincenttran-msft
requested review from
LarryOsterman,
RickWinter and
heaths
as code owners
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the set_legal_hold API to BlobClient and introduces a "playback only" option for test macros to support tests that can only run in playback mode.
Key Changes:
- Adds
set_legal_hold()method toBlobClientfor setting or removing legal holds on blobs - Implements
#[recorded::test(playback)]attribute for tests that should only run during playback - Updates TypeSpec commit reference to include the latest blob storage API specifications
Reviewed Changes
Copilot reviewed 7 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/storage/azure_storage_blob/tsp-location.yaml | Updates TypeSpec commit reference to newer version |
| sdk/storage/azure_storage_blob/tests/blob_client.rs | Adds comprehensive test for set_legal_hold API marked as playback-only |
| sdk/storage/azure_storage_blob/src/models/mod.rs | Updates public exports to include BlobClientSetLegalHoldOptions |
| sdk/storage/azure_storage_blob/src/generated/models/pub_models.rs | Removes unused result types and changes AccessPolicy date fields from OffsetDateTime to String |
| sdk/storage/azure_storage_blob/src/generated/models/models_impl.rs | Adds TryFrom implementation for SignedIdentifiers to support XML serialization |
| sdk/storage/azure_storage_blob/src/generated/models/header_traits.rs | Removes header traits for deleted result types and refactors SignedIdentifier handling |
| sdk/storage/azure_storage_blob/src/generated/clients/blob_container_client.rs | Updates get_access_policy and set_access_policy to use SignedIdentifiers wrapper type |
| sdk/storage/azure_storage_blob/src/generated/clients/blob_client.rs | Updates set_legal_hold to return Response<(), NoFormat> instead of custom result type |
| sdk/storage/azure_storage_blob/src/clients/blob_client.rs | Adds public set_legal_hold method to non-generated BlobClient wrapper |
| sdk/storage/azure_storage_blob/assets.json | Updates test asset tag reference |
| sdk/storage/azure_storage_blob/CHANGELOG.md | Documents addition of set_legal_hold API |
| sdk/core/azure_core_test_macros/src/test.rs | Implements playback-only test functionality with validation and tests |
| // Ignore playback-only tests if not running playback tests. | ||
| if recorded_attrs.playback && test_mode != TestMode::Playback { | ||
| test_attr.extend(quote! { | ||
| #[ignore = "skipping playback-only tests"] | ||
| }); | ||
| } |
There was a problem hiding this comment.
What's the use case for this? How do you record this in the first place then? If you don't need a recording, don't use #[recorded::test]. Use a normal #[tokio::test].
There was a problem hiding this comment.
I ran it first with #[recorded::test] and AZURE_TEST_MODE=record, to generate the initial recording, then I marked it as #[recorded::test(playback)] so that this test will not execute in the live test pipeline and only in playback mode.
There was a problem hiding this comment.
I understand what it does. I'm asking: why? You can use mock tests. This feels like a lot of work to re-record tests and I don't necessarily want to support a "record-only" concept for other services.
3 participants
.tsp: [Storage]set_legal_holdfor BlobClient TypeSpec Changes azure-rest-api-specs#38817This test does the following:
set_legal_holdAPI toBlobClientSetLegalHold: