Update ResourceManager SDK to 1.2.0-preview version. #2848

TianoMS · 2016-08-28T00:07:09Z

This checklist is used to make sure that common issues in a pull request are covered by the creator. You can find a more complete discussion of PowerShell cmdlet best practices here.

Below in Overall Changes, check off the boxes that apply to your PR. Within each of the categories that you select, make sure that you can check off all of the boxes.

General

Title of the PR is clear and informative
There are a small number of commits that each have an informative message
If it applies, references the bug/issue that the PR fixes
All files have the Microsoft copyright header
Cmdlets refer to management libraries through nuget references - no dlls are checked in
The PR does not introduce breaking changes (unless a major version change occurs in the assembly and module)

Tests

PR includes test coverage for the included changes
Tests must use xunit, and should either use Moq to mock management client calls, or use the scenario test framework
PowerShell scripts used in tests must not use hard-coded values for location
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 existing resources
Tests should not use App.config files for settings
Tests should use the built-in PowerShell functions for generating random names when unique names are necessary - this will store names in the test recording
Tests should use Start-Sleep to pause rather than Thread.Sleep

…tiano-d4

TianoMS · 2016-08-30T01:54:27Z

@markcowl My change upgrades to Microsoft.Azure.Management.ResourceManager.1.1.5-preview. I did it only for the Resources solution. Most of the other projects are still using 1.1.1-preview version. Let me know if you see a problem in it.

TianoMS · 2016-08-30T04:25:08Z

On demand job: http://azuresdkci.cloudapp.net/view/1-AzurePowerShell/job/powershell-demand/1113/

vivsriaus · 2016-08-30T05:26:10Z

@markcowl @hovsepm Adding on @TianoMS comment above regarding different versions of the same assembly. I recall there was an issue with the way powershell loaded assemblies that differed in just the maintenance version (1.1.1 vs 1.1.5). Since some projects reference 1.1.1 and ours (with this PR) reference 1.1.5, we want to ensure that this doesn't pose any problems for our cmdlets. i.e. - we don't have to force the powershell to load the 1.1.5 assembly via Add-Type to use it in our cmdlet.

vivsriaus · 2016-08-30T05:27:05Z

src/ResourceManager/Resources/Commands.Resources.Test/Commands.Resources.Test.csproj

@@ -116,7 +116,7 @@
    </Reference>
    <Reference Include="Microsoft.Rest.ClientRuntime.Azure.TestFramework, Version=1.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL">
      <SpecificVersion>False</SpecificVersion>
-      <HintPath>..\..\..\packages\Microsoft.Rest.ClientRuntime.Azure.TestFramework.1.3.4-preview\lib\net45\Microsoft.Rest.ClientRuntime.Azure.TestFramework.dll</HintPath>
+      <HintPath>..\..\..\packages\Microsoft.Rest.ClientRuntime.Azure.TestFramework.1.3.6-preview\lib\net45\Microsoft.Rest.ClientRuntime.Azure.TestFramework.dll</HintPath>


Was this intentional?

This is about another issue. I think it's already fixed. Should not be a problem.

vivsriaus · 2016-08-30T05:31:44Z

@TianoMS Since this is an api-version bump, why didn't we update the assembly's minor version instead (1.2.0 instead of 1.1.5)?

TianoMS · 2016-08-30T05:45:16Z

@vivsriaus I have another change in SDK (the parenthesis in resource group names) recently which updated the version to 1.1.6-preview. It's not published yet. Maybe we can update the version to 1.2.0 there.

vivsriaus · 2016-08-30T05:47:56Z

Yes, let's please do that @TianoMS. api-version bumps definitely warrants at least a minor version update. We should be fine keeping the powershell changes as-is, since the implementation is internal.

cormacpayne · 2016-08-30T16:10:17Z

@TianoMS Hey Tian, your on-demand build failed because of a bug in a test that we merged a fix for yesterday evening, so if you pull from dev and rebase, that should fix the problem.

…tiano-d4

markcowl · 2016-08-30T18:54:34Z

LGTM once the on-demand build passes.

on demand: http://azuresdkci.cloudapp.net/view/1-AzurePowerShell/job/powershell-demand/1122/

TianoMS · 2016-08-30T18:57:39Z

@markcowl Please hold on with the merge. We have a newer version of SDK coming out. I'll push one more commit to upgrade to that version (1.2.0) directly.

TianoMS · 2016-08-30T23:38:46Z

@markcowl This is now ready to merge. On demand job:
http://azuresdkci.cloudapp.net/view/1-AzurePowerShell/job/powershell-demand/1126/

markcowl · 2016-08-31T00:22:19Z

src/ResourceManager/Resources/Commands.Resources.Test/Providers/GetAzureProviderCmdletTests.cs

