Update Batch preview module to support data plane API version 2019-08-01.10.0 and mgmt API version 2019-08-01 #10001

matthchr · 2019-09-10T15:21:05Z

Description

Update Batch preview module to support data plane API version 2019-08-01.10.0 and mgmt API version 2019-08-01

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

- This fixes Azure#9938.

- This fixes Azure#9912.

- Target Microsoft.Azure.Batch 12.0. - Target Microsoft.Azure.Management.Batch 9.0. - Update tests.

matthchr · 2019-09-10T15:22:46Z

This does include breaking changes (on purpose).

The cmdlet review was done here: https://github.com/Azure/azure-powershell-cmdlet-review-pr/issues/371

markcowl · 2019-09-11T08:42:52Z

@matthchr Looks like you need to update the online help version header in your new markdown files

matthchr · 2019-09-11T16:35:26Z

Thanks @markcowl - I've fixed that. I believe there may also be some breaking change detections I need to fix up, I will look

markcowl · 2019-09-12T08:38:10Z

Failures:

This one you will need to fix:

Get-AzBatchSupportedImages   uses the noun 'AzBatchSupportedImages', which does not follow the enforced   naming convention of using a singular noun for a cmdlet name.

This one you will need to suppress


The cmdlet   'Get-AzBatchNodeAgentSku' has been removed and no alias was found for the   original cmdlet name.
--

markcowl · 2019-09-12T08:42:05Z

src/Batch/Batch/BatchAccounts/GetBatchAccountSupportedImagesCommand.cs

