Test-DbaReplLatency - Fix property assignments#10372
Conversation
Related to Issue dataplat#9028
|
thank you very much for the patch! being the first PR after 14 years is an honor 😊 will merge after tests pass. @claude what happened prior to break this? did the property names change in Smo? this must have worked at some point eh? `you have proper permissions now and should work try again. |
There was a problem hiding this comment.
Pull request overview
Fixes Test-DbaReplLatency failing early by aligning replication publication property access with Get-DbaReplPublication output.
Changes:
- Use
NameandDatabaseNamefrom publication objects when configuringTransPublicationandPublicationMonitor - Update output fields to use publication
Name/DatabaseNameand publisher instance identity from the connected server object
| $transPub.Name = $publication.Name | ||
| $transPub.DatabaseName = $publication.DatabaseName |
potatoqualitee
left a comment
There was a problem hiding this comment.
Thanks for the fix @haxr — nice catch for a first PR!
To answer @potatoqualitee's question: SMO didn't change. This has been broken since the initial replication commit (#8958, Sep 2023). Get-DbaReplPublication returns SMO TransPublication/MergePublication objects whose native properties are .Name, .DatabaseName, etc. But Test-DbaReplLatency was written using .PublicationName, .Database, and .Server — properties that never existed on these objects. Looks like it was drafted against a different object shape and never validated end-to-end. So this command has never worked correctly.
One missed fix: Line 130 still has the same stale property name:
$publicationNames = $publicationNames | Where-Object PublicationName -in $PublicationNameThis needs to be Where-Object Name to match the actual SMO property — otherwise the -PublicationName parameter filter silently returns nothing.
(do Test-DbaReplLatency)
(do Test-DbaReplLatency)
|
Very nice, it's been a while since I used the copilot review and it is better now and matches with Claude. I have made the small change that's needed. Thanks again for the fix, it'll be merged soon. |
(do Test-DbaReplLatency)
Type of Change
Invoke-ManualPester)Purpose
To get Test-DbaReplLatency cmdlet to function, it is currently broken
Approach
Fixes the property names to allow the cmdlet to work
Commands to test
The examples should suffice, the cmdlet breaks very early on currently
Screenshots
Learning
First PR, please feel free to edit as needed