@@ -153,7 +153,7 @@ public void GetsResourceProviderTests()
            };
            var pagableLocations = new Page<Location>();
            pagableLocations.SetItemValue<Location>(locationList);
-            var locationsResult = new AzureOperationResponse<IPage<Location>>()
+            var locationsResult = new AzureOperationResponse<IEnumerable<Location>>()


This is a breaking change - why is this operation no longer pageable?

The change really happens here: Azure/azure-rest-api-specs@5129caa.

At the bottom, it sets "nextLinkName" to null. This makes the generated SDK not pageable. ISubscriptionsOperations.ListLocationsWithHttpMessagesAsync now returns IEnumerable.

It's not a breaking change here. It's a breaking change in .NET SDK. I can sync up with Vivek on that change in swagger spec.

@TianoMS @markcowl That is correct. The list subscription locations was never pageable to begin with, it was incorrectly added. The above change to swagger spec @TianoMS referenced fixed the issue.

As long as it's not a breaking change here, it's OK, but IMO, you want to fix this in the sdk, you don't want to go from pageange to not pageable back to pageable

markcowl · 2016-08-31T00:23:50Z

@TianoMS One question about the removal of a pageable operation

TianoMS · 2016-08-31T01:03:32Z

@markcowl Addressed.

Tiano2017 added 2 commits August 27, 2016 17:01

Update ResourceManager SDK to 1.1.5-preview version.

6eb0ffe

Merge branch 'dev' of https://github.com/Azure/azure-powershell into …

ab978ab

…tiano-d4

azurecla added the cla-not-required label Aug 28, 2016

TianoMS changed the title ~~Tiano d4~~ Update ResourceManager SDK to 1.1.5-preview version. Aug 28, 2016

Suppress the credential check message in session record file.

ee7aff3

vivsriaus reviewed Aug 30, 2016
View reviewed changes

Merge branch 'dev' of https://github.com/Azure/azure-powershell into …

469d9e2

…tiano-d4

markcowl changed the title ~~Update ResourceManager SDK to 1.1.5-preview version.~~ [Hold]: Update ResourceManager SDK to 1.1.5-preview version. Aug 30, 2016

Update to the latest version of 1.2.0-preview.

bec2f74

TianoMS changed the title ~~[Hold]: Update ResourceManager SDK to 1.1.5-preview version.~~ [Hold]: Update ResourceManager SDK to 1.2.0-preview version. Aug 30, 2016

TianoMS changed the title ~~[Hold]: Update ResourceManager SDK to 1.2.0-preview version.~~ Update ResourceManager SDK to 1.2.0-preview version. Aug 30, 2016

markcowl reviewed Aug 31, 2016
View reviewed changes

TianoMS mentioned this pull request Aug 31, 2016

Support zones in provider and resource cmdlets. #2869

Merged

35 tasks

markcowl merged commit ec2a0c7 into Azure:dev Aug 31, 2016

Update ResourceManager SDK to 1.2.0-preview version. #2848

Update ResourceManager SDK to 1.2.0-preview version. #2848

Uh oh!

Conversation

TianoMS commented Aug 28, 2016 • edited by markcowl Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overall Changes

General

Tests

Uh oh!

TianoMS commented Aug 30, 2016

Uh oh!

TianoMS commented Aug 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vivsriaus commented Aug 30, 2016

Uh oh!

vivsriaus Aug 30, 2016

Choose a reason for hiding this comment

Uh oh!

TianoMS Aug 30, 2016

Choose a reason for hiding this comment

Uh oh!

vivsriaus commented Aug 30, 2016

Uh oh!

TianoMS commented Aug 30, 2016

Uh oh!

vivsriaus commented Aug 30, 2016

Uh oh!

cormacpayne commented Aug 30, 2016

Uh oh!

markcowl commented Aug 30, 2016

Uh oh!

TianoMS commented Aug 30, 2016

Uh oh!

TianoMS commented Aug 30, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markcowl Aug 31, 2016

Choose a reason for hiding this comment

Uh oh!

TianoMS Aug 31, 2016 • 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!

vivsriaus Aug 31, 2016

Choose a reason for hiding this comment

Uh oh!

markcowl Aug 31, 2016

Choose a reason for hiding this comment

Uh oh!

markcowl commented Aug 31, 2016

Uh oh!

TianoMS commented Aug 31, 2016

Uh oh!

Uh oh!

TianoMS commented Aug 28, 2016 •

edited by markcowl

Loading

TianoMS commented Aug 30, 2016 •

edited

Loading

TianoMS commented Aug 30, 2016 •

edited

Loading

TianoMS Aug 31, 2016 •

edited

Loading