-
Notifications
You must be signed in to change notification settings - Fork 320
[Storage] delete/set_immutability_policy for BlobClient
#3342
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?
[Storage] delete/set_immutability_policy for BlobClient
#3342
Conversation
API Change CheckAPIView identified API level changes in this PR and created the following API reviews |
get/set_immutability_policy for BlobClientdelete/set_immutability_policy for BlobClient
…to fix up legal hold to point to versioned as well + get bicep updated to work
| } | ||
|
|
||
| #[recorded::test] | ||
| async fn test_immutability_policy(ctx: TestContext) -> Result<(), Box<dyn Error>> { |
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.
Testing against a regular Storage account yields:
HTTP/1.1 400 ImmutableStorageWithVersioning: feature is not enabled.
Testing against versioned without immutable storage on:
ImmutableStorageWithVersioningIsNotEnabledImmutableStorageWithVersioning: feature is not enabled.
So, as a result, had to use a versioned account + immutable storage with versioning enabled.
As a result, having the same issues with deleting the container as set_legal_hold:
ContainerImmutableStorageWithVersioningEnabledThe requested operation is not allowed as the container has a immutable storage with versioning enabled.
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.
Mark as playback only for now, I guess. Open an issue to track getting proper clean up on this and the legal hold tests so we can enable them to run Live.
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.
#3358
Done, will mark playback only once we get set_legal_hold merged since I unfortunately did not base it off of that PR.
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.
Why can't you update your test-resources.bicep file? That's the whole reason it's there and you should be using it, just like in all other languages (I wrote those original scripts for every Azure SDK language). I'd rather we don't have a confusing concept of "playback-only" tests. That implies you have to record them at some point which means modifying source which means people have to manually interject at least a couple times. Just use a #[tokio::test] with a MockHttpClient from our azure_core_test crate instead. I do that in lots of places. Pretty easy to use.
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.
@heaths, we need to mark these tests as playback only because containers with immutability policies (and legal holds) are very difficult to clean up. They require the management plane to do so which we don't have yet in Rust. We will try and figure out a way to delete them and change them to regular recorded tests later. There is nothing that can be changed in the bicep to accomplish this.
We have the concept of playback only in every other language and we use it sparingly for situations like this. It would be nice to have that concept available in Rust. Mocking would require extra effort and means these tests will work and look different than the rest of our tests.
| } | ||
|
|
||
| #[recorded::test] | ||
| async fn test_immutability_policy(ctx: TestContext) -> Result<(), Box<dyn Error>> { |
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.
Mark as playback only for now, I guess. Open an issue to track getting proper clean up on this and the legal hold tests so we can enable them to run Live.
.tsp: Azure/azure-rest-api-specs#38750set/delete_immutability_policyto BlobClient