Support soft-delete feature in storge dataplane cmdlet #5457

blueww · 2018-02-06T05:55:10Z

Description

To support Soft-delete feature. Design has been reviewed in https://github.com/Azure/azure-powershell-pr/wiki/Azure-Storage-Blob-Soft-Delete

The PR is used to add softdelete feature.

Since I will be OOF from 2/7-2/12 and 2/15-2/21. Please help to review and give comments before 2/12, so I can fix the issues and upgrade to official package in 2/13-14, to make the change in next release which has code complete in 2/16.
And @markcowl has agreed to fix the upgrade issue of "Newtonsoft.Json" 10.0 before 2/12, or the PR won't pass check.

This checklist is used to make sure that common guidelines for a pull request are followed. You can find a more complete discussion of PowerShell cmdlet best practices here.

I have read the contribution guidelines.
If changes were made to any cmdlet, the XML help was regenerated using the platyPSHelp module.
If any large changes are made to a service, they are reflected in the respective change log.

General Guidelines

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.
The pull request does not introduce breaking changes (unless a major version change occurs in the assembly and module).

Testing Guidelines

Pull request includes test coverage for the included changes.
PowerShell scripts used in tests should do any necessary setup as part of the test or suite setup, and should not use hard-coded values for locations or existing resources.

Cmdlet Signature Guidelines

New cmdlets that make changes or have side effects should implement ShouldProcess and have SupportShouldProcess=true specified in the cmdlet attribute. You can find more information on ShouldProcess here.
Cmdlet specifies OutputType attribute if any output is produced - if the cmdlet produces no output, it should implement a PassThru parameter.

Cmdlet Parameter Guidelines

Parameter types should not expose types from the management library - complex parameter types should be defined in the module.
Complex parameter types are discouraged - a parameter type should be simple types as often as possible. If complex types are used, they should be shallow and easily creatable from a constructor or another cmdlet.
Cmdlet parameter sets should be mutually exclusive - each parameter set must have at least one mandatory parameter not in other parameter sets.

blueww · 2018-02-13T03:32:30Z

On-demand: https://azuresdkci.westus2.cloudapp.azure.com/job/powershell-demand/475/

markcowl · 2018-02-14T01:32:33Z

src/Storage/Commands.Storage/Common/Cmdlet/DisableAzureStorageDeleteRetentionPolicy.cs

+    /// </summary>
+    [Cmdlet(VerbsLifecycle.Disable, StorageNouns.ServiceDeleteRetentionPolicy, SupportsShouldProcess = true),
+        OutputType(typeof(PSDeleteRetentionPolicy))]
+    public class DisableAzureStorageServiceDeleteRetentionPolicyCommand : StorageCloudBlobCmdletBase


I think the comm and name should be shorter: Disable-AzureStorageSoftDelete

@markcowl
The name of the cmdlets Enable/Disable-AzureStorageServiceDeleteRetentionPolicy are reviewed and raised from your team, and feature team also agreed for that.
The current name is aligned with the rest and XSCL API, so customer who have looked into rest doc might can find it more easily.

Do you really want to change it?

As discussed with Mark, the alias for the cmdlets is added: Disable-AzureStorageSoftDelete

markcowl · 2018-02-14T01:32:51Z

src/Storage/Commands.Storage/Common/Cmdlet/EnableAzureStorageDeleteRetentionPolicy.cs

+    /// </summary>
+    [Cmdlet(VerbsLifecycle.Enable, StorageNouns.ServiceDeleteRetentionPolicy, SupportsShouldProcess = true),
+        OutputType(typeof(PSDeleteRetentionPolicy))]
+    public class EnableAzureStorageServiceDeleteRetentionPolicyCommand : StorageCloudBlobCmdletBase


Enable-AzureStorageSoftDelete

As discussed with Mark, the alias for the cmdlets is added: Enable-AzureStorageSoftDelete

markcowl · 2018-02-14T01:34:12Z

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

    </Reference>
-    <Reference Include="Newtonsoft.Json, Version=6.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed, processorArchitecture=MSIL">
-      <HintPath>..\..\packages\Newtonsoft.Json.6.0.8\lib\net45\Newtonsoft.Json.dll</HintPath>
+    <Reference Include="Newtonsoft.Json, Version=10.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed, processorArchitecture=MSIL">


This will have to wait until our runtime change is in. ETA is next week, which means this will likely not make it in for this release.

See, waiting for your fix.

markcowl · 2018-02-14T01:35:06Z

src/Storage/Commands.Storage/Commands.Storage.csproj

