Set-SqlDscRSDatabaseTimeout: Configure database timeout settings
#2414
Conversation
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
|
Caution Review failedThe pull request is closed. WalkthroughA new public PowerShell command Changes
Sequence Diagram(s)sequenceDiagram
participant User as PowerShell User
participant Cmd as Set-SqlDscRSDatabaseTimeout
participant CIM as CIM/WMI Layer
participant SSRS as SSRS/PBIRS Service
User->>Cmd: Invoke with Configuration + timeout params
Cmd->>Cmd: Validate params & choose parameter set
Cmd->>User: ShouldProcess prompt / use -Force to skip
User-->>Cmd: Confirm or -Force
alt LogonTimeout specified
Cmd->>CIM: Invoke SetDatabaseLogonTimeout(LogonTimeout)
CIM->>SSRS: Apply logon timeout
SSRS-->>CIM: Success / Error
CIM-->>Cmd: Result
end
alt QueryTimeout specified
Cmd->>CIM: Invoke SetDatabaseQueryTimeout(QueryTimeout)
CIM->>SSRS: Apply query timeout
SSRS-->>CIM: Success / Error
CIM-->>Cmd: Result
end
alt PassThru specified
Cmd->>User: Return Configuration object
else
Cmd->>User: No output (complete)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Around line 10-14: Update the changelog entry for Set-SqlDscRSDatabaseTimeout
to a single concise bullet that includes the issue reference in the format
[issue #<n>](https://github.com/dsccommunity/SqlServerDsc/issues/<n>); mention
the public command name Set-SqlDscRSDatabaseTimeout and that it sets
LogonTimeout and/or QueryTimeout using the WMI methods SetDatabaseLogonTimeout
and SetDatabaseQueryTimeout, all in one line.
In
`@tests/Integration/Commands/Remove-SqlDscRSEncryptionKey.Integration.Tests.ps1`:
- Around line 58-75: The verbose message in the BeforeAll block claims "Waiting
2 minutes..." while Start-Sleep is set to 60 seconds and a commented 300 exists;
update them to match: either change the Write-Verbose text to "Waiting 1
minute..." to reflect Start-Sleep -Seconds 60, or set Start-Sleep -Seconds 120
(and remove or update the trailing comment) to actually wait 2 minutes; update
the Start-Sleep call and the Write-Verbose message together in the BeforeAll
block (referencing Write-Verbose and Start-Sleep) so the log and sleep duration
are consistent.
In `@tests/Unit/Public/Set-SqlDscRSDatabaseTimeout.Tests.ps1`:
- Around line 272-276: The test currently asserts that Invoke-RsCimMethod with
MethodName 'SetDatabaseQueryTimeout' is not called but never actually invokes
the cmdlet under test, so the assertion passes trivially; update the "Should not
call SetDatabaseQueryTimeout method" It block to first call the cmdlet
(Set-SqlDscRSDatabaseTimeout) with the same parameters used in other tests and
then run the Should -Invoke -CommandName Invoke-RsCimMethod -ParameterFilter {
$MethodName -eq 'SetDatabaseQueryTimeout' } -Exactly -Times 0 to verify the CIM
method was not invoked; ensure the cmdlet call matches parameter values used
elsewhere in this test file so the assertion is meaningful.
🧹 Nitpick comments (2)
tests/Unit/Public/Set-SqlDscRSDatabaseTimeout.Tests.ps1 (1)
48-71: Move-ForEachdata toBeforeDiscoveryInlining the data set works, but the test guidelines prefer declaring
-ForEachdata inBeforeDiscoveryfor discovery‑time consistency and reuse.♻️ Suggested refactor
BeforeDiscovery { try { if (-not (Get-Module -Name 'DscResource.Test')) { # Assumes dependencies have been resolved, so if this module is not available, run 'noop' task. if (-not (Get-Module -Name 'DscResource.Test' -ListAvailable)) { # Redirect all streams to $null, except the error stream (stream 2) & "$PSScriptRoot/../../../build.ps1" -Tasks 'noop' 3>&1 4>&1 5>&1 6>&1 > $null } # If the dependencies have not been resolved, this will throw an error. Import-Module -Name 'DscResource.Test' -Force -ErrorAction 'Stop' } } catch [System.IO.FileNotFoundException] { throw 'DscResource.Test module dependency not found. Please run ".\build.ps1 -ResolveDependency -Tasks noop" first.' } + + $parameterSetTestCases = @( + @{ + ExpectedParameterSetName = 'LogonTimeout' + ExpectedParameters = '-Configuration <Object> -LogonTimeout <int> [-PassThru] [-Force] [-WhatIf] [-Confirm] [<CommonParameters>]' + } + @{ + ExpectedParameterSetName = 'QueryTimeout' + ExpectedParameters = '-Configuration <Object> -QueryTimeout <int> [-PassThru] [-Force] [-WhatIf] [-Confirm] [<CommonParameters>]' + } + @{ + ExpectedParameterSetName = 'BothTimeouts' + ExpectedParameters = '-Configuration <Object> -LogonTimeout <int> -QueryTimeout <int> [-PassThru] [-Force] [-WhatIf] [-Confirm] [<CommonParameters>]' + } + ) } @@ - It 'Should have the correct parameters in parameter set <ExpectedParameterSetName>' -ForEach @( - @{ - ExpectedParameterSetName = 'LogonTimeout' - ExpectedParameters = '-Configuration <Object> -LogonTimeout <int> [-PassThru] [-Force] [-WhatIf] [-Confirm] [<CommonParameters>]' - } - @{ - ExpectedParameterSetName = 'QueryTimeout' - ExpectedParameters = '-Configuration <Object> -QueryTimeout <int> [-PassThru] [-Force] [-WhatIf] [-Confirm] [<CommonParameters>]' - } - @{ - ExpectedParameterSetName = 'BothTimeouts' - ExpectedParameters = '-Configuration <Object> -LogonTimeout <int> -QueryTimeout <int> [-PassThru] [-Force] [-WhatIf] [-Confirm] [<CommonParameters>]' - } - ) { + It 'Should have the correct parameters in parameter set <ExpectedParameterSetName>' -ForEach $parameterSetTestCases {As per coding guidelines.
source/Public/Set-SqlDscRSDatabaseTimeout.ps1 (1)
91-93: Prefer a specificOutputTypefor PassThruThe cmdlet returns the configuration CIM instance when
-PassThruis used, so a specificOutputTypeimproves metadata accuracy and aligns with the help text.♻️ Suggested refactor
- [OutputType([System.Object])] + [OutputType([Microsoft.Management.Infrastructure.CimInstance])]As per coding guidelines.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2414 +/- ##
=====================================
Coverage 94% 94%
=====================================
Files 225 226 +1
Lines 11144 11182 +38
=====================================
+ Hits 10507 10545 +38
Misses 637 637
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
source/WikiSource/Change-Report-Server-Service-Account.md (1)
157-165: Remove inline code formatting from parameter names in explanations.Same issue as the previous section: parameters in the "Parameters explained" section should use bold only, not combined with
inline code.📝 Proposed fix
**Parameters explained:** -- `-Credential` — A `PSCredential` object containing the new service account +- **-Credential** — A `PSCredential` object containing the new service account username and password. -- `-RestartService` — Automatically restarts the Report Server service after +- **-RestartService** — Automatically restarts the Report Server service after the change. -- `-SuppressUrlReservationWarning` — Suppresses the warning about URL reservations +- **-SuppressUrlReservationWarning** — Suppresses the warning about URL reservations needing to be updated (we handle this in Step 6). -- `-Force` — Skips confirmation prompts for automation scenarios. +- **-Force** — Skips confirmation prompts for automation scenarios.As per coding guidelines, format parameters using bold in Markdown documentation.
🤖 Fix all issues with AI agents
In `@source/WikiSource/Change-Report-Server-Service-Account.md`:
- Around line 119-124: Update the "Parameters explained" section so parameter
names use bold formatting instead of inline code; replace occurrences of
`-Path`, `-Password`, and `-Force` with **-Path**, **-Password**, and **-Force**
respectively and ensure the surrounding explanatory text remains unchanged.
- Around line 13-15: Update the formatting of the product names in the paragraph
that currently bolds "SQL Server 2019 Reporting Services", "SQL Server 2022
Reporting Services", "Power BI Report Server", and "SQL Server 2017 Reporting
Services" so they use italic instead of bold; leave any parameter names (if
present elsewhere) as bold per guidelines and ensure the sentence preserves the
same wording and punctuation while swapping **...** to _..._ for those
product/module/resource names.
- Around line 286-290: Update the "Parameters explained" list so parameter names
use bold formatting instead of inline code: replace `-Path`, `-Password`, and
`-Force` with **-Path**, **-Password**, and **-Force** in the explanatory
bullets (the bullets describing the file path, SecureString password, and Force
behavior) to comply with the Markdown guidelines.
In
`@tests/Integration/Commands/Remove-SqlDscRSEncryptionKey.Integration.Tests.ps1`:
- Around line 58-75: Update the BeforeAll block to fix the grammar in the
Write-Verbose message and remove the commented-out "#300" after Start-Sleep:
change "1 minutes" to "1 minute" in the Write-Verbose call and delete the
trailing commented code so Start-Sleep -Seconds 60 stands alone; locate the
Write-Verbose and Start-Sleep usages in this BeforeAll block to apply the
change.
🧹 Nitpick comments (1)
tests/Integration/Commands/Set-SqlDscRSDatabaseTimeout.Integration.Tests.ps1 (1)
35-137: Consider restoring original timeout values after each context.These tests change server state; restoring original timeouts in
AfterAllhelps avoid cross-test side effects and environment drift.♻️ Example pattern (apply per context)
BeforeAll { $script:configuration = Get-SqlDscRSConfiguration -InstanceName 'SSRS' -ErrorAction 'Stop' + $script:originalLogonTimeout = $script:configuration.DatabaseLogonTimeout + $script:originalQueryTimeout = $script:configuration.DatabaseQueryTimeout } + + AfterAll { + $null = $script:configuration | Set-SqlDscRSDatabaseTimeout ` + -LogonTimeout $script:originalLogonTimeout ` + -QueryTimeout $script:originalQueryTimeout ` + -Force ` + -ErrorAction 'Stop' + }
tests/Integration/Commands/Remove-SqlDscRSEncryptionKey.Integration.Tests.ps1
Show resolved
Hide resolved
…redundancy in configuration setup
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Pull Request (PR) description
Set-SqlDscRSDatabaseTimeoutto set the database logontimeout and/or query timeout for SQL Server Reporting Services or Power BI
Report Server. Supports setting
LogonTimeout,QueryTimeout, or both viaparameter sets. Uses the
SetDatabaseLogonTimeoutandSetDatabaseQueryTimeoutWMI methods.
This Pull Request (PR) fixes the following issues
None.
Task list
file CHANGELOG.md. Entry should say what was changed and how that
affects users (if applicable), and reference the issue being resolved
(if applicable).
This change is