Update Cognitive Services Mgmt. PS with the latest Cognitive Services Mgmt. SDK #5200

felixwa · 2017-12-29T03:58:13Z

Apply Cognitive Services SDK 3.0.0.0
Update NuGet refs

Description

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.

1. Apply Cognitive Services SDK 3.0.0.0 2. Update NuGet refs

felixwa · 2017-12-29T04:03:46Z

Hi reviewer, I guess that I need to bump the AssemblyVersion/AssemblyFileVersion in AssemblyInfo.cs but I'm not sure what the version it should be as I see your team controls that for every release. So let me know.

Revision to 0.9.1

felixwa · 2017-12-29T04:05:01Z

...CognitiveServices/Commands.Management.CognitiveServices/Models/PSCognitiveServicesAccount.cs

@@ -55,7 +55,7 @@ public PSCognitiveServicesAccount(CognitiveServicesModels.CognitiveServicesAccou

        public string Etag { get; private set; }

-        public ProvisioningState? ProvisioningState { get; private set; }
+        public string ProvisioningState { get; private set; }


This is the only meaningful change introduced by new SDK (3.0.0.0) ~~but I don't think it is breaking.~~
Added to BreakingChangeIssues.csv.

@felixwa This is a breaking change, right. As in, I could have had scripts with things like:

$account.ProvisioningState == [ProvisioningState]::Running

and such a script will now be broken. I think you need to adapt this type in PSCognitiveServicesAccount so that this is not broken. It is OK to add an additional string field and deprecate this field, but you will need to do some translation between the string field from the wire and this enumeration until the next breaking change release (build)

@markcowl , the meaning of this PR is to introduce this change as we found we cannot enum all possible ProvisioningState so we changed the Swagger, SDK then this PS, change its type from an enum to a string.

I don't want to add back that enum in PS as it has gone in SDK also I cannot predicate possible values of that enum. If RP returns back a value not in the enum, PS throws an exception at client side.

Is there an option we wait "until the next breaking change release (build)"? If so we could wait but I really don't want to add that enum back in PS and only in PS.

@felixwa breaking changes are not allowed. You will need to make a powershell-specific fix for this one field, and you should be able to do this without throwing on the client side.

Downgrade Newtonsoft.Json 10->6 as 10 resutls in UT cases fail at buddy build (though can pass at my local).

Downgrade NuGet refs in Test project as UT cases in buddy build keep failing.

Add breaking changes into exception list.

Quote the breaking change exceptions.

Quote every field of BreakingCHangeIssues.csv

Quote columns of BreakingChangeIssues.csv

markcowl · 2018-01-05T17:46:52Z

...eServices/Commands.Management.CognitiveServices/Commands.Management.CognitiveServices.csproj

    </Reference>
+    <Reference Include="Microsoft.Rest.ClientRuntime, Version=2.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL" />


these should come from the common dependecies file

markcowl · 2018-01-05T17:51:32Z

...CognitiveServices/Commands.Management.CognitiveServices/Models/PSCognitiveServicesAccount.cs

@@ -55,7 +55,7 @@ public PSCognitiveServicesAccount(CognitiveServicesModels.CognitiveServicesAccou

        public string Etag { get; private set; }

-        public ProvisioningState? ProvisioningState { get; private set; }
+        public string ProvisioningState { get; private set; }


@felixwa This is a breaking change, right. As in, I could have had scripts with things like:

$account.ProvisioningState == [ProvisioningState]::Running

and such a script will now be broken. I think you need to adapt this type in PSCognitiveServicesAccount so that this is not broken. It is OK to add an additional string field and deprecate this field, but you will need to do some translation between the string field from the wire and this enumeration until the next breaking change release (build)

markcowl · 2018-01-05T17:51:59Z

tools/StaticAnalysis/Exceptions/BreakingChangeIssues.csv

@@ -781,3 +781,6 @@
 "Microsoft.Azure.Commands.Compute.dll","Microsoft.Azure.Commands.Compute.SetAzureVMCommand","Set-AzureRmVM","0","2080","The parameter 'Generalized' in parameter set 'GeneralizeIdParameterSetName' for cmdlet 'Set-AzureRmVM' no longer has the attribute 'ValueFromPipelineByPropertyName'.","Add the attribute 'ValueFromPipelineByPropertyName' back to parameter 'Generalized' in parameter set 'GeneralizeIdParameterSetName'."
 "Microsoft.Azure.Commands.Compute.dll","Microsoft.Azure.Commands.Compute.SetAzureVMCommand","Set-AzureRmVM","0","2060","The position of parameter 'Redeploy' has changed for parameter set 'RedeployIdParameterSetName' for cmdlet 'Set-AzureRmVM'.","Revert the position change made to parameter 'Redeploy' for parameter set 'RedeployIdParameterSetName'."
 "Microsoft.Azure.Commands.Compute.dll","Microsoft.Azure.Commands.Compute.SetAzureVMCommand","Set-AzureRmVM","0","2080","The parameter 'Redeploy' in parameter set 'RedeployIdParameterSetName' for cmdlet 'Set-AzureRmVM' no longer has the attribute 'ValueFromPipelineByPropertyName'.","Add the attribute 'ValueFromPipelineByPropertyName' back to parameter 'Redeploy' in parameter set 'RedeployIdParameterSetName'."
+"d:\workspace\powershell\src\Package\Debug\ResourceManager\AzureResourceManager\AzureRM.CognitiveServices\Microsoft.Azure.Commands.Management.CognitiveServices.dll","Microsoft.Azure.Commands.Management.CognitiveServices.GetAzureCognitiveServicesAccountCommand","Get-AzureRmCognitiveServicesAccount","0","3000","The type of property 'ProvisioningState' of type 'Microsoft.Azure.Commands.Management.CognitiveServices.Models.PSCognitiveServicesAccount' has changed from 'System.Nullable`1[Microsoft.Azure.Management.CognitiveServices.Models.ProvisioningState]' to 'System.String'.","Change the type of property 'ProvisioningState' back to 'System.Nullable`1[Microsoft.Azure.Management.CognitiveServices.Models.ProvisioningState]'."


not allowed

@felixwa breaking changes are not allowed until the breaking change release. You will need to deprecate this field and adapt to the breaking change in the wire - likely by intrpoducing a new field to your output type, and providing the best available mapping in the existing enum property

markcowl · 2018-01-05T17:54:10Z

@felixwa Also, please update changelog.md with these changes

Remove dlls exist in common dependencies

1. After disucssion with Mark, redefine Provisioning state in PS and mark it as obsolete. 2. Log changes in Changelog.md 3. Revert changes in BreakingChangeIssues.csv 4. Revision to 0.9.1

Revert BreakingCHangeIssues.csv

Update upcoming-breaking-changes.md

Update upcoming-breaking-changes.md 2

markcowl · 2018-01-17T00:45:05Z

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

Update PS with the latest Cognitive Services SDK

79e2a7b

1. Apply Cognitive Services SDK 3.0.0.0 2. Update NuGet refs

azuresdkci added the needs-review label Dec 29, 2017

azuresdkci assigned markcowl Dec 29, 2017

felixwa commented Dec 29, 2017

View reviewed changes

felixwa added 6 commits December 29, 2017 13:54

Downgrade Newtonsoft.Json 10->6

18eddf2

Downgrade Newtonsoft.Json 10->6 as 10 resutls in UT cases fail at buddy build (though can pass at my local).

Downgrade NuGet refs in Test project

6a65496

Downgrade NuGet refs in Test project as UT cases in buddy build keep failing.

Add breaking changes into exception list.

3369b53

Add breaking changes into exception list.

Quote the breaking change exceptions.

8c8854c

Quote the breaking change exceptions.

Quote every field of BreakingCHangeIssues.csv

f2f2e17

Quote every field of BreakingCHangeIssues.csv

Quote columns of BreakingChangeIssues.csv

f942576

Quote columns of BreakingChangeIssues.csv

markcowl requested changes Jan 5, 2018

View reviewed changes

markcowl added needs-revision and removed needs-review labels Jan 5, 2018

markcowl assigned felixwa and unassigned markcowl Jan 5, 2018

Remove dlls exist in common dependencies

5ae831f

Remove dlls exist in common dependencies

felixwa added needs-review and removed needs-revision labels Jan 8, 2018

markcowl added needs-revision and removed needs-review labels Jan 8, 2018

Redefine Provisioning state in PS and mark it as obsolete

ace3484

1. After disucssion with Mark, redefine Provisioning state in PS and mark it as obsolete. 2. Log changes in Changelog.md 3. Revert changes in BreakingChangeIssues.csv 4. Revision to 0.9.1

felixwa added needs-review and removed needs-revision labels Jan 10, 2018

Revert BreakingCHangeIssues.csv

edc4739

Revert BreakingCHangeIssues.csv

maddieclayton assigned markcowl and unassigned felixwa Jan 10, 2018

felixwa added 2 commits January 15, 2018 15:38

Update upcoming-breaking-changes.md

172c169

Update upcoming-breaking-changes.md

Update upcoming-breaking-changes.md 2

c151e94

Update upcoming-breaking-changes.md 2

markcowl approved these changes Jan 17, 2018

View reviewed changes

markcowl removed their assignment Jan 17, 2018

markcowl added Waiting for On Demand and removed needs-review labels Jan 17, 2018

markcowl merged commit 3576e35 into Azure:preview Jan 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update Cognitive Services Mgmt. PS with the latest Cognitive Services Mgmt. SDK #5200

Update Cognitive Services Mgmt. PS with the latest Cognitive Services Mgmt. SDK #5200

Uh oh!

felixwa commented Dec 29, 2017 •

edited

Loading

Uh oh!

felixwa commented Dec 29, 2017 •

edited

Loading

Uh oh!

felixwa Dec 29, 2017 •

edited

Loading

Uh oh!

markcowl Jan 5, 2018

Uh oh!

felixwa Jan 8, 2018 •

edited by markcowl

Loading

Uh oh!

markcowl Jan 5, 2018

Uh oh!

markcowl Jan 5, 2018

Uh oh!

markcowl Jan 5, 2018

Uh oh!

markcowl Jan 8, 2018

Uh oh!

markcowl commented Jan 5, 2018

Uh oh!

markcowl commented Jan 17, 2018

Uh oh!

Uh oh!

		</Reference>
		<Reference Include="Microsoft.Rest.ClientRuntime, Version=2.0.0.0, Culture=neutral, PublicKeyToken=31bf3856ad364e35, processorArchitecture=MSIL" />

Update Cognitive Services Mgmt. PS with the latest Cognitive Services Mgmt. SDK #5200

Update Cognitive Services Mgmt. PS with the latest Cognitive Services Mgmt. SDK #5200

Uh oh!

Conversation

felixwa commented Dec 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

felixwa commented Dec 29, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

felixwa Dec 29, 2017 • 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!

felixwa Jan 8, 2018 • edited by markcowl 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markcowl commented Jan 5, 2018

Uh oh!

markcowl commented Jan 17, 2018

Uh oh!

Uh oh!

felixwa commented Dec 29, 2017 •

edited

Loading

felixwa commented Dec 29, 2017 •

edited

Loading

felixwa Dec 29, 2017 •

edited

Loading

felixwa Jan 8, 2018 •

edited by markcowl

Loading