ServiceBus: Added properties to queue and subscription #5656

v-Ajnava · 2018-03-02T03:38:01Z

Description

Added below optional properties :

Added EnableBatchedOperations property to Queue
Added DeadLetteringOnFilterEvaluationExceptions property to Subscriptions

Checklist

I have read the Submitting Changes section of CONTRIBUTING.md
The title of the PR is clear and informative
The appropriate change log has been updated
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:
- the changes have gone through a cmdlet design review
- the cmdlet markdown files were generated using the platyPS module

…lterEvaluationExceptions property to Subscriptions

cormacpayne

@v-Ajnava a couple of comments that need to be resolved. Also, please make sure to update your change log with an entry that reflects the changes being made in this PR.

cormacpayne · 2018-03-02T06:04:45Z

src/ResourceManager/ServiceBus/Commands.ServiceBus/Cmdlets/Queue/NewAzureServiceBusQueue.cs

+        [Parameter(Mandatory = false, ValueFromPipelineByPropertyName = true, HelpMessage = "Enable Batched Operations - value that indicates whether server-side batched operations are enabled")]
+        [ValidateSet("TRUE", "FALSE", IgnoreCase = true)]
+        [ValidateNotNullOrEmpty]
+        public bool? EnableBatchedOperations { get; set; }


@v-Ajnava bool parameters are strongly discouraged in PowerShell -- please change this to be a SwitchParameter

implemented SwitchParameter

cormacpayne · 2018-03-02T06:05:17Z

...anager/ServiceBus/Commands.ServiceBus/Cmdlets/Subscription/NewAzureServiceBusSubscription.cs

+        [Parameter(Mandatory = false, ValueFromPipelineByPropertyName = true, HelpMessage = "Value that indicates whether a subscription has dead letter support on filter evaluation exceptions.")]
+        [ValidateSet("TRUE", "FALSE", IgnoreCase = true)]
+        [ValidateNotNullOrEmpty]
+        public bool? DeadLetteringOnFilterEvaluationExceptions { get; set; }


@v-Ajnava same comment about changing this to be a SwitchParameter

implemented SwitchParameter

v-Ajnava · 2018-03-02T15:57:58Z

@cormacpayne have updated change log also.

cormacpayne

@v-Ajnava a few additional comments to take a look at

cormacpayne · 2018-03-02T16:37:49Z

src/ResourceManager/ServiceBus/Commands.ServiceBus/Cmdlets/Queue/NewAzureServiceBusQueue.cs


+        [Parameter(Mandatory = false, ValueFromPipelineByPropertyName = true, HelpMessage = "Enable Batched Operations - value that indicates whether server-side batched operations are enabled")]
+        [ValidateSet("TRUE", "FALSE", IgnoreCase = true)]
+        [ValidateNotNullOrEmpty]


@v-Ajnava please remove the ValueFromPipelineByPropertyName property, the ValidateSet attribute, and the ValidateNotNullOrEmpty attribute

cormacpayne · 2018-03-02T16:38:26Z

...anager/ServiceBus/Commands.ServiceBus/Cmdlets/Subscription/NewAzureServiceBusSubscription.cs

+
+        [Parameter(Mandatory = false, ValueFromPipelineByPropertyName = true, HelpMessage = "Value that indicates whether a subscription has dead letter support on filter evaluation exceptions.")]
+        [ValidateSet("TRUE", "FALSE", IgnoreCase = true)]
+        [ValidateNotNullOrEmpty]


@v-Ajnava same comment about removing ValueFromPipelineByPropertyName, ValidateSet and ValidateNotNullOrEmpty

cormacpayne · 2018-03-02T16:38:52Z

...anager/ServiceBus/Commands.ServiceBus/Cmdlets/Subscription/NewAzureServiceBusSubscription.cs

+        [Parameter(Mandatory = false, ValueFromPipelineByPropertyName = true, HelpMessage = "Value that indicates whether a subscription has dead letter support on filter evaluation exceptions.")]
+        [ValidateSet("TRUE", "FALSE", IgnoreCase = true)]
+        [ValidateNotNullOrEmpty]
+        public SwitchParameter DeadLetteringOnFilterEvaluationExceptions { get; set; }


@v-Ajnava is there any way that we can shorten the length of this parameter name?

property name it explains the purpose and shortening it will be not clear.

cormacpayne · 2018-03-02T16:39:36Z

...rceManager/ServiceBus/Commands.ServiceBus.Test/ScenarioTests/ServiceBusSubscriptionTests.ps1

 	$subName = getAssetName "Subscription-"

