Remove-DbaLinkedServer: Add option to drop logins along with the linked server #7952
Conversation
functions/Remove-DbaLinkedServer.ps1
Outdated
| $lsToDrop.Drop($true) | ||
| } else { | ||
| $lsToDrop.Drop() | ||
| } |
There was a problem hiding this comment.
Could you not just do this as $lsToDrop.Drop([boolean]$Force)? The method can accept a boolean value so if the user does not provide -Force that should equate to doing Drop($false).
Though I do not have an lab to test this on.
There was a problem hiding this comment.
sounds good, updated the code
| $linkedServerName1 = "LS1_$random" | ||
| $linkedServerName2 = "LS2_$random" | ||
| $linkedServerName3 = "LS3_$random" | ||
| $linkedServerName4 = "LS4_$random" |
There was a problem hiding this comment.
Please use dbatoolscli as a prefix for these objects. Majority of our test should be using this when we create objects on the instance incase a user wants to run our tests in their own environment.
There was a problem hiding this comment.
updated this as well
| $warn | Should -BeNullOrEmpty | ||
| $results = Get-DbaLinkedServer -SqlInstance $instance2 -LinkedServer "LS2_$random" | ||
| $results = Get-DbaLinkedServer -SqlInstance $instance2 -LinkedServer $linkedServerName2 | ||
| $results | Should -BeNullOrEmpty |
There was a problem hiding this comment.
Seems a bit excessive to make so many calls in one It block. Is there anything wrong with just having it test the below line. Not sure why we need to perform this test twice either (piping the command or the object via a variable is the same thing).
Get-DbaLinkedServer -SqlInstance $instance2 -LinkedServer $linkedServerName2 | Remove-DbaLinkedServer -WarningVariable warn -Confirm:$false
There was a problem hiding this comment.
The goal is to test both of the input object types. I split this into two separate tests.
|
|
||
| It "Removes a linked server that requires the -Force param" { | ||
| Get-DbaLinkedServer -SqlInstance $instance2 -LinkedServer $linkedServerName4 | Remove-DbaLinkedServer -Confirm:$false -WarningVariable warnings | ||
| $warnings | Should -BeLike "*There are still remote logins or linked logins for the server*" |
There was a problem hiding this comment.
This should be removed, or put it a unique it block for testing the warning message. If this fails it will appear in the results that the use of -Force is broken.
There was a problem hiding this comment.
I moved this into its own test.
|
pawesome! 🐾 thank you both so much |
|
Thanks @lancasteradam! |
Type of Change
.\tests\manual.pester.ps1)Purpose
Add a -Force param so that the logins for a linked server can be dropped at the same time as the linked server.
Approach
Minor change to use a new param.
Commands to test
See the new pester test.