Skip to content

Sql Connection Optimization #2433

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.

Sign up for GitHub

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

Merged
johlju merged 18 commits into from
Jan 26, 2026
Merged

Sql Connection Optimization #2433

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
531be16
Remove explicit disconnect as there was no explicit connect
dan-hughes Jan 23, 2026
d6c6633
Create a connection object to create a server connection
dan-hughes Jan 23, 2026
1753689
Update changelog
dan-hughes Jan 23, 2026
1ca5485
Add output type
dan-hughes Jan 23, 2026
4bbb076
Remove copilot instruction
dan-hughes Jan 23, 2026
b08129f
Fix HQRM
dan-hughes Jan 23, 2026
78dfa00
Fix tests
dan-hughes Jan 23, 2026
5afe316
Move ServerConnection to correct namespace
dan-hughes Jan 24, 2026
9896500
Update tests to use correct type for ServerConnection
dan-hughes Jan 24, 2026
6bd572d
Add missing ctor and method
dan-hughes Jan 24, 2026
67e37a4
Put refresh in correct class
dan-hughes Jan 24, 2026
8663f19
Add set to allow test use
dan-hughes Jan 24, 2026
85cc72f
Revert to using New-Object
dan-hughes Jan 24, 2026
46d2a8f
Debug Connect-Sql
dan-hughes Jan 25, 2026
2dcc3f7
Fix HQRM
dan-hughes Jan 25, 2026
32ea5d6
Trigger tests hopefully
dan-hughes Jan 25, 2026
c1949b3
More debug
dan-hughes Jan 25, 2026
07f78cd
Update output type
dan-hughes Jan 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/instructions/SqlServerDsc-guidelines.instructions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ applyTo: "**"
- Unit tests: Add `$env:SqlServerDscCI = $true` in `BeforeAll`, remove in `AfterAll`
- Integration tests:
- Use `Connect-SqlDscDatabaseEngine` for SQL Server DB session, and always with correct CI credentials
- Use `Disconnect-SqlDscDatabaseEngine` after `Connect-SqlDscDatabaseEngine`
- Test config: tests/Integration/Commands/README.md and tests/Integration/Resources/README.md
- Integration test script files must be added to a group within the test stage in ./azure-pipelines.yml.
- Choose the appropriate group number based on the required dependencies
Expand Down
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- SqlServerDsc
- Refactor integration tests for _SQL Server Reporting Services_ and _Power BI_
_Report Server_ ([issue #2431](https://github.com/dsccommunity/SqlServerDsc/issues/2431)).
- `Connect-Sql` create connection and server objects as per documentation.
- `Invoke-SqlDscQuery` remove disconnect as there is not an explicit connect.

## [17.4.0] - 2026-01-19

Expand Down
20 changes: 11 additions & 9 deletions source/Modules/SqlServerDsc.Common/Public/Connect-Sql.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
function Connect-Sql
{
[CmdletBinding(DefaultParameterSetName = 'SqlServer')]
[OutputType([Microsoft.SqlServer.Management.Smo.Server])]
param
(
[Parameter(ParameterSetName = 'SqlServer')]
Expand Down Expand Up @@ -127,8 +128,7 @@ function Connect-Sql
$databaseEngineInstance = '{0}:{1}' -f $Protocol, $databaseEngineInstance
}

$sqlServerObject = New-Object -TypeName 'Microsoft.SqlServer.Management.Smo.Server'
$sqlConnectionContext = $sqlServerObject.ConnectionContext
$sqlConnectionContext = [Microsoft.SqlServer.Management.Common.ServerConnection]::new()
$sqlConnectionContext.ServerInstance = $databaseEngineInstance
$sqlConnectionContext.StatementTimeout = $StatementTimeout
$sqlConnectionContext.ConnectTimeout = $StatementTimeout
Expand Down Expand Up @@ -179,14 +179,16 @@ function Connect-Sql
{
$onlineStatus = 'Online'
$connectTimer = [System.Diagnostics.StopWatch]::StartNew()
$sqlConnectionContext.Connect()
$sqlServerObject = [Microsoft.SqlServer.Management.Smo.Server]::new($sqlConnectionContext)

<#
The addition of the ConnectTimeout property to the ConnectionContext will force the
Connect() method to block until successful. THe SMO object's Status property may not
report 'Online' immediately even though the Connect() was successful. The loop is to
ensure the SMO's Status property was been updated.
The addition of the ConnectTimeout property to the ConnectionContext will force the
Connect() method to block until successful. The SMO object's Status property may not
report 'Online' immediately even though the Connect() was successful. The loop is to
ensure the SMO's Status property was been updated.
#>
$sqlServerObject.ConnectionContext.Connect()

$sleepInSeconds = 2
do
{
Expand Down Expand Up @@ -270,12 +272,12 @@ function Connect-Sql
{
$connectTimer.Stop()
<#
Connect will ensure we actually can connect, but we need to disconnect
Connect() will ensure we actually can connect, but we need to disconnect
from the session so we don't have anything hanging. If we need run a
method on the returned $sqlServerObject it will automatically open a
new session and then close, therefore we don't need to keep this
session open.
#>
$sqlConnectionContext.Disconnect()
$sqlServerObject.ConnectionContext.Disconnect()
Comment on lines 273 to +281
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Jan 23, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Guard against null objects in finally
If Server construction or Connect() throws, Line 281 can throw on $null, masking the original failure. Add null guards (and for $connectTimer) before calling members.

🐛 Proposed fix 
 finally
 {
-    $connectTimer.Stop()
+    if ($null -ne $connectTimer)
+    {
+        $connectTimer.Stop()
+    }
     <#
         Connect() will ensure we actually can connect, but we need to disconnect
         from the session so we don't have anything hanging. If we need run a
         method on the returned $sqlServerObject it will automatically open a
         new session and then close, therefore we don't need to keep this
         session open.
     #>
-    $sqlServerObject.ConnectionContext.Disconnect()
+    if ($null -ne $sqlServerObject -and $null -ne $sqlServerObject.ConnectionContext)
+    {
+        $sqlServerObject.ConnectionContext.Disconnect()
+    }
 }
🤖 Prompt for AI Agents 
In `@source/Modules/SqlServerDsc.Common/Public/Connect-Sql.ps1` around lines 273 -
281, The finally block may call members on null objects and mask the original
exception; update the Connect-Sql.ps1 finally to null-check $connectTimer and
$sqlServerObject before invoking $connectTimer.Stop() and
$sqlServerObject.ConnectionContext.Disconnect() (e.g., only call Stop() if
$connectTimer -ne $null and only call ConnectionContext.Disconnect() if
$sqlServerObject -ne $null and $sqlServerObject.ConnectionContext -ne $null),
preserving existing behavior but avoiding secondary NREs from Connect()
failures.

}
}
4 changes: 0 additions & 4 deletions source/Public/Invoke-SqlDscQuery.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,9 +273,5 @@ function Invoke-SqlDscQuery

end
{
if ($PSCmdlet.ParameterSetName -eq 'ByServerName')
{
$ServerObject | Disconnect-SqlDscDatabaseEngine -Force -Verbose:$VerbosePreference
}
}
}
Loading