+
+	# $resltNewSub = New-AzureRmServiceBusSubscription -ResourceGroupName "RGName-970" -Namespace "Namespace-786" -Topic "topic-3510" -Name "TestingSub1"


@v-Ajnava please remove this if it's going to be commented out

my bad, removed.

cormacpayne · 2018-03-02T16:40:31Z

src/ResourceManager/ServiceBus/Commands.ServiceBus.Test/ScenarioTests/ServiceBusQueueTests.ps1

 	$resultGetQueue.DeadLetteringOnMessageExpiration = $True
 	$resultGetQueue.MaxDeliveryCount = 5
 	$resultGetQueue.MaxSizeInMegabytes = 1024
+	$resultGetQueue.EnableBatchedOperations = $True


@v-Ajnava are there any tests that cover the new switch parameters being used rather than the property being set on the object?

its used to set the batchoperations service side. nothing no client side besides setting it.

cormacpayne · 2018-03-02T16:41:10Z

src/ResourceManager/ServiceBus/Commands.ServiceBus/help/New-AzureRmServiceBusQueue.md

+Type: SwitchParameter
+Parameter Sets: (All)
+Aliases:
+Accepted values: TRUE, FALSE


@v-Ajnava make sure to regenerate the help for these two cmdlets after you remove the unnecessary properties on the parameters

Done, the extra properties got removed when removed from cmd file.

cormacpayne · 2018-03-02T19:25:34Z

src/ResourceManager/ServiceBus/Commands.ServiceBus/help/New-AzureRmServiceBusSubscription.md

 ---
 external help file: Microsoft.Azure.Commands.ServiceBus.dll-Help.xml
 Module Name: AzureRM.ServiceBus
-online version: https://docs.microsoft.com/en-us/powershell/module/azurerm.servicebus/new-azurermservicebussubscription


@v-Ajnava please add this link back -- it may have been overridden during the generation of this file

added back the online link

sazeesha · 2018-03-02T22:55:27Z

@cormacpayne we need this for the PS, we have customers asking for this property in PS and the code freeze for that is today. This property is already supported in other resources we have, it was just a bug that we did not have it for Queues.

…nto SBProperties

cormacpayne · 2018-03-05T16:57:04Z

On-demand: https://azuresdkci.westus2.cloudapp.azure.com/job/powershell-demand/552/

Added EnableBatchedOperations property to Queue and DeadLetteringOnFi…

6fca178

…lterEvaluationExceptions property to Subscriptions

v-Ajnava requested a review from cormacpayne March 2, 2018 03:38

cormacpayne suggested changes Mar 2, 2018

View reviewed changes

cormacpayne assigned v-Ajnava Mar 2, 2018

cormacpayne added the needs-revision label Mar 2, 2018

bool to switch, changelog and markdown

af467c9

cormacpayne suggested changes Mar 2, 2018

View reviewed changes

azuresdkci added the needs-review label Mar 2, 2018

v-ajnava added 2 commits March 2, 2018 09:46

review Comments

c8519a7

recordings and test

11e440b

v-Ajnava removed the needs-revision label Mar 2, 2018

cormacpayne reviewed Mar 2, 2018

View reviewed changes

added online help New-AzureRmServiceBusSubscription markdown

188b148

v-Ajnava assigned cormacpayne and unassigned v-Ajnava Mar 2, 2018

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

2bc7a71

…nto SBProperties

cormacpayne approved these changes Mar 5, 2018

View reviewed changes

cormacpayne removed their assignment Mar 5, 2018

cormacpayne added Waiting for On Demand and removed needs-review labels Mar 5, 2018

maddieclayton merged commit 1ad2db5 into Azure:preview Mar 5, 2018

v-Ajnava deleted the SBProperties branch April 12, 2018 01:18

		$subName = getAssetName "Subscription-"


		# $resltNewSub = New-AzureRmServiceBusSubscription -ResourceGroupName "RGName-970" -Namespace "Namespace-786" -Topic "topic-3510" -Name "TestingSub1"

ServiceBus: Added properties to queue and subscription #5656

ServiceBus: Added properties to queue and subscription #5656

Uh oh!

Conversation

v-Ajnava commented Mar 2, 2018

Description

Checklist

Uh oh!

cormacpayne left a comment

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!

v-Ajnava commented Mar 2, 2018

Uh oh!

cormacpayne left a comment

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!

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!

sazeesha commented Mar 2, 2018

Uh oh!

cormacpayne commented Mar 5, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants