[HDInsight] Call out breaking changes #9999

aim-for-better · 2019-09-10T03:33:01Z

Description

This PR is related to previous PR #9941 . In PR #9941 we did some breaking changes and merged into Az.HDInsight-preview branch. This PR is in order to call out breaking changes in master branch.

Deprecated Get/Enable/Disable-AzHDInsightOMS and replaced them with Get/Enable/Disable-AzHDInsightMonitoring
Deprecated Grant/Revoke-AzHDInsightRdpServicesAccess cmdlets with no replacement.
Change the output type of Get-AzHDInsightProperty and Set-AzHDInsightGatewayCredential
remove a outdated breaking change notes

Checklist

I have read the Submitting Changes section of CONTRIBUTING.md
The title of the PR is clear and informative
The appropriate ChangeLog.md file(s) has been updated:
- For any service, the ChangeLog.md file can be found at src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md
- A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header -- no new version header should be added
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:
- a cmdlet design review was approved for the changes in this repository (Microsoft internal only)
- the markdown help files have been regenerated using the commands listed here

src/HDInsight/HDInsight/Az.HDInsight.psd1

src/HDInsight/HDInsight/ManagementCommands/AddAzureHDInsightConfigValuesCommand.cs

src/HDInsight/HDInsight/ManagementCommands/GetAzureHDInsightPropertiesCommand.cs

msJinLei · 2019-09-11T08:56:09Z

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

@@ -0,0 +1,5 @@
+"AssemblyFileName","ClassName","Target","Severity","ProblemId","Description","Remediation"


These lines will suppress breaking change warnings of static analysis, which are not expected in a breaking change notice release.

Hi @msJinLei , I want to call out breaking change and remove outdated alias in the same PR.

Hi @msJinLei , I want to call out breaking change and remove outdated alias in the same PR.

Two separate prs are expected in that cases. One for breaking changes preview release & one for breaking changes notice release.

Hi @msJinLei , I want to call out breaking change and remove outdated alias in the same PR.

Two separate prs are expected in that cases. One for breaking changes preview release & one for breaking changes notice release.

Hi , I have updated the PR, this PR only call out breaking change. But we still add suppression otherwise I will failed at analyse phrase for a false positive violdation error.

OK. I will resolve it if the previous issues are fixed.

msJinLei · 2019-09-11T09:14:09Z

