[RecoveryServices.Backup] SDK Update #8529

siddharth7 · 2019-02-13T10:02:17Z

Description

https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/203

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

maddieclayton · 2019-02-13T17:51:43Z

@siddharth7 Please either link your existing design review or fill out a new one here: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues

siddharth7 · 2019-02-13T19:27:26Z

@maddieclayton Updated in description

…dev_test" This reverts commit e766f60, reversing changes made to 8c3b2d1.

vladimir-shcherbakov · 2019-02-19T18:47:38Z

.../RecoveryServices.Backup/Cmdlets/Container/RegisterAzureRmRecoveryServicesBackupContainer.cs

+                var containerModels = ConversionHelpers.GetContainerModelList(listResponse);
+                WriteObject(containerModels, enumerateCollection: true);
+            }, ShouldProcess(ResourceId, VerbsLifecycle.Register));
+        }


Notice that to automatically skip prompts you may want to enable the Force parameter.

vladimir-shcherbakov · 2019-02-19T20:38:04Z

...es/RecoveryServices.Backup/Cmdlets/Item/EnableAzureRmRecoveryServicesBackupAutoProtection.cs

+        [Alias("Id")]
+        [ValidateNotNullOrEmpty]
+        public string InputItem { get; set; }
+


I see a couple of problems here:

PowerShell only matches parameter name, not alias for the ValueFromPipelineByPropertyName.

It's a little confusing to use Id as alias.

Please consider using the InputObject parameter instead.

The object that GetProtectableItem returns has the InputItem property as Id. To enable piping, an alias was added. Is there any other way to enable that?

vladimir-shcherbakov · 2019-02-19T21:38:05Z

...eryServices.Backup/Cmdlets/Restore/GetAzureRmRecoveryServicesBackupWorkloadRecoveryConfig.cs

+    /// </summary>
+    [Cmdlet("Get", ResourceManager.Common.AzureRMConstants.AzureRMPrefix + "RecoveryServicesBackupWorkloadRecoveryConfig",
+        DefaultParameterSetName = RpParameterSet, SupportsShouldProcess = true), OutputType(typeof(RecoveryConfigBase))]
+    public class GetAzureRmRecoveryServicesBackupWorkloadRecoveryConfig : RSBackupVaultCmdletBase


Remind me please why you need SupportsShouldProcess = true in the cmdlet. Does it change the system?

vladimir-shcherbakov · 2019-02-19T21:55:24Z

tools/StaticAnalysis/Exceptions/Az.RecoveryServices/BreakingChangeIssues.csv

+"AssemblyFileName","ClassName","Target","Severity","ProblemId","Description","Remediation"
+"Microsoft.Azure.PowerShell.Cmdlets.RecoveryServices.Backup.dll","Microsoft.Azure.Commands.RecoveryServices.Backup.Cmdlets.RestoreAzureRmRecoveryServicesBackupItem","Restore-AzRecoveryServicesBackupItem","0","1050","The parameter set '__AllParameterSets' for cmdlet 'Restore-AzRecoveryServicesBackupItem' has been removed.","Add parameter set '__AllParameterSets' back to cmdlet 'Restore-AzRecoveryServicesBackupItem'."


What was the reason to suppress the exception?

The restore cmdlet used to have a parameter RecoveryPoint which was used by all paramsets and was mandatory. With the introduction of a new workload, WLRecoveryConfig is the only parameter required for this cmdlet.

vladimir-shcherbakov · 2019-02-19T22:15:13Z

src/RecoveryServices/RecoveryServices.Backup.Test/ScenarioTests/AzureWorkload/ItemTests.ps1

+				-BackupManagementType "AzureWorkload" `
+				-WorkloadType "MSSQL" `
+				-Policy $policy;
+


What is your approach to check the cmdlet works as expected without using Assert-* through the test?

AutoProtection APIs are intents rather than long-running operations and we dont have any cmdlet to get intent's status. We have added this in our ToDos to come up with a better test plan.

vladimir-shcherbakov · 2019-02-21T04:21:47Z

Some comments are not addressed (discussed offline).

vladimir-shcherbakov · 2019-02-23T02:42:11Z

src/RecoveryServices/RecoveryServices.Backup/RecoveryServices.Backup.csproj

-    <PackageReference Include="Microsoft.Azure.Management.RecoveryServices.Backup" Version="3.0.1-preview" />
+    <PackageReference Include="Microsoft.Azure.Management.RecoveryServices.Backup" Version="3.1.1-preview" />
+    <PackageReference Include="Microsoft.Rest.ClientRuntime" Version="2.3.19" />
+    <PackageReference Include="Microsoft.Rest.ClientRuntime.Azure" Version="3.3.19" />
  </ItemGroup>


Please remove all references of ClientRuntime packages from all your .csproj files.

This Import adds all common deps for you:

azure-powershell/src/RecoveryServices/RecoveryServices.Backup/RecoveryServices.Backup.csproj

Line 7 in 0322f7f

<Import Project="$(MSBuildThisFileDirectory)..\..\Az.props" />