+    <Reference Include="Microsoft.WindowsAzure.Storage.DataMovement, Version=0.7.1.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
+      <HintPath>..\..\packages\Microsoft.Azure.Storage.DataMovement.0.7.1\lib\net45\Microsoft.WindowsAzure.Storage.DataMovement.dll</HintPath>
+    </Reference>
+    <Reference Include="Newtonsoft.Json, Version=10.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed, processorArchitecture=MSIL">


Same comment

See, waiting for your fix.

cormacpayne · 2018-02-14T01:39:16Z

Sign job: https://azuresdkci.westus2.cloudapp.azure.com/job/ps-sign/135/

blueww · 2018-02-14T02:13:46Z

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

@@ -280,6 +280,7 @@
    </ProjectReference>
  </ItemGroup>
  <ItemGroup>
+    <None Include="app.config" />


Need remove this line, or build will fail.

I will raise a commit to remove it.

Add it back since the app.config already check in.

The fix is merged, please help to review and merge this PR.

blueww · 2018-02-14T09:09:36Z

@markcowl , @cormacpayne
[resolved] The build also fail since test case in Microsoft.WindowsAzure.Management.Storage.Test.Common.Cmdlet.StorageContextDisconnectedTests failed with following error.
It's caused by the 'Newtonsoft.Json‘ version conflict between Commands.Storage.Test and Commands.Common.Authentication. Please help to resolve it. Thanks!

Result Message: System.IO.FileLoadException : Could not load file or assembly 'Newtonsoft.Json, Version=6.0.0.0, Culture=neutral, PublicKeyToken=30ad4fe6b2a6aeed' or one of its dependencies. The located assembly's manifest definition does not match the assembly reference. (Exception from HRESULT: 0x80131040)

cormacpayne · 2018-02-22T18:55:33Z

@blueww Hey Wei, is there a reason why you removed the app.config file? The build was passing before it was removed, and it is necessary for your test project so the tests use the 10.0.3 version of Newtonsoft.Json instead of 6.0.8 (or any other version in between).

blueww · 2018-02-23T03:04:26Z

@cormacpayne
The app.config file will not be check in before, so need to remove it in csproj file to avoid build fail. Just notice you have checked in the app.config file, so it's OK to add it. I have reverted my change of removing it.

blueww · 2018-02-23T09:29:01Z

on-demand: https://azuresdkci.westus2.cloudapp.azure.com/job/powershell-demand/506/

blueww · 2018-03-01T09:05:54Z

on-demand: https://azuresdkci.westus2.cloudapp.azure.com/job/powershell-demand/526/

blueww · 2018-03-01T09:09:50Z

@markcowl , @cormacpayne
As the #5599 is merged for 'Newtonsoft.Json‘ upgrade issue , I have rebased the softdelete PR on latest code.
I have also relaunch the on-demand test.

If no other comments, please help to merge the PR when the test passed.

markcowl · 2018-03-02T03:26:53Z

src/ResourceManager/Profile/Commands.Profile/StartupScripts/AzureRmProfileStartup.ps1

