[LogicApp] Add new Integration Account artifact cmdlets #8483
Conversation
|
@refortie Please fix merge conflicts and I'll take a look tomorrow. |
refortie
force-pushed
the
add-new-ia-artifact-cmdlets
branch
from
22fec5c to
f935301
Compare
src/LogicApp/LogicApp/Cmdlets/IntegrationAccount/GetAzureIntegrationAccountAssemblyCommand.cs
Outdated
Show resolved
Hide resolved
src/LogicApp/LogicApp/Cmdlets/IntegrationAccount/NewAzureIntegrationAccountAssemblyCommand.cs
Outdated
Show resolved
Hide resolved
src/LogicApp/LogicApp/Cmdlets/IntegrationAccount/NewAzureIntegrationAccountAssemblyCommand.cs
Outdated
Show resolved
Hide resolved
| case ParameterSet.ByIntegrationAccountAndParameters: | ||
| { | ||
| var releaseCriteria = new BatchReleaseCriteria(); | ||
| if (this.MessageCount > 0) |
There was a problem hiding this comment.
Instead of doing this validation here, please add a [ValidateRange(0, int.MaxValue)] for MessageCount, BatchSize, and ScheduleInterval
There was a problem hiding this comment.
This isn't validation. When the parameter isn't specified the default value is set, I'm checking to see if the optional parameter was specified.
There was a problem hiding this comment.
Shouldn't you be checking if the parameter is set rather than is greater than 0? Currently there is nothing preventing the user from providing a value less than zero. You can check if that parameter is set using this.MyInvocation.BoundParameters.ContainsKey("MessageCount")
There was a problem hiding this comment.
I'll move over and use that and add the validation in the inputs section.
EDIT: Does using the ValdiateRange supersede the ValidateNotNullOrEmpty? Aka, should I include both or does the ValidateRange also check for nulls
src/LogicApp/LogicApp/Cmdlets/IntegrationAccount/NewAzureIntegrationAccountAssemblyCommand.cs
Outdated
Show resolved
Hide resolved
src/LogicApp/LogicApp/help/Remove-AzIntegrationAccountAssembly.md
Outdated
Show resolved
Hide resolved
| case ParameterSet.ByIntegrationAccountAndParameters: | ||
| { | ||
| var releaseCriteria = new BatchReleaseCriteria(); | ||
| if (this.MessageCount > 0) |
There was a problem hiding this comment.
Shouldn't you be checking if the parameter is set rather than is greater than 0? Currently there is nothing preventing the user from providing a value less than zero. You can check if that parameter is set using this.MyInvocation.BoundParameters.ContainsKey("MessageCount")
| /// <param name="resourceGroupName">Resource group name</param> | ||
| /// <param name="subscriptionId">Subscription id</param> | ||
| /// <returns>App service plan id</returns> | ||
| internal static string BuildAppServicePlanId(string planName, string resourceGroupName, string subscriptionId) |
There was a problem hiding this comment.
Just noticed this wasn't used anywhere and removed it
|
@refortie Nice work! And thanks so much for splitting this up. |
Description
Adding in the cmdlets for Assemblies and Batch Configurations. Completed Design Review: Design Review
Checklist
CONTRIBUTING.mdplatyPSmodule