azuresdkci added the needs-review label Feb 13, 2019

siddharth7 force-pushed the mercury_dev_test branch from ad07e94 to d07e7ca Compare February 13, 2019 11:02

siddharth7 mentioned this pull request Feb 13, 2019

Revert "Backup Support for SQL Server in Azure VM" #8499

Merged

maddieclayton added Cmdlet Review Required ⚠️ and removed needs-review labels Feb 13, 2019

maddieclayton assigned siddharth7 Feb 13, 2019

maddieclayton added Cmdlet Review Complete and removed Cmdlet Review Required ⚠️ labels Feb 13, 2019

maddieclayton unassigned siddharth7 Feb 13, 2019

siddharth7 force-pushed the mercury_dev_test branch from d07e7ca to 779b509 Compare February 14, 2019 06:07

SDK Update

48cd964

siddharth7 force-pushed the mercury_dev_test branch from 779b509 to 48cd964 Compare February 14, 2019 06:52

Revert "Merge pull request Azure#8499 from Azure/revert-8349-mercury_…

00f2c6f

…dev_test" This reverts commit e766f60, reversing changes made to 8c3b2d1.

maddieclayton added the needs-review label Feb 14, 2019

maddieclayton assigned vladimir-shcherbakov Feb 14, 2019

vladimir-shcherbakov suggested changes Feb 19, 2019

View reviewed changes

vladimir-shcherbakov assigned siddharth7 and unassigned vladimir-shcherbakov Feb 19, 2019

vladimir-shcherbakov added needs-revision and removed needs-review labels Feb 19, 2019

review changes

051c06a

maddieclayton added needs-review and removed needs-revision labels Feb 20, 2019

maddieclayton assigned vladimir-shcherbakov and unassigned siddharth7 Feb 20, 2019

vladimir-shcherbakov added needs-revision and removed needs-review labels Feb 21, 2019

vladimir-shcherbakov assigned siddharth7 and unassigned vladimir-shcherbakov Feb 21, 2019

updated test for autoprotection

b925475

siddharth7 force-pushed the mercury_dev_test branch from 9fe5e15 to b925475 Compare February 21, 2019 16:25

updated help files

0322f7f

siddharth7 force-pushed the mercury_dev_test branch from 3cd35b2 to 0322f7f Compare February 22, 2019 12:45

vladimir-shcherbakov suggested changes Feb 23, 2019

View reviewed changes

Updated csproj

5f80d93

vladimir-shcherbakov approved these changes Feb 24, 2019

View reviewed changes

vladimir-shcherbakov merged commit 866ef22 into Azure:master Feb 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[RecoveryServices.Backup] SDK Update #8529

[RecoveryServices.Backup] SDK Update #8529

Uh oh!

siddharth7 commented Feb 13, 2019 •

edited

Loading

Uh oh!

maddieclayton commented Feb 13, 2019

Uh oh!

siddharth7 commented Feb 13, 2019

Uh oh!

vladimir-shcherbakov Feb 19, 2019

Uh oh!

siddharth7 Feb 20, 2019

Uh oh!

vladimir-shcherbakov Feb 19, 2019

Uh oh!

siddharth7 Feb 20, 2019

Uh oh!

vladimir-shcherbakov Feb 19, 2019

Uh oh!

siddharth7 Feb 20, 2019

Uh oh!

vladimir-shcherbakov Feb 19, 2019

Uh oh!

siddharth7 Feb 20, 2019 •

edited

Loading

Uh oh!

vladimir-shcherbakov Feb 19, 2019

Uh oh!

siddharth7 Feb 20, 2019

Uh oh!

vladimir-shcherbakov commented Feb 21, 2019 •

edited

Loading

Uh oh!

vladimir-shcherbakov Feb 23, 2019

Uh oh!

Uh oh!

		"AssemblyFileName","ClassName","Target","Severity","ProblemId","Description","Remediation"
		"Microsoft.Azure.PowerShell.Cmdlets.RecoveryServices.Backup.dll","Microsoft.Azure.Commands.RecoveryServices.Backup.Cmdlets.RestoreAzureRmRecoveryServicesBackupItem","Restore-AzRecoveryServicesBackupItem","0","1050","The parameter set '__AllParameterSets' for cmdlet 'Restore-AzRecoveryServicesBackupItem' has been removed.","Add parameter set '__AllParameterSets' back to cmdlet 'Restore-AzRecoveryServicesBackupItem'."

[RecoveryServices.Backup] SDK Update #8529

[RecoveryServices.Backup] SDK Update #8529

Uh oh!

Conversation

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

Uh oh!

Description

Checklist

Uh oh!

maddieclayton commented Feb 13, 2019

Uh oh!

siddharth7 commented Feb 13, 2019

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!

siddharth7 Feb 20, 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!

vladimir-shcherbakov commented Feb 21, 2019 • 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!

Uh oh!

siddharth7 commented Feb 13, 2019 •

edited

Loading

siddharth7 Feb 20, 2019 •

edited

Loading

vladimir-shcherbakov commented Feb 21, 2019 •

edited

Loading