@@ -19,8 +19,8 @@

 namespace Microsoft.Azure.Commands.Batch
 {
-    [Cmdlet("Get", ResourceManager.Common.AzureRMConstants.AzurePrefix + "BatchNodeAgentSku"),OutputType(typeof(PSNodeAgentSku))]
-    public class GetBatchAccountNodeAgentSkuCommand : BatchObjectModelCmdletBase
+    [Cmdlet("Get", ResourceManager.Common.AzureRMConstants.AzurePrefix + "BatchSupportedImages"),OutputType(typeof(PSImageInformation))]


BatchSupportedImage

Hey Mark, why should we change this commandlet to be singular? It always returns a collection with more than one item in it.

Because cmdlet names should be singular, even when they act on multiple items, see: https://docs.microsoft.com/en-us/powershell/developer/cmdlet/strongly-encouraged-development-guidelines#use-a-specific-noun-for-a-cmdlet-name-sd01

It is best to follow this rule for all cmdlet names, even when a cmdlet is likely to act upon more than one item.

It is not likely to return more than one item, it is guaranteed to return more than one item. It is not possible to get a single item back from this API call, it's always a collection with multiple values.

https://docs.microsoft.com/en-us/powershell/developer/cmdlet/strongly-encouraged-development-guidelines#use-singular-parameter-names has an allowance for cases where the input is always multi-value

Plural parameter names should be used only in those cases where the value of the parameter is always a multiple-element value.

is there no equivalent for cmdlet names?

No, there is not. This PR will not be accepted without this change.

I believe I've made the required change now, please check

markcowl · 2019-09-16T03:04:09Z

tools/StaticAnalysis/Exceptions/Az.Batch/SignatureIssues.csv

+"Microsoft.Azure.PowerShell.Cmdlets.Batch.dll","Microsoft.Azure.Commands.Batch.NewBatchResourceFileCommand","New-AzBatchResourceFile","1","8100","New-AzBatchResourceFile Does not support ShouldProcess but the cmdlet verb New indicates that it should.","Determine if the cmdlet should implement ShouldProcess and if so determine if it should implement Force / ShouldContinue"
+"D:\a\1\s\artifacts\Debug\Az.Batch\Microsoft.Azure.PowerShell.Cmdlets.Batch.dll","Microsoft.Azure.Commands.Batch.GetBatchAccountSupportedImagesCommand","Get-AzBatchSupportedImages","1","8400","Get-AzBatchSupportedImages uses the noun 'AzBatchSupportedImages', which does not follow the enforced naming convention of using a singular noun for a cmdlet name.","Consider using a singular noun for the cmdlet name."


This is not allowed.

matthchr · 2019-09-17T01:06:14Z

It seems like the test-linux job is failing but it looks to be some internal build error to me...?

And the other 3 seem stuck?

- Includes renaming Get-AzBatchSupportedImages to Get-AzBatchSupportedImage.

matthchr · 2019-09-17T16:43:10Z

@markcowl - Hey Mark, I think this should be good to merge now based on your comments, but for some reason some of the PR gates seem stuck?

matthchr · 2019-09-18T16:18:33Z

Looks like the PR gates got sorted out

matthchr · 2019-09-19T17:25:01Z

@markcowl - Can we merge this now?

matthchr added 3 commits September 10, 2019 08:19

New-AzBatchTask should allow empty command line

be48054

- This fixes Azure#9938.

Fix bug with New-AzureBatchTask -Tasks parameter

a4b5080

- This fixes Azure#9912.

Update Batch Powershell to target new SDKs

172db75

- Target Microsoft.Azure.Batch 12.0. - Target Microsoft.Azure.Management.Batch 9.0. - Update tests.

matthchr changed the base branch from master to Az.Batch-preview September 10, 2019 15:21

azuresdkci added the needs-review label Sep 10, 2019

markcowl self-assigned this Sep 11, 2019

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

markcowl assigned matthchr and unassigned markcowl Sep 11, 2019

matthchr force-pushed the feature/more-preview-stuff branch from 6ea177b to f150e79 Compare September 11, 2019 16:34

This was referenced Sep 11, 2019

[Batch] Commandline argument of New-AzBatchTask should accept empty #9938

Closed

New-AzureBatchTask with -Tasks parameter doesn't propagate OutputFiles #9912

Closed

matthchr added 2 commits September 11, 2019 14:04

Regenerate help

5b3e557

Re-record tests

521e91a

matthchr force-pushed the feature/more-preview-stuff branch from f150e79 to 521e91a Compare September 11, 2019 21:04

markcowl requested changes Sep 12, 2019

View reviewed changes

Add exceptions for expected breaking changes

dc43fd2

markcowl requested changes Sep 16, 2019

View reviewed changes

matthchr force-pushed the feature/more-preview-stuff branch from 8ec30c6 to 2878145 Compare September 16, 2019 18:26

Fix usages of plural (s suffix)

f173f25

- Includes renaming Get-AzBatchSupportedImages to Get-AzBatchSupportedImage.

matthchr force-pushed the feature/more-preview-stuff branch from 2878145 to f173f25 Compare September 17, 2019 15:07

markcowl approved these changes Sep 20, 2019

View reviewed changes

markcowl removed the needs-revision label Sep 20, 2019

markcowl merged commit 07f9d34 into Azure:Az.Batch-preview Sep 20, 2019

matthchr deleted the feature/more-preview-stuff branch September 20, 2019 15:25

matthchr mentioned this pull request Sep 20, 2019

Add deprecation notice on Batch cmdlet #10079

Merged

8 tasks

matthchr mentioned this pull request Oct 15, 2019

Merge Az.Batch-preview to master #10286

Merged

8 tasks

		"Microsoft.Azure.PowerShell.Cmdlets.Batch.dll","Microsoft.Azure.Commands.Batch.NewBatchResourceFileCommand","New-AzBatchResourceFile","1","8100","New-AzBatchResourceFile Does not support ShouldProcess but the cmdlet verb New indicates that it should.","Determine if the cmdlet should implement ShouldProcess and if so determine if it should implement Force / ShouldContinue"
		"D:\a\1\s\artifacts\Debug\Az.Batch\Microsoft.Azure.PowerShell.Cmdlets.Batch.dll","Microsoft.Azure.Commands.Batch.GetBatchAccountSupportedImagesCommand","Get-AzBatchSupportedImages","1","8400","Get-AzBatchSupportedImages uses the noun 'AzBatchSupportedImages', which does not follow the enforced naming convention of using a singular noun for a cmdlet name.","Consider using a singular noun for the cmdlet name."

Update Batch preview module to support data plane API version 2019-08-01.10.0 and mgmt API version 2019-08-01 #10001

Update Batch preview module to support data plane API version 2019-08-01.10.0 and mgmt API version 2019-08-01 #10001

Uh oh!

Conversation

matthchr commented Sep 10, 2019

Description

Checklist

Uh oh!

matthchr commented Sep 10, 2019

Uh oh!

markcowl commented Sep 11, 2019

Uh oh!

matthchr commented Sep 11, 2019

Uh oh!

markcowl commented Sep 12, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

markcowl Sep 12, 2019

Choose a reason for hiding this comment

Uh oh!

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

markcowl Sep 13, 2019

Choose a reason for hiding this comment

Uh oh!

matthchr Sep 13, 2019

Choose a reason for hiding this comment

Uh oh!

markcowl Sep 16, 2019

Choose a reason for hiding this comment

Uh oh!

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

markcowl Sep 16, 2019

Choose a reason for hiding this comment

Uh oh!

matthchr Sep 16, 2019

Choose a reason for hiding this comment

Uh oh!

matthchr commented Sep 17, 2019

Uh oh!

matthchr commented Sep 17, 2019

Uh oh!

matthchr commented Sep 18, 2019

Uh oh!

matthchr commented Sep 19, 2019

Uh oh!

Uh oh!

markcowl commented Sep 12, 2019 •

edited

Loading

matthchr Sep 12, 2019 •

edited

Loading

matthchr Sep 16, 2019 •

edited

Loading