[Storage] support StandardBlobtier and Rehydrate priority #9781

blueww · 2019-08-07T08:48:47Z

Description

The design review passed on:

Rehydrate priority: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/323 (Rehydrate priority is still in public preview, but feature team confirm it's already public available in all regions, and feature team would like to release it in stable PSH release.)
StandardBlobtier: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/248

Checklist

I have read the Submitting Changes section of CONTRIBUTING.md
The title of the PR is clear and informative
The appropriate change log has been updated
The PR does not introduce breaking changes
If applicable, the changes made in the PR have proper test coverage
For public API changes to cmdlets:
- the changes have gone through a cmdlet design review (Microsoft internal partners only)
- the cmdlet markdown files were generated using the platyPS module

erich-wang

@blueww , the review is ongoing, I'll change label to needs-revision after review is done.

erich-wang · 2019-08-08T12:26:42Z

src/Storage/Storage/Blob/StorageCloudBlobCmdletBase.cs

        {
            if ((pageBlobTier != null)
                && (type != BlobType.PageBlob))
            {
                throw new ArgumentOutOfRangeException("BlobType, PageBlobTier", String.Format("PremiumPageBlobTier can only be set to Page Blob. The Current BlobType is: {0}", type));
            }
+            if (((standardBlobTier != null || rehydratePriority != null))


nit, please remove duplicate ()

erich-wang · 2019-08-08T12:42:37Z

src/Storage/Storage/Blob/Cmdlet/SetAzureStorageBlobContent.cs

@@ -170,6 +170,22 @@ public PremiumPageBlobTier PremiumPageBlobTier

        private PremiumPageBlobTier? pageBlobTier = null;

+        [Parameter(HelpMessage = "Block Blob Tier, valid values are Hot/Cool/Archive. See detail in https://docs.microsoft.com/en-us/azure/storage/blobs/storage-blob-storage-tiers", Mandatory = false)]
+        [ValidateSet("Hot", "Cool", "Archive", IgnoreCase = true)]
+        public StandardBlobTier StandardBlobTier


Suggest to use PSArgumentCompleter

Same comment

The Type of this parameter should be StandardBlobTier.
As PSArgumentCompleter must work with String type parameter, I have changed it back.

The advice is to make this a string parameter, use a completer, and convert the value to the enum in your cmdlet code.

I have make it a string, and then parse the value to StandardBlobTier.

erich-wang · 2019-08-08T12:45:47Z

src/Storage/Storage/Blob/Cmdlet/StartAzureStorageBlobCopy.cs

        private PremiumPageBlobTier? pageBlobTier = null;

+        [Parameter(HelpMessage = "Block Blob Tier, valid values are Hot/Cool/Archive. See detail in https://docs.microsoft.com/en-us/azure/storage/blobs/storage-blob-storage-tiers", Mandatory = false)]
+        [ValidateSet("Hot", "Cool", "Archive", IgnoreCase = true)]
+        public StandardBlobTier StandardBlobTier


Suggest to use PSArgumentCompleter

If you use the completer, the type would be string, in this case.

The Type of this parameter should be StandardBlobTier.
As PSArgumentCompleter must work with String type parameter, I have changed it back.

Same comment

I have made it a string, and then parse the value to StandardBlobTier.

erich-wang · 2019-08-08T13:09:25Z

Review

General

How about add test cases to cover new parameters?

blueww · 2019-08-09T07:22:03Z

@erich-wang
I have fixed all your comments and test cases added.
Please help to review again and add all comments.

src/Storage/Storage.Management/Storage.Management.csproj

markcowl · 2019-08-09T22:40:57Z

src/Storage/Storage.Test/Storage.Test.csproj

-    <PackageReference Include="Microsoft.Azure.Storage.Blob" Version="10.0.3" />
-    <PackageReference Include="Microsoft.Azure.Storage.File" Version="10.0.3" />
-    <PackageReference Include="Microsoft.Azure.Storage.Queue" Version="10.0.3" />
+    <PackageReference Include="Microsoft.Azure.Storage.Blob" Version="11.0.0" />


Doesn't this require changes to the Storage.DataMovement library?

As we discussed before, since Az.Accounts reference a old DMlib, we will use a private signed DMlib with different name in PSH. And I have already updated the private signed DMlib to new build with this PR. (The last file in "Files Changed")

src/Storage/Storage/Storage.csproj

markcowl · 2019-08-09T22:48:10Z

src/Storage/Storage/Blob/Cmdlet/StartAzureStorageBlobCopy.cs

        private PremiumPageBlobTier? pageBlobTier = null;

+        [Parameter(HelpMessage = "Block Blob Tier, valid values are Hot/Cool/Archive. See detail in https://docs.microsoft.com/en-us/azure/storage/blobs/storage-blob-storage-tiers", Mandatory = false)]
+        [ValidateSet("Hot", "Cool", "Archive", IgnoreCase = true)]
+        public StandardBlobTier StandardBlobTier


If you use the completer, the type would be string, in this case.

markcowl · 2019-08-09T22:48:26Z

src/Storage/Storage/Blob/Cmdlet/SetAzureStorageBlobContent.cs

@@ -170,6 +170,22 @@ public PremiumPageBlobTier PremiumPageBlobTier

        private PremiumPageBlobTier? pageBlobTier = null;

+        [Parameter(HelpMessage = "Block Blob Tier, valid values are Hot/Cool/Archive. See detail in https://docs.microsoft.com/en-us/azure/storage/blobs/storage-blob-storage-tiers", Mandatory = false)]
+        [ValidateSet("Hot", "Cool", "Archive", IgnoreCase = true)]
+        public StandardBlobTier StandardBlobTier


Same comment

src/Storage/Storage.Management/help/Start-AzStorageBlobCopy.md

src/Storage/Storage/Storage.csproj

markcowl · 2019-08-12T22:07:59Z

sign job here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/psnetsign-esrp/484/

blueww · 2019-08-13T05:47:34Z

@markcowl
I have changed the parameter to string as you recommanded.
Would you please help to check if any further comments?
And help to merged it if no other comments.

BTW, a new sign job with latest code change is: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/psnetsign-esrp/485/

erich-wang

@blueww , I added two additional comments, please take a look.

src/Storage/Storage/Blob/Cmdlet/SetAzureStorageBlobContent.cs

src/Storage/Storage/Blob/Cmdlet/StartAzureStorageBlobCopy.cs

blueww · 2019-08-14T03:23:43Z

@erich-wang
I have resolved your 2 new comments.
A new sign job with latest code change is:
https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/psnetsign-esrp/486/

erich-wang · 2019-08-14T09:10:23Z

/azp run azure-powershell - powershell-core

azure-pipelines · 2019-08-14T09:10:33Z

Azure Pipelines successfully started running 1 pipeline(s).

blueww · 2019-08-14T09:25:31Z

/azp run

markcowl · 2019-08-14T23:50:13Z

@blueww will do some sign testing tomorrow, otherwise lgtm.

markcowl · 2019-08-14T23:51:22Z

sign job here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/psnetsign-esrp/487/

blueww · 2019-08-15T03:07:42Z

@markcowl @erich-wang
Thanks both!
Waiting for the PR to be merged...

blueww · 2019-08-15T05:58:13Z

/azp run

azure-pipelines · 2019-08-15T05:58:28Z

Azure Pipelines successfully started running 3 pipeline(s).

blueww · 2019-08-16T01:28:48Z

@markcowl @erich-wang
Let me know if anything else I need to do to make this PR merge.
Hope this can be merged soon.

markcowl · 2019-08-16T17:36:00Z

/azp run azure-powershell - powershell-core

azure-pipelines · 2019-08-16T17:36:11Z

Azure Pipelines successfully started running 1 pipeline(s).

blueww added 2 commits August 7, 2019 10:40

[Storage] support blob tier in upload/copy block blob

19fe279

[Storage] support Rehydrate Priority in copy blob

1256742

azuresdkci added the needs-review label Aug 7, 2019

markcowl assigned erich-wang Aug 8, 2019

markcowl requested a review from erich-wang August 8, 2019 01:42

erich-wang reviewed Aug 8, 2019

View reviewed changes

markcowl requested changes Aug 9, 2019

View reviewed changes

markcowl added needs-revision and removed needs-review labels Aug 9, 2019

markcowl requested changes Aug 9, 2019

View reviewed changes

src/Storage/Storage.Management/help/Start-AzStorageBlobCopy.md Outdated Show resolved Hide resolved

markcowl requested changes Aug 12, 2019

View reviewed changes

src/Storage/Storage/Storage.csproj Show resolved Hide resolved

blueww force-pushed the blobtier branch from ffe7bb6 to c5a1814 Compare August 13, 2019 05:43

erich-wang suggested changes Aug 13, 2019

View reviewed changes

src/Storage/Storage/Blob/Cmdlet/SetAzureStorageBlobContent.cs Show resolved Hide resolved

src/Storage/Storage/Blob/Cmdlet/StartAzureStorageBlobCopy.cs Show resolved Hide resolved

erich-wang assigned blueww and unassigned erich-wang Aug 13, 2019

[Storage] Update perreview feedback

976a1bc

blueww force-pushed the blobtier branch from c5a1814 to 976a1bc Compare August 14, 2019 03:05

blueww added needs-review and removed needs-revision labels Aug 14, 2019

erich-wang approved these changes Aug 14, 2019

View reviewed changes

markcowl added Do Not Merge 🚫 and removed needs-review labels Aug 14, 2019

markcowl removed the Do Not Merge 🚫 label Aug 16, 2019

markcowl unassigned blueww Aug 16, 2019

markcowl approved these changes Aug 16, 2019

View reviewed changes

markcowl added the Waiting for CI label Aug 16, 2019

markcowl merged commit 4674d4b into Azure:master Aug 16, 2019

[Storage] support StandardBlobtier and Rehydrate priority #9781

[Storage] support StandardBlobtier and Rehydrate priority #9781

Uh oh!

Conversation

blueww commented Aug 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

erich-wang left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blueww Aug 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blueww Aug 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

erich-wang commented Aug 8, 2019

Review

General

Uh oh!

blueww commented Aug 9, 2019

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blueww Aug 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

markcowl commented Aug 12, 2019

Uh oh!

blueww commented Aug 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

erich-wang left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

blueww commented Aug 14, 2019

Uh oh!

erich-wang commented Aug 14, 2019

Uh oh!

azure-pipelines bot commented Aug 14, 2019

Uh oh!

blueww commented Aug 7, 2019 •

edited

Loading

blueww Aug 12, 2019 •

edited

Loading

blueww Aug 12, 2019 •

edited

Loading

blueww Aug 12, 2019 •

edited

Loading

blueww commented Aug 13, 2019 •

edited

Loading