Connect-DbaInstance - Report whether a new connection was opened - #10559
Closed
andreasjordan wants to merge 1 commit into
Closed
Connect-DbaInstance - Report whether a new connection was opened#10559andreasjordan wants to merge 1 commit into
andreasjordan wants to merge 1 commit into
Conversation
Commands that connect and clean up afterwards had no way to tell whether Connect-DbaInstance opened a connection for them or handed back the object the caller passed in. They inferred it from their own parameters, which is wrong whenever Connect-DbaInstance has no reason to copy anything, and then they closed a connection that belongs to the caller - taking the session, its temp tables and its database context with it. Connect-DbaInstance now writes that information into the variable behind -IsNewConnectionReference, and Invoke-DbaQuery only closes the connection when it opened one itself. A reference is used instead of a variable name because a variable set via $PSCmdlet.SessionState never reaches a caller inside of dbatools: both share the module session state, so the value lands in the local scope of Connect-DbaInstance instead of the scope of the calling command. See #10554 (do Connect-DbaInstance, Invoke-DbaQuery) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
One unrelated test failed: Will try to analyze later. |
andreasjordan
marked this pull request as draft
August 12, 2026 08:24
Collaborator
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
First step of #10554. Adds the ownership signal and uses it in
Invoke-DbaQuery; the other seven sites follow in their own pull requests.What was wrong
A command that connects and cleans up afterwards had no way to tell whether
Connect-DbaInstanceopened a connection for it or handed back the object the caller passed in.Invoke-DbaQueryinferred it from its own parameters:Connect-DbaInstanceonly copies the connection context when something about it actually has to change, so it frequently returns the very object that was passed in - and then this closed the caller's connection. With-NonPooledConnectionthat takes the session with it:What this changes
Connect-DbaInstancegets-IsNewConnectionReference, which reports whether it opened a new connection, andInvoke-DbaQueryonly disconnects when it did.The value is already known internally as
$isNewConnection:$falsefor Server and SqlConnection inputs,$truefor string, connection string and RegisteredServer inputs, and$truewhen a connection context has to be copied. Only its exposure is new; no connection logic changed. Both places that emit a server set it, including the-SqlConnectionOnlypath.Why a reference and not a variable name
The plan in the issue proposed
-IsNewConnectionVariablewritten through$PSCmdlet.SessionState.PSVariable.Set(). That works for a caller outside of dbatools, but not for a caller inside it, which is exactly the case that matters here. Both commands share the module session state, so the value lands in the local scope ofConnect-DbaInstanceinstead of the scope of the calling command:Set-Variable -Scope 1does not help either - from a module function the parent scope is the module scope, not the caller. A[ref]behaves the same way in both cases, so that is what the parameter takes.The first test run caught this: with the variable-name version,
Invoke-DbaQuerynever learned that it owned its connections and the new session-count test failed with five leaked sessions. That test is in this pull request.Tests
Connect-DbaInstance: the parameter list, plus four assertions that the reference is$truefor a string,$falsewhen a server object is passed back in,$truewhen the context has to be copied, and that the same object comes back when nothing has to changeInvoke-DbaQuery: the caller's connection stays open and its temp table survives, and the connections the command opens itself are still closed, so [Bug] Invoke-DbaQuery doesn't close connections which it creates聽#6210 does not come backBoth files pass against SQL Server 2025 and 2022:
Connect-DbaInstance32 passed, 1 skipped (Azure),Invoke-DbaQuery29 passed.This text was created by Claude and reviewed by Andreas Jordan.