Remove Default sku setting from NewAzureVirtualNetworkGateway commandlet #6314
Conversation
| vnetGateway.Sku.Name = MNM.VirtualNetworkGatewaySkuTier.Basic; | ||
| } | ||
| // If gateway sku param value is not passed, - Let NRP make the decision, just pass it as null here | ||
| vnetGateway.Sku = null; |
There was a problem hiding this comment.
I would expect this change to break this test.
Looking at the PS test code
The test tries to create a new gateway and the cmdlet call does not specify a SKU.
Heres the link in the test recording PUT request where the Sku is not null.
With the new code change I would expect to see the SKU to be null in the request which should have flagged the test as a failure (since the recording does not match what went on the wire)...
@ritwikbasu
I would recommend that even though the test did not fail, we re-record the test as it does not reflect the new behavior.
There was a problem hiding this comment.
@praries880 - This does not break that test since NRP assigns the same SKU when the SKU is null. The idea of this change is to allow NRP to make the decision instead of Powershell assigning a default sku. I will push an additional test case where the SKU is null.
None of the flows are broken, neither has any logic changed. As mentioned, even if the Sku is set as null in the request, NRP would still assign the same default sku, hence the test would still work.
There was a problem hiding this comment.
@ritwikbasu
The request's body will change based on the current change. Nothing is broken, but the Json representing what went over the wire is wrong. How much time/effort does it take to re-record the current test?
There was a problem hiding this comment.
@praries880 - The test is being re-recorded. Should be done in the next couple of hours.
There was a problem hiding this comment.
Added re-recorded test case.
|
@MikhailTryakhov can you look at this PR? |
|
@ritwikbasu Please also open a PR targeting this branch: https://github.com/Azure/azure-powershell/tree/Network_preview. We will do the out of band release from here. |
Description
Checklist
CONTRIBUTING.mdplatyPSmodule