@@ -27,4 +27,4 @@
 if (Test-Path "$PSScriptRoot\..\NewAssemblies")
 {


Please revert any change to this file

markcowl · 2018-03-02T03:27:00Z

src/ResourceManager/Storage/Commands.Management.Storage/packages.config

@@ -3,4 +3,4 @@
  <package id="Microsoft.Azure.KeyVault.Core" version="1.0.0" targetFramework="net45" />
  <package id="Microsoft.Azure.Management.Storage" version="7.1.0-preview" targetFramework="net452" />
  <package id="WindowsAzure.Storage" version="8.1.1" targetFramework="net452" />
-</packages>


markcowl · 2018-03-02T03:27:41Z

src/ResourceManager/Storage/Commands.Management.Storage/packages.config

@@ -3,4 +3,4 @@
  <package id="Microsoft.Azure.KeyVault.Core" version="1.0.0" targetFramework="net45" />
  <package id="Microsoft.Azure.Management.Storage" version="7.1.0-preview" targetFramework="net452" />
  <package id="WindowsAzure.Storage" version="8.1.1" targetFramework="net452" />


You will need to update the version here as well

markcowl · 2018-03-02T03:30:27Z

src/Storage/Commands.Storage/help/Get-AzureStorageBlob.md

@@ -14,14 +15,14 @@ Lists blobs in a container.

 ### BlobName (Default)
 ```
-Get-AzureStorageBlob [[-Blob] <String>] [-Container] <String> [-MaxCount <Int32>]
+Get-AzureStorageBlob [[-Blob] <String>] [-Container] <String> [-IncludeDeleted] [-MaxCount <Int32>]


Please include output in the examples (the updated output)

The output added to example. Please check.

blueww · 2018-03-02T07:06:22Z

@markcowl

I have resolved all your comments. Since there are some revert fix, I merged the commits into 1 commit.
The on-demand on latest code can be found in : https://azuresdkci.westus2.cloudapp.azure.com/job/powershell-demand/537

cormacpayne · 2018-03-02T16:34:30Z

Sign-job: https://azuresdkci.westus2.cloudapp.azure.com/job/ps-sign/225/

cormacpayne · 2018-03-05T16:49:50Z

Tested the signed bits and confirmed that

both Newtonsoft.Json 6.0.8 and 10.0.3 are loaded
AzureRM.Storage and Azure.Storage cmdlets can be used in the same sessions

azuresdkci added the needs-review label Feb 6, 2018

azuresdkci assigned markcowl Feb 6, 2018

blueww force-pushed the softdelete branch 2 times, most recently from a3df663 to 0658761 Compare February 6, 2018 11:50

maddieclayton added the Do Not Merge 🚫 label Feb 7, 2018

blueww force-pushed the softdelete branch from 0658761 to aa22245 Compare February 13, 2018 03:30

blueww changed the title ~~[Do not merge] Support soft-delete feature in storge dataplane cmdlet~~ Support soft-delete feature in storge dataplane cmdlet Feb 13, 2018

blueww removed the Do Not Merge 🚫 label Feb 13, 2018

blueww force-pushed the softdelete branch from aa22245 to 66db2a3 Compare February 13, 2018 11:36

markcowl requested changes Feb 14, 2018

View reviewed changes

blueww commented Feb 14, 2018

View reviewed changes

maddieclayton added the Do Not Merge 🚫 label Feb 15, 2018

blueww mentioned this pull request Feb 22, 2018

Load Newtonsoft.Json 10.0.3 side-by-side with 6.0.8 #5599

Merged

8 tasks

cormacpayne assigned blueww and unassigned markcowl Feb 22, 2018

cormacpayne added needs-revision and removed needs-review labels Feb 22, 2018

blueww force-pushed the softdelete branch from 9b8d443 to 607541a Compare February 23, 2018 03:00

blueww assigned cormacpayne and unassigned blueww Feb 23, 2018

maddieclayton added needs-review and removed Do Not Merge 🚫 needs-revision labels Feb 26, 2018

maddieclayton unassigned cormacpayne Feb 26, 2018

cormacpayne added the needs-review label Feb 27, 2018

blueww force-pushed the softdelete branch from 607541a to 6435bda Compare March 1, 2018 09:04

blueww removed the Do Not Merge 🚫 label Mar 1, 2018

markcowl requested changes Mar 2, 2018

View reviewed changes

markcowl added needs-revision and removed needs-review labels Mar 2, 2018

markcowl assigned blueww and unassigned cormacpayne Mar 2, 2018

blueww force-pushed the softdelete branch from f464510 to df98eb8 Compare March 2, 2018 03:53

blueww assigned markcowl and cormacpayne and unassigned blueww Mar 2, 2018

blueww force-pushed the softdelete branch 3 times, most recently from 5688c9b to ebdf43c Compare March 2, 2018 06:56

blueww force-pushed the softdelete branch 2 times, most recently from 8348515 to fe8b7c9 Compare March 2, 2018 08:49

Upgrade to XSCL 9.0 and support soft-delete

72e52e9

blueww force-pushed the softdelete branch from fe8b7c9 to 72e52e9 Compare March 2, 2018 09:07

cormacpayne added needs-review and removed needs-revision labels Mar 2, 2018

maddieclayton added the Waiting for On Demand label Mar 3, 2018

markcowl approved these changes Mar 5, 2018

View reviewed changes

cormacpayne approved these changes Mar 5, 2018

View reviewed changes

cormacpayne merged commit a186cdc into Azure:preview Mar 5, 2018

		@@ -27,4 +27,4 @@
		if (Test-Path "$PSScriptRoot\..\NewAssemblies")
		{

Support soft-delete feature in storge dataplane cmdlet #5457

Support soft-delete feature in storge dataplane cmdlet #5457

Uh oh!

Conversation

blueww commented Feb 6, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

blueww commented Feb 13, 2018

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!

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!

cormacpayne commented Feb 14, 2018

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 commented Feb 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cormacpayne commented Feb 22, 2018

Uh oh!

blueww commented Feb 23, 2018

Uh oh!

blueww commented Feb 23, 2018

Uh oh!

blueww commented Mar 1, 2018

Uh oh!

blueww commented Mar 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blueww commented Mar 2, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cormacpayne commented Mar 2, 2018

Uh oh!

cormacpayne commented Mar 5, 2018

Uh oh!

Uh oh!

blueww commented Feb 6, 2018 •

edited

Loading

blueww commented Feb 14, 2018 •

edited

Loading

blueww commented Mar 1, 2018 •

edited

Loading

blueww commented Mar 2, 2018 •

edited

Loading