Skip to content
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

Update Copy-DbaAgentAlert.ps1 #5121

Merged
merged 8 commits into from
Feb 27, 2019
22 changes: 22 additions & 0 deletions functions/Copy-DbaAgentAlert.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,28 @@ function Copy-DbaAgentAlert {
continue
}

$operatorDoesntExist = $false
$sourceAlertNotifications = Get-DbaAgentAlert -SqlInstance $sourceServer.Name | Where-Object Name -eq $serverAlert.name | Select-Object Notifications
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs to pass in just the $sourceServer object and not the property value.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @wsmelton . Fixed and committed.

foreach ($sourceAlertNotification in $sourceAlertNotifications)
{
$operatorName = $sourceAlertNotification.Notifications.OperatorName
if (-Not(Get-DbaAgentOperator -SqlInstance $destinstance | Where-Object Name -eq $operatorName))
{
$operatorDoesntExist = $true
if ($PSCmdlet.ShouldProcess($destinstance, "Operator [$operatorName] does not exist at destination.")) {
$copyAgentAlertStatus.Status = "Skipped"
$copyAgentAlertStatus.Notes = "Operator [$operatorName] doesn't exist on destination"
$copyAgentAlertStatus | Select-DefaultView -Property DateTime, SourceServer, DestinationServer, Name, Type, Status, Notes -TypeName MigrationObject
Write-Message -Message "Operator [$operatorName] does not exist at destination." -Level Verbose
}
}
}
if ($operatorDoesntExist)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be within your ShouldProcess block at line 161 so when user's user -WhatIf they do not get eronous warning messages in the output...we keep the output only to the WhatIf messages.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So something like

if ($operatorNotExists) {
    if ($PSCmdlet.ShouldProcess....) {
        $copyAgentAlertSTatus....
        ...
        Write-Message -Message "Operator []..." -Level Warning
        continue
    }
}

Stop-Function should be used in try/catch blocks and not with if statements. You can simply just output a warning that the alert is being skipped because the operator does not exist, and then continue on.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small note I'd rather use operatorNotExists than use contractions in variable names (just reads better).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @wsmelton . Fixed and committed along with incorporating the approach by @alevyinroc to get the list of operators (with a small correction found while testing). Thanks, @alevyinroc!

{
Stop-Function -Message "Issue moving notifications for the alert [$alertName]: Operator [$operatorName] does not exist at destination." -Category InvalidOperation -Target $destServer
continue
}

if ($destAlerts.name -contains $serverAlert.name) {
if ($force -eq $false) {
if ($PSCmdlet.ShouldProcess($destinstance, "Alert [$alertName] exists at destination. Use -Force to drop and migrate.")) {
Expand Down