src/HDInsight/HDInsight/ManagementCommands/GrantAzureHDInsightRdpServicesAccessCommand.cs

 using System;
 using System.Management.Automation;

 namespace Microsoft.Azure.Commands.HDInsight
 {
+    [CmdletDeprecation()]


Please provide with an instruction for the customers who are using the cmdlet on how they should do with their jobs and scripts.

Please provide with an instruction for the customers who are using the cmdlet on how they should do with their jobs and scripts.

Hi @msJinLei , I think that we don't have to do this. There is no replacement of this cmdlet. This cmdlet is related with specific os type cluster which has been deprecated. Customers can't use this cmdlet any more. We have removed this cmdlet in previous PR #9941 .

Please provide with an instruction for the customers who are using the cmdlet on how they should do with their jobs and scripts.

Hi @msJinLei , I think that we don't have to do this. There is no replacement of this cmdlet. This cmdlet is related with specific os type cluster which has been deprecated. Customers can't use this cmdlet any more. We have removed this cmdlet in previous PR #9941 .

when customer use this cmdlet they will get a message: "The cmdlet is being deprecated. There will be no replacement for it." This is actually a good instruction.

What about the previous scripts that contains this cmdlet. How the users are proposed to modify their scripts?

What about the previous scripts that contains this cmdlet. How the users are proposed to modify their scripts?

Hi @msJinLei , I have updated the PR and added instructions reference AddAzureASAccount.cs

msJinLei · 2019-09-11T09:14:25Z

src/HDInsight/HDInsight/ManagementCommands/RevokeAzureHDInsightRdpServicesAccessCommand.cs

 using System.Management.Automation;

 namespace Microsoft.Azure.Commands.HDInsight
 {
+    [CmdletDeprecation()]


Ditto

the same with above.

See above.

Hi @msJinLei , I have updated the PR and added instructions reference AddAzureASAccount.cs

msJinLei · 2019-09-12T08:09:05Z

You have removed outdated breaking change notes. Actually I misunderstood your purpose previously. You can keep it in this patch.

aim-for-better · 2019-09-12T09:53:51Z

You have removed outdated breaking change notes. Actually I misunderstood your purpose previously. You can keep it in this patch.

Thanks I have added back. Thanks for your review.

Remove outdated breaking change Add suppression

markcowl · 2019-09-13T03:55:35Z

src/HDInsight/HDInsight/ManagementCommands/AddAzureHDInsightConfigValuesCommand.cs

    [Cmdlet("Add", ResourceManager.Common.AzureRMConstants.AzureRMPrefix + "HDInsightConfigValue"),OutputType(typeof(AzureHDInsightConfig))]
-    [Alias("Add-AzHDInsightConfigValues")]


So, to be clear, if you did not remove this alias at the last breaking chaneg release, you cannot remove it now. If you want to remove it at the next breaking change release, then leave the braking change notce and the alias here, and you can remove it in November.

Otherwise, please keep the alias and removing the braking change notice is OK.

Note that removing this alias is one of the current static analysis failures preventing us from merging this PR.

So, to be clear, if you did not remove this alias at the last breaking chaneg release, you cannot remove it now. If you want to remove it at the next breaking change release, then leave the braking change notce and the alias here, and you can remove it in November.

Otherwise, please keep the alias and removing the braking change notice is OK.

Hi @markcowl Thanks for your comments. I decided to keep the alias and breaking change notice. I will remove it in November.

Note that removing this alias is one of the current static analysis failures preventing us from merging this PR.

As for static analysis failure, per my side I think it is false positive validation error.
Because the cmdlets in the breaking change result has been removed at the version 2.0.1.
And I have added suppression.

More details:
I exec msbuild build.proj /t:StaticAnalysis /p:Configuration=Debug at my local machine, I got the following breaking changes:

AssemblyFileName ClassName Target Severity ProblemId Description Remediation

Microsoft.Azure.PowerShell.Cmdlets.HDInsight.dll Microsoft.Azure.Commands.HDInsight.GrantAzureHDInsightHttpServicesAccessCommand Grant-AzHDInsightHttpServicesAccess 0 1000 The cmdlet 'Grant-AzHDInsightHttpServicesAccess' has been removed and no alias was found for the original cmdlet name. Add the cmdlet 'Grant-AzHDInsightHttpServicesAccess' back to the module, or add an alias to the original cmdlet name.

Microsoft.Azure.PowerShell.Cmdlets.HDInsight.dll Microsoft.Azure.Commands.HDInsight.RevokeAzureHDInsightHttpServicesAccessCommand Revoke-AzHDInsightHttpServicesAccess 0 1000 The cmdlet 'Revoke-AzHDInsightHttpServicesAccess' has been removed and no alias was found for the original cmdlet name. Add the cmdlet 'Revoke-AzHDInsightHttpServicesAccess' back to the module, or add an alias to the original cmdlet name.

I don't remove this two cmdlets in this PR, they have been removed at the version 2.0.1.

markcowl · 2019-09-13T03:58:23Z

src/HDInsight/HDInsight/ManagementCommands/GetAzureHDInsightPropertiesCommand.cs

    [Cmdlet("Get", ResourceManager.Common.AzureRMConstants.AzureRMPrefix + "HDInsightProperty"),OutputType(typeof(CapabilitiesResponse))]
-    [Alias("Get-AzHDInsightProperties")]


Same comment. You cannot remove this alias until a breaking change release. If you want to remove the alias at the next breaking change release in November, then leave this alias and the breaking change attribute here and you can remove it then.

Otherwise remove the old breakign change attribute and leave the alias.

This is the other thing that is causing static analysis failures, which prevent us from merging this PR.

Same comment. You cannot remove this alias until a breaking change release. If you want to remove the alias at the next breaking change release in November, then leave this alias and the breaking change attribute here and you can remove it then.

Otherwise remove the old breakign change attribute and leave the alias.

This is the other thing that is causing static analysis failures, which prevent us from merging this PR.

Thanks. The same with above.

markcowl · 2019-09-16T02:56:26Z

src/HDInsight/HDInsight/ManagementCommands/GrantAzureHDInsightRdpServicesAccessCommand.cs

 using System;
 using System.Management.Automation;

 namespace Microsoft.Azure.Commands.HDInsight
 {
+    [CmdletDeprecation("3.0.0")]


You need to provide a good, customer-facing string here. What should the customer do if this cmdlet is not here to grant access? Is there a different mechanism? Is RDP service being discontinued? The customer needs to be able to use this string to figure out how to change their script.

You need to provide a good, customer-facing string here. What should the customer do if this cmdlet is not here to grant access? Is there a different mechanism? Is RDP service being discontinued? The customer needs to be able to use this string to figure out how to change their script.

Fixed.

markcowl · 2019-09-16T02:56:46Z

src/HDInsight/HDInsight/ManagementCommands/RevokeAzureHDInsightRdpServicesAccessCommand.cs

 using System.Management.Automation;

 namespace Microsoft.Azure.Commands.HDInsight
 {
+    [CmdletDeprecation("3.0.0")]


Same comment as with Grant above.

Same comment as with Grant above.

Fixed.

markcowl · 2019-09-16T02:57:20Z

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

@@ -0,0 +1,3 @@
+"AssemblyFileName","ClassName","Target","Severity","ProblemId","Description","Remediation"
+"Microsoft.Azure.PowerShell.Cmdlets.HDInsight.dll","Microsoft.Azure.Commands.HDInsight.GrantAzureHDInsightHttpServicesAccessCommand","Grant-AzHDInsightHttpServicesAccess","0","1000","The cmdlet 'Grant-AzHDInsightHttpServicesAccess' has been removed and no alias was found for the original cmdlet name.","Add the cmdlet 'Grant-AzHDInsightHttpServicesAccess' back to the module, or add an alias to the original cmdlet name."


These still need to be removed.

These still need to be removed.

Removed.

But I still have static analysis error at my local machine even through I executed static analysis under newest master branch.

markcowl · 2019-09-16T02:58:56Z

@aim-for-better Code complete for this release is EOD Monday. Please make requested changes by then if you want to be in the release.

aim-for-better · 2019-09-16T03:06:19Z

@aim-for-better Code complete for this release is EOD Monday. Please make requested changes by then if you want to be in the release.

Thanks for your reminder.

suppression.

aim-for-better · 2019-09-16T03:22:30Z

@aim-for-better Code complete for this release is EOD Monday. Please make requested changes by then if you want to be in the release.

I have updated the PR.

markcowl · 2019-09-16T04:43:38Z

src/HDInsight/HDInsight/ManagementCommands/GrantAzureHDInsightRdpServicesAccessCommand.cs

 using System;
 using System.Management.Automation;

 namespace Microsoft.Azure.Commands.HDInsight
 {
+    [CmdletDeprecation("This cmdlet will be deprecated in an upcoming breaking change release 3.0.0. Please use the cmdlet Set-AzHDInsightGatewayCredential to set connection credential.")]


Suggested change

[CmdletDeprecation("This cmdlet will be deprecated in an upcoming breaking change release 3.0.0. Please use the cmdlet Set-AzHDInsightGatewayCredential to set connection credential.")]

[CmdletDeprecation("This cmdlet will be deprecated in an upcoming breaking change release. Please use the cmdlet Set-AzHDInsightGatewayCredential to set a connection credential.")]

markcowl · 2019-09-16T04:45:02Z

src/HDInsight/HDInsight/ManagementCommands/RevokeAzureHDInsightRdpServicesAccessCommand.cs

 using System.Management.Automation;

 namespace Microsoft.Azure.Commands.HDInsight
 {
+    [CmdletDeprecation("Clusters based Windows Os Type don't be supported, this cmdlet will be deprecated in an upcoming breaking change release 3.0.0 with no replacement. Please create cluster based Linux Os Type.")]


Suggested change

[CmdletDeprecation("Clusters based Windows Os Type don't be supported, this cmdlet will be deprecated in an upcoming breaking change release 3.0.0 with no replacement. Please create cluster based Linux Os Type.")]

[CmdletDeprecation("Clusters using Windows Os Type will not be supported in the future, this cmdlet will be deprecated in an upcoming breaking change release with no replacement. Please create a cluster using Linux Os Type instead.")]

erich-wang · 2019-09-17T06:06:12Z

/azp run azure-powershell - security-tools

azure-pipelines · 2019-09-17T06:06:19Z

Azure Pipelines successfully started running 1 pipeline(s).

erich-wang · 2019-09-17T06:46:39Z

Merge as Admin since there's issue in ADO pipeline notification.

aim-for-better requested a review from cormacpayne September 10, 2019 03:33

azuresdkci added the needs-review label Sep 10, 2019

markcowl assigned markcowl and msJinLei Sep 10, 2019

aim-for-better force-pushed the CallOutBreakingChange branch 3 times, most recently from 9752e4a to dfb37b8 Compare September 11, 2019 02:41

msJinLei reviewed Sep 11, 2019

View reviewed changes

src/HDInsight/HDInsight/Az.HDInsight.psd1 Outdated Show resolved Hide resolved

msJinLei reviewed Sep 11, 2019

View reviewed changes

src/HDInsight/HDInsight/ManagementCommands/AddAzureHDInsightConfigValuesCommand.cs Outdated Show resolved Hide resolved

msJinLei reviewed Sep 11, 2019

View reviewed changes

src/HDInsight/HDInsight/ManagementCommands/GetAzureHDInsightPropertiesCommand.cs Outdated Show resolved Hide resolved

msJinLei reviewed Sep 11, 2019

View reviewed changes

msJinLei added needs-revision and removed needs-review labels Sep 11, 2019

msJinLei assigned aim-for-better and unassigned markcowl and msJinLei Sep 11, 2019

aim-for-better force-pushed the CallOutBreakingChange branch from dfb37b8 to 7e628b3 Compare September 11, 2019 11:04

aim-for-better requested a review from msJinLei September 12, 2019 02:19

erich-wang added needs-review Sprint Candidate and removed needs-revision labels Sep 12, 2019

erich-wang assigned msJinLei Sep 12, 2019

msJinLei added needs-revision and removed needs-review labels Sep 12, 2019

aim-for-better force-pushed the CallOutBreakingChange branch from 7e628b3 to 738b6c8 Compare September 12, 2019 09:45

aim-for-better force-pushed the CallOutBreakingChange branch from 738b6c8 to 403c65e Compare September 12, 2019 10:03

Call out breaking change and remove

46e6be6

Remove outdated breaking change Add suppression

aim-for-better force-pushed the CallOutBreakingChange branch from 403c65e to 46e6be6 Compare September 12, 2019 11:09

markcowl requested changes Sep 13, 2019

View reviewed changes

markcowl unassigned msJinLei Sep 13, 2019

Just call out breaking changes

fdd076f

markcowl requested changes Sep 16, 2019

View reviewed changes

markcowl added the Discuss label Sep 16, 2019

Add a good, customer-facing string and remove breaking changes

041f3f0

suppression.

aim-for-better requested a review from markcowl September 16, 2019 03:25

markcowl requested changes Sep 16, 2019

View reviewed changes

aim-for-better requested a review from markcowl September 16, 2019 05:09

aim-for-better force-pushed the CallOutBreakingChange branch from b139ea9 to 3d4727d Compare September 16, 2019 05:19

Modify breaking change notes.

b56e04a

aim-for-better force-pushed the CallOutBreakingChange branch from 3d4727d to b56e04a Compare September 16, 2019 08:21

erich-wang added needs-review and removed needs-revision labels Sep 17, 2019

erich-wang assigned markcowl Sep 17, 2019

erich-wang added Waiting for CI and removed Discuss needs-review labels Sep 17, 2019

erich-wang self-assigned this Sep 17, 2019

erich-wang approved these changes Sep 17, 2019

View reviewed changes

erich-wang merged commit ef3f6bf into Azure:master Sep 17, 2019

		@@ -0,0 +1,5 @@
		"AssemblyFileName","ClassName","Target","Severity","ProblemId","Description","Remediation"

		[Cmdlet("Add", ResourceManager.Common.AzureRMConstants.AzureRMPrefix + "HDInsightConfigValue"),OutputType(typeof(AzureHDInsightConfig))]
		[Alias("Add-AzHDInsightConfigValues")]

AssemblyFileName	ClassName	Target	Severity	ProblemId	Description	Remediation
Microsoft.Azure.PowerShell.Cmdlets.HDInsight.dll	Microsoft.Azure.Commands.HDInsight.GrantAzureHDInsightHttpServicesAccessCommand	Grant-AzHDInsightHttpServicesAccess	0	1000	The cmdlet 'Grant-AzHDInsightHttpServicesAccess' has been removed and no alias was found for the original cmdlet name.	Add the cmdlet 'Grant-AzHDInsightHttpServicesAccess' back to the module, or add an alias to the original cmdlet name.
Microsoft.Azure.PowerShell.Cmdlets.HDInsight.dll	Microsoft.Azure.Commands.HDInsight.RevokeAzureHDInsightHttpServicesAccessCommand	Revoke-AzHDInsightHttpServicesAccess	0	1000	The cmdlet 'Revoke-AzHDInsightHttpServicesAccess' has been removed and no alias was found for the original cmdlet name.	Add the cmdlet 'Revoke-AzHDInsightHttpServicesAccess' back to the module, or add an alias to the original cmdlet name.

		[Cmdlet("Get", ResourceManager.Common.AzureRMConstants.AzureRMPrefix + "HDInsightProperty"),OutputType(typeof(CapabilitiesResponse))]
		[Alias("Get-AzHDInsightProperties")]

		@@ -0,0 +1,3 @@
		"AssemblyFileName","ClassName","Target","Severity","ProblemId","Description","Remediation"
		"Microsoft.Azure.PowerShell.Cmdlets.HDInsight.dll","Microsoft.Azure.Commands.HDInsight.GrantAzureHDInsightHttpServicesAccessCommand","Grant-AzHDInsightHttpServicesAccess","0","1000","The cmdlet 'Grant-AzHDInsightHttpServicesAccess' has been removed and no alias was found for the original cmdlet name.","Add the cmdlet 'Grant-AzHDInsightHttpServicesAccess' back to the module, or add an alias to the original cmdlet name."

	[CmdletDeprecation("This cmdlet will be deprecated in an upcoming breaking change release 3.0.0. Please use the cmdlet Set-AzHDInsightGatewayCredential to set connection credential.")]
	[CmdletDeprecation("This cmdlet will be deprecated in an upcoming breaking change release. Please use the cmdlet Set-AzHDInsightGatewayCredential to set a connection credential.")]

	[CmdletDeprecation("Clusters based Windows Os Type don't be supported, this cmdlet will be deprecated in an upcoming breaking change release 3.0.0 with no replacement. Please create cluster based Linux Os Type.")]
	[CmdletDeprecation("Clusters using Windows Os Type will not be supported in the future, this cmdlet will be deprecated in an upcoming breaking change release with no replacement. Please create a cluster using Linux Os Type instead.")]

[HDInsight] Call out breaking changes #9999

[HDInsight] Call out breaking changes #9999

Uh oh!

Conversation

aim-for-better commented Sep 10, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

Uh oh!

Uh oh!

Uh oh!

msJinLei Sep 11, 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!

aim-for-better Sep 11, 2019 • edited by msJinLei 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!

aim-for-better Sep 11, 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!

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!

aim-for-better Sep 12, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

msJinLei commented Sep 12, 2019

Uh oh!

aim-for-better commented Sep 12, 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aim-for-better Sep 16, 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!

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!

aim-for-better Sep 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

aim-for-better commented Sep 10, 2019 •

edited

Loading

msJinLei Sep 11, 2019 •

edited

Loading

aim-for-better Sep 11, 2019 •

edited by msJinLei

Loading

aim-for-better Sep 11, 2019 •

edited

Loading

aim-for-better Sep 12, 2019 •

edited

Loading

aim-for-better commented Sep 12, 2019 •

edited

Loading

aim-for-better Sep 16, 2019 •

edited

Loading

aim-for-better Sep 16, 2019 •

edited

Loading