-
-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Delay option #99
base: dev
Are you sure you want to change the base?
Add Delay option #99
Conversation
Added Delay option to give number of seconds before disabling a server. Changed Graceful option to use the Delay value if present on the command.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @daimhin - thanks for the PR. Looking at the API and your code, I think we should add a parameter set to make sure only -Graceful 60
or -Delay 30
can be used, i.e. they cannot be specified together?
@@ -64,6 +67,8 @@ function Disable-NSLBServer { | |||
[parameter(Mandatory,ValueFromPipeline = $true, ValueFromPipelineByPropertyName)] | |||
[string[]]$Name = (Read-Host -Prompt 'LB server name'), | |||
|
|||
[int]$Delay, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like the -Graceful
and -Delay
parameters should be mutually exclusive, e.g. use one or the other? If this is the case, we should introduce the Delay
in a separate parameter set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have updated the code to use parameter sets that makes the Graceful and Delay options mutually exclusive.
@@ -87,7 +92,17 @@ function Disable-NSLBServer { | |||
if ($Graceful -gt 0) { | |||
$params.Add('delay', $Graceful) | |||
} else { | |||
$params.Add('delay', 0) | |||
if ($PSBoundParameters.ContainsKey('Delay')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need this check as [Int]
parameters are always 0 unless otherwise specified. You should just be able to safely use $params.Add('delay', $Delay)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I simplified this section now that the Graceful and Delay are mutually exclusive.
- Add Parameter Sets to make the Graceful and Delay options mutually exclusive. Also cleaned up the code where it checks for the values on these parameters.
Added Delay option to give number of seconds before disabling a server.
Changed Graceful option to use the Delay value if present on the command.
Description
Added -Delay option to the command that will give the number of seconds before disabling a server. Also changed the -Graceful to use the value supplied by a Delay.
Related Issue
#98
Motivation and Context
Adding a missing option that is part of the Nitro API.
How Has This Been Tested?
I ran the updated version of this cmdlet against several of our NetScalers to disable servers.
Types of changes
Checklist: