[Compute] Update Compute management client library to 20.0.0 #6394

hyonholee · 2018-06-05T23:31:09Z

Description

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
- the cmdlet markdown files were generated using the platyPS module

cormacpayne · 2018-06-06T17:29:53Z

@hyonholee Hey Hyonho, would you mind taking a look at the build errors after we merged in the latest from the preview branch?

praries880 · 2018-06-06T00:02:59Z

src/ResourceManager/Compute/Commands.Compute.Test/ScenarioTests/AvailabilitySetTests.ps1

@@ -70,6 +70,11 @@ function Test-AvailabilitySet
        $job = Remove-AzureRmAvailabilitySet -ResourceGroupName $rgname -Name $asetName -Force -AsJob;
        $result = $job | Wait-Job;
        Assert-AreEqual "Completed" $result.State;
+        $st = $job | Receive-Job;
+        [System.Guid]::Parse($st.RequestId);


is this here for the side-effects? should we wrap it up in an assert to check that it does not throw?

praries880 · 2018-06-06T00:06:13Z

src/ResourceManager/Compute/Commands.Compute.Test/ScenarioTests/ComputeTestCommon.ps1

+{
+    param ($result)
+
+    [System.Guid]::Parse($result.OperationId);


is this here for the side-effects? should we wrap it up in an assert to check that it does not throw?

praries880 · 2018-06-06T00:06:20Z

src/ResourceManager/Compute/Commands.Compute.Test/ScenarioTests/ComputeTestCommon.ps1

+{
+    param ($result)
+
+    [System.Guid]::Parse($result.Name);


is this here for the side-effects? should we wrap it up in an assert to check that it does not throw?

…nto june # Conflicts: # src/ResourceManager/Compute/Commands.Compute.Test/ScenarioTests/AEMExtensionTests.cs

hyonholee · 2018-06-10T06:44:47Z

@cormacpayne @praries880 I resolved merge conflict and updated the PR to address the comment.

…backup tests.

praries880 · 2018-06-11T16:31:54Z

src/ResourceManager/Compute/ChangeLog.md

@@ -18,6 +18,10 @@
        - Additional information about change #1
 -->
 ## Current Release
+* Update Compute client library version to fix following cmdlets
+    - Grant-AzureRmDisk


Is this Grant-AzureRMDiskAccess?

praries880

A nit-pick.. otherwise LGTM

…failing in NETSTANDARD.

markcowl

It looks like there is a substantial amount of added code thatr is not used anywhere. Also, there are ambiguous changes in the method calss, so that it looks like, for some methods the writeobject call is removed

markcowl · 2018-06-11T21:19:27Z

...urceManager/Compute/Commands.Compute/Generated/VirtualMachine/VirtualMachineRestartMethod.cs

@@ -79,8 +79,7 @@ protected void ExecuteVirtualMachineRestartMethod(object[] invokeMethodInputPara
            string resourceGroupName = (string)ParseParameter(invokeMethodInputParameters[0]);
            string vmName = (string)ParseParameter(invokeMethodInputParameters[1]);

-            var result = VirtualMachinesClient.Restart(resourceGroupName, vmName);
-            WriteObject(result);
+            VirtualMachinesClient.Restart(resourceGroupName, vmName);


This seems like a breaking change, is this value written elsewhere?

markcowl · 2018-06-11T21:19:47Z

...rceManager/Compute/Commands.Compute/Generated/VirtualMachine/VirtualMachineRedeployMethod.cs

@@ -79,8 +79,7 @@ protected void ExecuteVirtualMachineRedeployMethod(object[] invokeMethodInputPar
            string resourceGroupName = (string)ParseParameter(invokeMethodInputParameters[0]);
            string vmName = (string)ParseParameter(invokeMethodInputParameters[1]);

-            var result = VirtualMachinesClient.Redeploy(resourceGroupName, vmName);
-            WriteObject(result);
+            VirtualMachinesClient.Redeploy(resourceGroupName, vmName);


This seems like a breaking change, is this written elsewhere?

markcowl · 2018-06-11T21:20:01Z

...rceManager/Compute/Commands.Compute/Generated/VirtualMachine/VirtualMachinePowerOffMethod.cs

@@ -79,8 +79,7 @@ protected void ExecuteVirtualMachinePowerOffMethod(object[] invokeMethodInputPar
            string resourceGroupName = (string)ParseParameter(invokeMethodInputParameters[0]);
            string vmName = (string)ParseParameter(invokeMethodInputParameters[1]);

-            var result = VirtualMachinesClient.PowerOff(resourceGroupName, vmName);
-            WriteObject(result);
+            VirtualMachinesClient.PowerOff(resourceGroupName, vmName);


breaking change

markcowl · 2018-06-11T21:20:09Z

.../Compute/Commands.Compute/Generated/VirtualMachine/VirtualMachinePerformMaintenanceMethod.cs

@@ -79,8 +79,7 @@ protected void ExecuteVirtualMachinePerformMaintenanceMethod(object[] invokeMeth
            string resourceGroupName = (string)ParseParameter(invokeMethodInputParameters[0]);
            string vmName = (string)ParseParameter(invokeMethodInputParameters[1]);

-            var result = VirtualMachinesClient.PerformMaintenance(resourceGroupName, vmName);
-            WriteObject(result);
+            VirtualMachinesClient.PerformMaintenance(resourceGroupName, vmName);


breaking change?

markcowl · 2018-06-11T21:20:23Z

...eManager/Compute/Commands.Compute/Generated/VirtualMachine/VirtualMachineGeneralizeMethod.cs

@@ -79,8 +79,7 @@ protected void ExecuteVirtualMachineGeneralizeMethod(object[] invokeMethodInputP
            string resourceGroupName = (string)ParseParameter(invokeMethodInputParameters[0]);
            string vmName = (string)ParseParameter(invokeMethodInputParameters[1]);

-            var result = VirtualMachinesClient.Generalize(resourceGroupName, vmName);
-            WriteObject(result);
+            VirtualMachinesClient.Generalize(resourceGroupName, vmName);


breaking change?

markcowl · 2018-06-11T21:44:03Z

...pute/Commands.Compute/Generated/VirtualMachineExtension/VirtualMachineExtensionListMethod.cs

@@ -0,0 +1,113 @@
+//
+// Copyright (c) Microsoft and contributors.  All rights reserved.


what is this for?

markcowl · 2018-06-11T21:44:15Z

...te/Commands.Compute/Generated/VirtualMachineExtension/VirtualMachineExtensionUpdateMethod.cs

@@ -0,0 +1,127 @@
+//
+// Copyright (c) Microsoft and contributors.  All rights reserved.


Is this needed?

markcowl · 2018-06-11T21:44:48Z

....Compute/Generated/VirtualMachineScaleSet/VirtualMachineScaleSetGetOSUpgradeHistoryMethod.cs

@@ -0,0 +1,99 @@
+//


WHat is this for?

markcowl · 2018-06-11T21:45:17Z

...pute/Generated/VirtualMachineScaleSet/VirtualMachineScaleSetGetOSUpgradeHistoryNextMethod.cs

@@ -0,0 +1,85 @@
+//


what is this for?

markcowl · 2018-06-11T21:48:55Z

src/ResourceManager/Compute/Commands.Compute/Manual/PSOperationStatusResponse.cs

+{
+    public class PSOperationStatusResponse
+    {
+        public string Name { get; set; }


did this not exist before? Are all the properties of the previous output type included?

It looks like there are a lot of unnecessary changes in this PR

markcowl

See my comments - if we can remove the files that look as though they are unused, and verify that removing the WriteObject calls does not result in a breaking change, then this should be OK.

markcowl

wenWnet over changes with Hyonho, approving

markcowl · 2018-06-12T00:05:47Z

on demand run here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/929/

krhynerson · 2018-09-20T22:17:07Z

I'm also getting the null AccessSAS value on the Grant-AzureRmDiskAccess cmdlet.

I've upgraded the AzureRM module to version 6.8.1.

What do I need to do to apply the change from @hyonholee ?

Update Compute management client library to 20.0.0

28f2ac7

azuresdkci added the needs-review label Jun 5, 2018

praries880 self-assigned this Jun 6, 2018

praries880 self-requested a review June 6, 2018 00:23

Merge branch 'preview' into june

4536df6

cormacpayne assigned hyonholee and unassigned praries880 Jun 6, 2018

cormacpayne added needs-revision and removed needs-review labels Jun 6, 2018

praries880 reviewed Jun 8, 2018

View reviewed changes

praries880 added the Merge Conflicts label Jun 8, 2018

hyonholee added 2 commits June 9, 2018 23:34

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

7a82aa5

…nto june # Conflicts: # src/ResourceManager/Compute/Commands.Compute.Test/ScenarioTests/AEMExtensionTests.cs

Update tests for parsing operation ID and update change log.

4e3017b

Update compute client library version for Network tests and recovery …

ad1c047

…backup tests.

hyonholee force-pushed the june branch from 504ff69 to ad1c047 Compare June 10, 2018 21:05

praries880 reviewed Jun 11, 2018

View reviewed changes

praries880 mentioned this pull request Jun 11, 2018

Look into GetLocation issues in Compute tests on Netstandard builds #6436

Closed

Disable the tests in NETSTANDARD mode that are passing in NET452 and …

3559b01

…failing in NETSTANDARD.

hyonholee force-pushed the june branch from 002215d to 3559b01 Compare June 11, 2018 17:29

maddieclayton assigned markcowl and unassigned hyonholee Jun 11, 2018

maddieclayton added needs-review and removed Merge Conflicts needs-revision labels Jun 11, 2018

praries880 previously approved these changes Jun 11, 2018

View reviewed changes

markcowl reviewed Jun 11, 2018

View reviewed changes

markcowl requested changes Jun 11, 2018

View reviewed changes

markcowl reviewed Jun 11, 2018

View reviewed changes

markcowl approved these changes Jun 11, 2018

View reviewed changes

markcowl removed the needs-review label Jun 12, 2018

markcowl removed their assignment Jun 12, 2018

markcowl added the Waiting for On Demand label Jun 12, 2018

maddieclayton merged commit 0a6370a into Azure:preview Jun 12, 2018

hyonholee deleted the june branch July 17, 2018 01:34

		@@ -0,0 +1,113 @@
		//
		// Copyright (c) Microsoft and contributors. All rights reserved.

		@@ -0,0 +1,127 @@
		//
		// Copyright (c) Microsoft and contributors. All rights reserved.

[Compute] Update Compute management client library to 20.0.0 #6394

[Compute] Update Compute management client library to 20.0.0 #6394

Uh oh!

Conversation

hyonholee commented Jun 5, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

cormacpayne commented Jun 6, 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!

hyonholee commented Jun 10, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

praries880 left a comment

Choose a reason for hiding this comment

Uh oh!

markcowl 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!

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!

markcowl Jun 11, 2018 • 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!

markcowl left a comment

Choose a reason for hiding this comment

Uh oh!

markcowl left a comment

Choose a reason for hiding this comment

Uh oh!

markcowl commented Jun 12, 2018

Uh oh!

krhynerson commented Sep 20, 2018

Uh oh!

Uh oh!

hyonholee commented Jun 5, 2018 •

edited

Loading

markcowl Jun 11, 2018 •

edited

Loading