dataplat
Added AppendServerInstance to Invoke-DbaWhoIsActive.
Type of Change
- [ ] Bug fix (non-breaking change, fixes # )
- [X] New feature (non-breaking change, adds functionality, fixes #9540 )
- [ ] Breaking change (affects multiple commands or functionality, fixes # )
- [ ] Ran manual Pester test and has passed (
Invoke-ManualPester) - [ ] Adding code coverage to existing functionality
- [ ] Pester test is included
- [ ] If new file reference added for test, has is been added to github.com/dataplat/appveyor-lab ?
- [ ] Unit test is included
- [X] Documentation
- [ ] Build system
Purpose
When you call Invoke-DbaWhoIsActive on a bunch of servers at once, it pays to know which server you're seeing.
Approach
The key was to realise that the bulk of the code in Invoke-DbaWhoIsActive is for SQL parameters, not PowerShell ones.
Commands to test
I used Invoke-DbaWhoIsActive -SqlInstance $ins -ShowSystemSpids -AppendServerInstance. The glaring omission in my testing is that I only have one instance running locally. I'm already pushing my ancient laptop too hard to risk spinning up another.
Screenshots
Learning
-AppendServerInstance:$AppendServerInstance is a good trick that I didn't know previously. In fact, it concerns me. Is the EnableException parameter in Invoke-DbaWhoIsActive dead code?
Hi @ReeceGoding , when closing https://github.com/dataplat/dbatools/issues/9540 what I meant is "given it's doable in 4 lines let's not add yet another piece of code". Changing code means also adding regression tests to make sure nothing breaks down the line and, frankly, the manpower required to support each and every small iteration of "new params" just adds weight without providing "meaningful" value. I said this lots of times during the years here and in Slack: "dbatools is not dbasolutions".
I totally agree with @niphlod's point.
Just one general thought: If we have a command that takes [DbaInstanceParameter[]]$SqlInstance - an array of instances - the output should always include the originating instance. I think all of the the main (SMO based) commands do this by adding "ComputerName, InstanceName, SqlInstance" as the first three properies in the DefaultView. Maybe we should have a general look at this.
I stick with my original idea of not adding something that can be added easily but I see @andreasjordan point, too.
I'd say this can be closed (at the very least it still needs modifications to the accompanying test) and let's open an improvement to add ComputerName, InstanceName, SqlInstance in the output as @andreasjordan suggested.
Thank you very much, niph. And thank you both @ReeceGoding and @andreasjordan 🙇🏼