SqlRs: Add RestartTimeout parameter #2297
Conversation
WalkthroughAdds an optional RestartTimeout (UInt32) to DSC_SqlRS, implementing conditional post-restart Start-Sleep waits in Get/Set/Test flows. Schema and localization strings are extended, integration/unit tests updated to set/mock the timeout, and CHANGELOG documents the fix. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as DSC Engine
participant R as DSC_SqlRS (Set-TargetResource)
participant S as Reporting Services Service
C->>R: Invoke Set-TargetResource(...)
R->>S: Restart-ReportingServicesService
alt RestartTimeout provided
rect #eef8ff
Note over R: WaitingForServiceReady (wait RestartTimeout seconds)
R->>R: Start-Sleep(RestartTimeout)
end
else No RestartTimeout
Note over R: Proceed immediately
end
R->>S: Initialize/Configure Reporting Services
R-->>C: Return result
sequenceDiagram
autonumber
participant C as DSC Engine
participant R as DSC_SqlRS (Test/Get)
participant S as Reporting Services Service
C->>R: Invoke Test-TargetResource / Get-TargetResource(...)
opt Flow triggers service restart
R->>S: Restart-ReportingServicesService
alt RestartTimeout provided
rect #eef8ff
Note over R: Wait RestartTimeout seconds
R->>R: Start-Sleep(RestartTimeout)
end
else No RestartTimeout
Note over R: No delay
end
end
R->>S: Read configuration/status
R-->>C: State / Compliance result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks✅ Passed checks (3 passed)
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: 2
🧹 Nitpick comments (1)
tests/Unit/DSC_SqlRS.Tests.ps1 (1)
516-516: Add test assertions for RestartTimeout behavior.The mock for Start-Sleep is correctly added, but there are no test assertions verifying the RestartTimeout functionality. Consider adding test cases that:
- Verify Start-Sleep is invoked with the correct timeout value when RestartTimeout is provided
- Verify Start-Sleep is NOT invoked when RestartTimeout is omitted
Example assertion to add in relevant test blocks:
Should -Invoke -CommandName Start-Sleep -ParameterFilter { $Seconds -eq 30 } -Exactly -Times 1 -Scope It
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.md(1 hunks)source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1(6 hunks)source/DSCResources/DSC_SqlRS/DSC_SqlRS.schema.mof(1 hunks)source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1(1 hunks)tests/Integration/Resources/DSC_SqlRS.config.ps1(1 hunks)tests/Integration/Resources/DSC_SqlRS_Default.config.ps1(1 hunks)tests/Unit/DSC_SqlRS.Tests.ps1(1 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-markdown.instructions.md)
**/*.md: Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
Use 2 spaces for indentation in Markdown documents
Use '1.' for all items in ordered lists (1/1/1 numbering style)
Disable MD013 for tables/code blocks exceeding 80 characters via an inline comment
Require empty lines before and after code blocks and headings (except before line 1)
Escape backslashes in file paths only, not inside code blocks
All fenced code blocks must specify a language identifier
Format parameter names as bold
Format values/literals as inline code
Format resource/module/product names as italic
Format commands, file names, and paths as inline code
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters- Empty lines required before/after code blocks and headings (except before line 1)
- Escape backslashes in file paths only (not in code blocks)
- Code blocks must specify language identifiers
Text Formatting
- Parameters: bold
- Values/literals:
inline code- Resource/module/product names: italic
- Commands/files/paths:
inline code
Files:
CHANGELOG.md
CHANGELOG.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-changelog.instructions.md)
CHANGELOG.md: Always update the Unreleased section in CHANGELOG.md
Use Keep a Changelog format
Describe notable changes briefly, with no more than 2 items per change type
Reference issues using the format issue #<issue_number>
No empty lines between list items in the same section
Skip adding an entry if the same change already exists in the Unreleased section
No duplicate sections or items in the Unreleased sectionAlways update the Unreleased section of CHANGELOG.md
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe notable changes briefly, ≤2 items per change type
- Reference issues using format issue #<issue_number>
- No empty lines between list items in same section
- Skip adding entry if same change already exists in Unreleased section
- No duplicate sections or items in Unreleased section
Files:
CHANGELOG.md
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
CHANGELOG.mdtests/Integration/Resources/DSC_SqlRS.config.ps1source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1tests/Unit/DSC_SqlRS.Tests.ps1source/DSCResources/DSC_SqlRS/DSC_SqlRS.schema.mofsource/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1tests/Integration/Resources/DSC_SqlRS_Default.config.ps1
**/*.{ps1,psm1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1}: Use descriptive names (3+ characters, no abbreviations)
Function names use PascalCase Verb-Noun with approved verbs
Parameter names use PascalCase
Variable names use camelCase
Keywords are lower-case
Class names use PascalCase
Include scope prefixes for script/global/environment variables: $script:, $global:, $env:
Use one space around operators (e.g., $a = 1 + 2)
Use one space between type and variable (e.g., [String] $name)
Use one space between keyword and parenthesis (e.g., if ($condition))
Place newline before opening brace (except variable assignments)
One newline after opening brace
Two newlines after closing brace (one if followed by another brace or continuation)
Use single quotes unless variable expansion is needed
Arrays on one line use @('one','two'); multi-line arrays have one element per line with proper indentation
Do not use unary comma in return statements to force an array
Single-line comments: '# Comment' capitalized on its own line
Multi-line comments use <# #> with brackets on their own lines and indented text
No commented-out code
Add comment-based help to all functions and scripts
Comment-based help must include SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, and EXAMPLE sections before function/class
Comment-based help indentation: keywords at 4 spaces, text at 8 spaces
Include examples for all parameter sets and combinations
INPUTS: list each pipeline-accepted type with one-line description; repeat .INPUTS per type
OUTPUTS: list each return type with one-line description; repeat .OUTPUTS per type; must match [OutputType()] and actual returns
.NOTES only if critical (constraints, side effects, security, version compatibility, breaking behavior), ≤2 short sentences
Avoid aliases; use full command names
Avoid Write-Host; prefer Write-Verbose/Write-Information/etc.
Avoid Write-Output; use return instead
Do not use ConvertTo-SecureString -AsPlainText in production code
Do not redefine reserved parameters (Verbose, Debug, etc.)
In...
Files:
tests/Integration/Resources/DSC_SqlRS.config.ps1source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1tests/Unit/DSC_SqlRS.Tests.ps1tests/Integration/Resources/DSC_SqlRS_Default.config.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1,psd1}: Indent with 4 spaces; do not use tabs
No spaces on empty lines
Try to limit lines to 120 characters
Empty hashtable is @{}
Hashtable: each property on its own line with proper indentation
Hashtable property names use PascalCase
End files with exactly one blank line
Use line endings as defined by .gitattributes policy
Allow at most two consecutive newlines
No trailing whitespace on any line
Use UTF-8 encoding without BOM for all files
Files:
tests/Integration/Resources/DSC_SqlRS.config.ps1source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1tests/Unit/DSC_SqlRS.Tests.ps1source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1tests/Integration/Resources/DSC_SqlRS_Default.config.ps1
**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Follow PowerShell style and test guideline instructions strictly
Files:
tests/Integration/Resources/DSC_SqlRS.config.ps1tests/Unit/DSC_SqlRS.Tests.ps1tests/Integration/Resources/DSC_SqlRS_Default.config.ps1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description...
Files:
tests/Integration/Resources/DSC_SqlRS.config.ps1source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1tests/Unit/DSC_SqlRS.Tests.ps1source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1tests/Integration/Resources/DSC_SqlRS_Default.config.ps1
source/DSCResources/**/*.psm1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md)
source/DSCResources/**/*.psm1: Every MOF DSC resource module must define the functions: Get-TargetResource, Set-TargetResource, and Test-TargetResource
Export resource functions using the *-TargetResource naming pattern
Get-TargetResource must return a hashtable containing all resource properties
Test-TargetResource must return a boolean ($true/$false)
Set-TargetResource must not return anything (void)
Get-TargetResource parameters: include only those needed to retrieve actual current state values
Get-TargetResource: remove non-mandatory parameters that are never used
Set-TargetResource and Test-TargetResource must have identical parameters
For unused mandatory parameters in Set-TargetResource or Test-TargetResource, add a help comment: "Not used in <function_name>"
Each of Get-/Set-/Test-TargetResource must include at least one Write-Verbose call
Get-TargetResource: Write-Verbose message should start with "Getting the current state of..."
Set-TargetResource: Write-Verbose message should start with "Setting the desired state of..."
Test-TargetResource: Write-Verbose message should start with "Determining the current state of..."
Use localized strings for all messages (e.g., Write-Verbose, Write-Error)
Import localized strings at the top of the module using Get-LocalizedData
Use try/catch blocks to handle exceptions in MOF-based resources
Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
Files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
⚙️ CodeRabbit configuration file
source/DSCResources/**/*.psm1: # MOF-based Desired State Configuration (DSC) Resources GuidelinesRequired Functions
- Every DSC resource must define:
Get-TargetResource,Set-TargetResource,Test-TargetResource- Export using
*-TargetResourcepatternFunction Return Types
Get-TargetResource: Must return hashtable with all resource propertiesTest-TargetResource: Must return boolean ($true/$false)Set-TargetResource: Must not return anything (void)Parameter Guidelines
Get-TargetResource: Only include parameters needed to retrieve actual current state valuesGet-TargetResource: Remove non-mandatory parameters that are never usedSet-TargetResourceandTest-TargetResource: Must have identical parametersSet-TargetResourceandTest-TargetResource: Unused mandatory parameters: Add "Not used in <function_name>" to help commentRequired Elements
- Each function must include
Write-Verboseat least once
Get-TargetResource: Use verbose message starting with "Getting the current state of..."Set-TargetResource: Use verbose message starting with "Setting the desired state of..."Test-TargetResource: Use verbose message starting with "Determining the current state of..."- Use localized strings for all messages (Write-Verbose, Write-Error, etc.)
- Import localized strings using
Get-LocalizedDataat module topError Handling
- Do not use
throwfor terminating errors- Use
try/catchblocks to handle exceptions- Throw localized exceptions using the appropriate
New-*Exceptioncmdlet:
New‑InvalidDataExceptionNew-ArgumentExceptionNew-InvalidOperationException- [
New-ObjectNotFoundException](https:/...
Files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-pester.instructions.md)
**/*.[Tt]ests.ps1: All public commands, private functions, and classes must have unit tests
All public commands and class-based resources must have integration tests
Use Pester v5 syntax only
Test code only inside Describe blocks
Assertions only in It blocks
Never test verbose messages, debug messages, or parameter binding behavior
Pass all mandatory parameters to avoid prompts
Inside It blocks, assign unused return objects to $null (unless part of a pipeline)
Call the tested entity from within the It blocks
Keep results and assertions in the same It block
Avoid try/catch/finally for cleanup; use AfterAll or AfterEach
Avoid unnecessary remove/recreate cycles
One Describe block per file matching the tested entity name
Context descriptions start with 'When'
It descriptions start with 'Should' and must not contain 'when'
Mock variables must be prefixed with 'mock'
Public commands: never use InModuleScope (except for retrieving localized strings)
Private functions and class resources: always use InModuleScope
Each class method gets a separate Context block
Each scenario gets a separate Context block
Use nested Context blocks for complex scenarios
Perform mocking in BeforeAll (use BeforeEach only when required)
Place setup/teardown (BeforeAll/BeforeEach/AfterAll/AfterEach) close to usage
Use PascalCase for Pester keywords: Describe, Context, It, Should, BeforeAll, BeforeEach, AfterAll, AfterEach
Use -BeTrue/-BeFalse; never use -Be $true/-Be $false
Never use Assert-MockCalled; use Should -Invoke instead
Do not use Should -Not -Throw; invoke commands directly
Never add an empty -MockWith block
Omit -MockWith when returning $null
Set $PSDefaultParameterValues for Mock:ModuleName, Should:ModuleName, and InModuleScope:ModuleName
Omit the -ModuleName parameter on Pester commands
Never use Mock inside an InModuleScope block
Define variables for -ForEach in a separate BeforeDiscovery near usage
Use -ForEach only on Context and It blocks
Keep variable scope close to the usage c...
Files:
tests/Unit/DSC_SqlRS.Tests.ps1
⚙️ CodeRabbit configuration file
**/*.[Tt]ests.ps1: # Tests GuidelinesCore Requirements
- All public commands, private functions and classes must have unit tests
- All public commands and class-based resources must have integration tests
- Use Pester v5 syntax only
- Test code only inside
Describeblocks- Assertions only in
Itblocks- Never test verbose messages, debug messages or parameter binding behavior
- Pass all mandatory parameters to avoid prompts
Requirements
- Inside
Itblocks, assign unused return objects to$null(unless part of pipeline)- Tested entity must be called from within the
Itblocks- Keep results and assertions in same
Itblock- Avoid try-catch-finally for cleanup, use
AfterAllorAfterEach- Avoid unnecessary remove/recreate cycles
Naming
- One
Describeblock per file matching the tested entity nameContextdescriptions start with 'When'Itdescriptions start with 'Should', must not contain 'when'- Mock variables prefix: 'mock'
Structure & Scope
- Public commands: Never use
InModuleScope(unless retrieving localized strings)- Private functions/class resources: Always use
InModuleScope- Each class method = separate
Contextblock- Each scenario = separate
Contextblock- Use nested
Contextblocks for complex scenarios- Mocking in
BeforeAll(BeforeEachonly when required)- Setup/teardown in
BeforeAll,BeforeEach/AfterAll,AfterEachclose to usageSyntax Rules
- PascalCase:
Describe,Context,It,Should,BeforeAll,BeforeEach,AfterAll,AfterEach- Use
-BeTrue/-BeFalsenever-Be $true/-Be $false- Never use
Assert-MockCalled, useShould -Invokeinstead- No
Should -Not -Throw- invoke commands directly- Never add an empty
-MockWithblock- Omit
-MockWithwhen returning$null- Set
$PSDefaultParameterValuesforMock:ModuleName,Should:ModuleName,InModuleScope:ModuleName- Omit
-ModuleNameparameter on Pester commands- Never use
Mockinside `InModuleSc...
Files:
tests/Unit/DSC_SqlRS.Tests.ps1
tests/[Uu]nit/**/*.[Tt]ests.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-unit-tests.instructions.md)
tests/[Uu]nit/**/*.[Tt]ests.ps1: In unit tests, access localized strings using InModuleScope -ScriptBlock { $script:localizedData.Key }
When mocking files in tests, use the $TestDrive variable for file system operations
All public commands must include parameter set validation tests
Include the exact Pester setup block before Describe: SuppressMessage attribute with param (); BeforeDiscovery to ensure DscResource.Test is available (fallback to build.ps1 -Tasks 'noop') and Import-Module; BeforeAll to set $script:moduleName, import the module, and set PSDefaultParameterValues for InModuleScope/Mock/Should; AfterAll to remove those defaults and unload the tested module
Use the provided Parameter Set Validation test template to assert command ParameterSetName and full parameter list string; for multiple parameter sets, supply multiple hashtables via -ForEach
Use the Parameter Properties template to assert a parameter is mandatory via $parameterInfo = (Get-Command -Name 'CommandName').Parameters['ParameterName']; $parameterInfo.Attributes.Mandatory | Should -BeTrue
Files:
tests/Unit/DSC_SqlRS.Tests.ps1
⚙️ CodeRabbit configuration file
tests/[Uu]nit/**/*.[Tt]ests.ps1: # Unit Tests Guidelines
- Test with localized strings: Use
InModuleScope -ScriptBlock { $script:localizedData.Key }- Mock files: Use
$TestDrivevariable (path to the test drive)- All public commands require parameter set validation tests
- After modifying classes, always run tests in new session (for changes to take effect)
Test Setup Requirements
Use this exact setup block before
Describe:[System.Diagnostics.CodeAnalysis.SuppressMessageAttribute('PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Suppressing this rule because Script Analyzer does not understand Pester syntax.')] param () 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.' } } BeforeAll { $script:moduleName = '{MyModuleName}' Import-Module -Name $script:moduleName -Force -ErrorAction 'Stop' $PSDefaultParameterValues['InModuleScope:ModuleName'] = $script:moduleName $PSDefaultParameterValues['Mock:ModuleName'] = $script:moduleName $PSDefaultParameterValues['Should:ModuleName'] = $script:moduleName } AfterAll { $PSDefaultParameterValues.Remove('InModuleScope:ModuleName') $PSDefaultParameterValues.Remove('Mock:ModuleNam...
Files:
tests/Unit/DSC_SqlRS.Tests.ps1
source/DSCResources/**/en-US/DSC_*.strings.psd1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md)
Each resource directory must contain an en-US localization folder with a strings file named DSC_.strings.psd1
Files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
source/DSCResources/**/[a-z][a-z]-[A-Z][A-Z]/DSC_*.strings.psd1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md)
source/DSCResources/**/[a-z][a-z]-[A-Z][A-Z]/DSC_*.strings.psd1: Additional localization folders must be named using Get-UICulture names (e.g., fr-FR) and contain DSC_.strings.psd1
In .strings.psd1 files, use underscores as word separators in localized string key names for multi-word keys
Files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
**/*.psd1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
In module manifests, do not use NestedModules for shared commands without specifying RootModule
Files:
source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (8)
source/DSCResources/DSC_SqlRS/DSC_SqlRS.schema.mof (1)
13-13: LGTM!The RestartTimeout property is correctly defined with:
- Appropriate UInt32 type for timeout in seconds
- Write attribute for optional parameter
- Clear, descriptive documentation
tests/Integration/Resources/DSC_SqlRS_Default.config.ps1 (1)
188-192: LGTM!The RestartTimeout parameter is appropriately configured for integration testing:
- 30-second timeout is reasonable for resource-constrained CI systems
- Clear comment explains the purpose
- Proper placement in the configuration block
tests/Integration/Resources/DSC_SqlRS.config.ps1 (1)
190-194: LGTM!The RestartTimeout configuration matches the changes in DSC_SqlRS_Default.config.ps1, maintaining consistency across integration test configurations.
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1 (5)
183-187: LGTM!The parameter documentation is clear, complete, and follows the comment-based help guidelines. It accurately describes the purpose and behavior of RestartTimeout.
272-274: LGTM!The parameter declaration follows coding guidelines:
- Correct type (System.UInt32)
- Properly formatted with type on separate line
- Appropriate placement between SuppressRestart and Encrypt
539-549: LGTM!The restart timeout implementation is correct:
- Uses PSBoundParameters to check if parameter was provided (backward compatible)
- Writes verbose message using localized data
- Applies wait only when parameter is specified
- Well-commented explaining the purpose
830-839: LGTM!The restart timeout logic is correctly applied in the reconfiguration path, consistent with the initialization path implementation at lines 539-549.
887-888: LGTM!The RestartTimeout parameter is correctly included in Test-TargetResource with the "Not used" help comment. This follows the coding guideline that Set-TargetResource and Test-TargetResource must have identical parameters, even when a parameter is not used for testing.
Also applies to: 937-939
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2297 +/- ##
====================================
- Coverage 94% 94% -1%
====================================
Files 150 150
Lines 9227 9236 +9
====================================
+ Hits 8745 8748 +3
- Misses 482 488 +6
🚀 New features to boost your workflow:
|
…for Reporting Services
…ation - Added support for an optional `RestartTimeout` parameter to allow a wait period after service restart, ensuring the WMI provider is ready before initialization. - Updated documentation to clarify the behavior of the new parameter.
johlju
force-pushed
the
fix/sqlrs-intermittent-errors
branch
from
ce2dff5 to
2b81eae
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1 (3)
183-187: Clarify that RestartTimeout is additive to the built-in service wait.The documentation explains the parameter's purpose well, but doesn't mention that this timeout is applied in addition to the 30-second wait already performed by
Restart-ReportingServicesService(line 537, 839). Users settingRestartTimeout = 30will experience a total 60-second wait (30s for service start + 30s for initialization readiness).Consider updating the documentation to clarify:
.PARAMETER RestartTimeout - The number of seconds to wait after restarting Reporting Services before - continuing with configuration. This is useful on resource-constrained - systems where the service may take longer to fully initialize. If not - specified, no additional wait time is applied after service restart. + The number of seconds to wait after the service has started and before + continuing with configuration. This wait is applied in addition to the + 30-second wait for service startup. This is useful on resource-constrained + systems where the WMI provider may take longer to become fully available + after the service starts. If not specified, no additional wait time is + applied.
272-274: Consider adding validation range for RestartTimeout.While
UInt32prevents negative values, there's no upper bound. A user could inadvertently set an extremely large value (e.g., typing an extra digit resulting in 300 instead of 30), causing excessive delays.Consider adding a reasonable range validation:
[Parameter()] + [ValidateRange(0, 600)] [System.UInt32] $RestartTimeout,This would limit the timeout to 10 minutes (600 seconds), which should be sufficient even for severely resource-constrained systems while preventing accidental configuration errors.
539-549: LGTM! Consider extracting repeated wait logic to reduce duplication.The conditional wait implementation is correct and properly uses
PSBoundParameters.ContainsKey()to only wait when explicitly specified. The verbose messages provide good visibility into the wait operations.The same 5-line wait pattern is repeated three times. For improved maintainability, consider extracting to a private helper function:
function Wait-ReportingServicesReady { [CmdletBinding()] param ( [Parameter()] [System.Collections.Hashtable] $BoundParameters, [Parameter()] [System.UInt32] $Timeout ) if ($BoundParameters.ContainsKey('RestartTimeout')) { Write-Verbose -Message ($script:localizedData.WaitingForServiceReady -f $Timeout) Start-Sleep -Seconds $Timeout } }Then replace each wait block with:
Wait-ReportingServicesReady -BoundParameters $PSBoundParameters -Timeout $RestartTimeoutAlso applies to: 565-574, 840-850
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.md(1 hunks)source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1(7 hunks)source/DSCResources/DSC_SqlRS/DSC_SqlRS.schema.mof(1 hunks)source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1(1 hunks)tests/Integration/Resources/DSC_SqlRS.config.ps1(1 hunks)tests/Integration/Resources/DSC_SqlRS_Default.config.ps1(1 hunks)tests/Unit/DSC_SqlRS.Tests.ps1(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- source/DSCResources/DSC_SqlRS/en-US/DSC_SqlRS.strings.psd1
- tests/Unit/DSC_SqlRS.Tests.ps1
- tests/Integration/Resources/DSC_SqlRS_Default.config.ps1
🧰 Additional context used
📓 Path-based instructions (8)
**
⚙️ CodeRabbit configuration file
**: # DSC Community GuidelinesTerminology
- Command: Public command
- Function: Private function
- Resource: DSC class-based resource
Build & Test Workflow Requirements
- Run PowerShell script files from repository root
- Setup build and test environment (once per
pwshsession):./build.ps1 -Tasks noop- Build project before running tests:
./build.ps1 -Tasks build- Always run tests in new
pwshsession:Invoke-Pester -Path @({test paths}) -Output DetailedFile Organization
- Public commands:
source/Public/{CommandName}.ps1- Private functions:
source/Private/{FunctionName}.ps1- Unit tests:
tests/Unit/{Classes|Public|Private}/{Name}.Tests.ps1- Integration tests:
tests/Integration/Commands/{CommandName}.Integration.Tests.ps1Requirements
- Follow instructions over existing code patterns
- Follow PowerShell style and test guideline instructions strictly
- Always update CHANGELOG.md Unreleased section
- Localize all strings using string keys; remove any orphaned string keys
- Check DscResource.Common before creating private functions
- Separate reusable logic into private functions
- DSC resources should always be created as class-based resources
- Add unit tests for all commands/functions/resources
- Add integration tests for all public commands and resources
Files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.schema.mofsource/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1CHANGELOG.mdtests/Integration/Resources/DSC_SqlRS.config.ps1
source/DSCResources/**/*.psm1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-mof-resources.instructions.md)
source/DSCResources/**/*.psm1: Every MOF DSC resource module must define the functions: Get-TargetResource, Set-TargetResource, and Test-TargetResource
Export resource functions using the *-TargetResource naming pattern
Get-TargetResource must return a hashtable containing all resource properties
Test-TargetResource must return a boolean ($true/$false)
Set-TargetResource must not return anything (void)
Get-TargetResource parameters: include only those needed to retrieve actual current state values
Get-TargetResource: remove non-mandatory parameters that are never used
Set-TargetResource and Test-TargetResource must have identical parameters
For unused mandatory parameters in Set-TargetResource or Test-TargetResource, add a help comment: "Not used in <function_name>"
Each of Get-/Set-/Test-TargetResource must include at least one Write-Verbose call
Get-TargetResource: Write-Verbose message should start with "Getting the current state of..."
Set-TargetResource: Write-Verbose message should start with "Setting the desired state of..."
Test-TargetResource: Write-Verbose message should start with "Determining the current state of..."
Use localized strings for all messages (e.g., Write-Verbose, Write-Error)
Import localized strings at the top of the module using Get-LocalizedData
Use try/catch blocks to handle exceptions in MOF-based resources
Do not use throw for terminating errors; instead use New-*Exception helper commands (e.g., New-InvalidDataException, New-ArgumentException, New-InvalidOperationException, New-ObjectNotFoundException, New-InvalidResultException, New-NotImplementedException)
Files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
⚙️ CodeRabbit configuration file
source/DSCResources/**/*.psm1: # MOF-based Desired State Configuration (DSC) Resources GuidelinesRequired Functions
- Every DSC resource must define:
Get-TargetResource,Set-TargetResource,Test-TargetResource- Export using
*-TargetResourcepatternFunction Return Types
Get-TargetResource: Must return hashtable with all resource propertiesTest-TargetResource: Must return boolean ($true/$false)Set-TargetResource: Must not return anything (void)Parameter Guidelines
Get-TargetResource: Only include parameters needed to retrieve actual current state valuesGet-TargetResource: Remove non-mandatory parameters that are never usedSet-TargetResourceandTest-TargetResource: Must have identical parametersSet-TargetResourceandTest-TargetResource: Unused mandatory parameters: Add "Not used in <function_name>" to help commentRequired Elements
- Each function must include
Write-Verboseat least once
Get-TargetResource: Use verbose message starting with "Getting the current state of..."Set-TargetResource: Use verbose message starting with "Setting the desired state of..."Test-TargetResource: Use verbose message starting with "Determining the current state of..."- Use localized strings for all messages (Write-Verbose, Write-Error, etc.)
- Import localized strings using
Get-LocalizedDataat module topError Handling
- Do not use
throwfor terminating errors- Use
try/catchblocks to handle exceptions- Throw localized exceptions using the appropriate
New-*Exceptioncmdlet:
New‑InvalidDataExceptionNew-ArgumentExceptionNew-InvalidOperationException- [
New-ObjectNotFoundException](https:/...
Files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
**/*.{ps1,psm1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1}: Use descriptive names (3+ characters, no abbreviations)
Function names use PascalCase Verb-Noun with approved verbs
Parameter names use PascalCase
Variable names use camelCase
Keywords are lower-case
Class names use PascalCase
Include scope prefixes for script/global/environment variables: $script:, $global:, $env:
Use one space around operators (e.g., $a = 1 + 2)
Use one space between type and variable (e.g., [String] $name)
Use one space between keyword and parenthesis (e.g., if ($condition))
Place newline before opening brace (except variable assignments)
One newline after opening brace
Two newlines after closing brace (one if followed by another brace or continuation)
Use single quotes unless variable expansion is needed
Arrays on one line use @('one','two'); multi-line arrays have one element per line with proper indentation
Do not use unary comma in return statements to force an array
Single-line comments: '# Comment' capitalized on its own line
Multi-line comments use <# #> with brackets on their own lines and indented text
No commented-out code
Add comment-based help to all functions and scripts
Comment-based help must include SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, and EXAMPLE sections before function/class
Comment-based help indentation: keywords at 4 spaces, text at 8 spaces
Include examples for all parameter sets and combinations
INPUTS: list each pipeline-accepted type with one-line description; repeat .INPUTS per type
OUTPUTS: list each return type with one-line description; repeat .OUTPUTS per type; must match [OutputType()] and actual returns
.NOTES only if critical (constraints, side effects, security, version compatibility, breaking behavior), ≤2 short sentences
Avoid aliases; use full command names
Avoid Write-Host; prefer Write-Verbose/Write-Information/etc.
Avoid Write-Output; use return instead
Do not use ConvertTo-SecureString -AsPlainText in production code
Do not redefine reserved parameters (Verbose, Debug, etc.)
In...
Files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1tests/Integration/Resources/DSC_SqlRS.config.ps1
**/*.{ps1,psm1,psd1}
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-powershell.instructions.md)
**/*.{ps1,psm1,psd1}: Indent with 4 spaces; do not use tabs
No spaces on empty lines
Try to limit lines to 120 characters
Empty hashtable is @{}
Hashtable: each property on its own line with proper indentation
Hashtable property names use PascalCase
End files with exactly one blank line
Use line endings as defined by .gitattributes policy
Allow at most two consecutive newlines
No trailing whitespace on any line
Use UTF-8 encoding without BOM for all files
Files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1tests/Integration/Resources/DSC_SqlRS.config.ps1
{**/*.ps1,**/*.psm1,**/*.psd1}
⚙️ CodeRabbit configuration file
{**/*.ps1,**/*.psm1,**/*.psd1}: # PowerShell GuidelinesNaming
- Use descriptive names (3+ characters, no abbreviations)
- Functions: PascalCase with Verb-Noun format using approved verbs
- Parameters: PascalCase
- Variables: camelCase
- Keywords: lower-case
- Classes: PascalCase
- Include scope for script/global/environment variables:
$script:,$global:,$env:File naming
- Class files:
###.ClassName.ps1format (e.g.001.SqlReason.ps1,004.StartupParameters.ps1)Formatting
Indentation & Spacing
- Use 4 spaces (no tabs)
- One space around operators:
$a = 1 + 2- One space between type and variable:
[String] $name- One space between keyword and parenthesis:
if ($condition)- No spaces on empty lines
- Try to limit lines to 120 characters
Braces
- Newline before opening brace (except variable assignments)
- One newline after opening brace
- Two newlines after closing brace (one if followed by another brace or continuation)
Quotes
- Use single quotes unless variable expansion is needed:
'text'vs"text $variable"Arrays
- Single line:
@('one', 'two', 'three')- Multi-line: each element on separate line with proper indentation
- Do not use the unary comma operator (
,) in return statements to force
an arrayHashtables
- Empty:
@{}- Each property on separate line with proper indentation
- Properties: Use PascalCase
Comments
- Single line:
# Comment(capitalized, on own line)- Multi-line:
<# Comment #>format (opening and closing brackets on own line), and indent text- No commented-out code
Comment-based help
- Always add comment-based help to all functions and scripts
- Comment-based help: SYNOPSIS, DESCRIPTION (40+ chars), PARAMETER, EXAMPLE sections before function/class
- Comment-based help indentation: keywords 4 spaces, text 8 spaces
- Include examples for all parameter sets and combinations
- INPUTS: List each pipeline‑accepted type (one per line) with a 1‑line description...
Files:
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1tests/Integration/Resources/DSC_SqlRS.config.ps1
**/*.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-markdown.instructions.md)
**/*.md: Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
Use 2 spaces for indentation in Markdown documents
Use '1.' for all items in ordered lists (1/1/1 numbering style)
Disable MD013 for tables/code blocks exceeding 80 characters via an inline comment
Require empty lines before and after code blocks and headings (except before line 1)
Escape backslashes in file paths only, not inside code blocks
All fenced code blocks must specify a language identifier
Format parameter names as bold
Format values/literals as inline code
Format resource/module/product names as italic
Format commands, file names, and paths as inline code
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
**/*.md: # Markdown Style Guidelines
- Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
- Use 2 spaces for indentation
- Use '1.' for all items in ordered lists (1/1/1 numbering style)
- Disable
MD013rule by adding a comment for tables/code blocks exceeding 80 characters- Empty lines required before/after code blocks and headings (except before line 1)
- Escape backslashes in file paths only (not in code blocks)
- Code blocks must specify language identifiers
Text Formatting
- Parameters: bold
- Values/literals:
inline code- Resource/module/product names: italic
- Commands/files/paths:
inline code
Files:
CHANGELOG.md
CHANGELOG.md
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines-changelog.instructions.md)
CHANGELOG.md: Always update the Unreleased section in CHANGELOG.md
Use Keep a Changelog format
Describe notable changes briefly, with no more than 2 items per change type
Reference issues using the format issue #<issue_number>
No empty lines between list items in the same section
Skip adding an entry if the same change already exists in the Unreleased section
No duplicate sections or items in the Unreleased sectionAlways update the Unreleased section of CHANGELOG.md
Files:
CHANGELOG.md
⚙️ CodeRabbit configuration file
CHANGELOG.md: # Changelog Guidelines
- Always update the Unreleased section in CHANGELOG.md
- Use Keep a Changelog format
- Describe notable changes briefly, ≤2 items per change type
- Reference issues using format issue #<issue_number>
- No empty lines between list items in same section
- Skip adding entry if same change already exists in Unreleased section
- No duplicate sections or items in Unreleased section
Files:
CHANGELOG.md
**/*.ps1
📄 CodeRabbit inference engine (.github/instructions/dsc-community-style-guidelines.instructions.md)
Follow PowerShell style and test guideline instructions strictly
Files:
tests/Integration/Resources/DSC_SqlRS.config.ps1
🧠 Learnings (1)
📚 Learning: 2025-08-29T17:22:37.775Z
Learnt from: CR
PR: dsccommunity/SqlServerDsc#0
File: .github/instructions/dsc-community-style-guidelines-markdown.instructions.md:0-0
Timestamp: 2025-08-29T17:22:37.775Z
Learning: Applies to **/*.md : Wrap lines at word boundaries when over 80 characters (except tables/code blocks)
Applied to files:
CHANGELOG.md
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: dsccommunity.SqlServerDsc (Build Package Module)
- GitHub Check: PSScriptAnalyzer
- GitHub Check: PSScriptAnalyzer
🔇 Additional comments (4)
CHANGELOG.md (1)
133-141: Changelog entry looks goodThanks for capturing the RestartTimeout fix under Fixed with the usual nested bullet formatting—reads clearly and aligns with the rest of the section.
source/DSCResources/DSC_SqlRS/DSC_SqlRS.schema.mof (1)
1-16: Schema update matches resource behaviorThe new
RestartTimeoutwrite property is documented clearly and lines up with the implementation/test updates—nice addition.tests/Integration/Resources/DSC_SqlRS.config.ps1 (1)
190-197: Good call adding RestartTimeout to the integration configSetting
RestartTimeout = 30here (with the rationale documented) keeps the integration flow aligned with the new schema without affecting other parameters.source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1 (1)
898-899: LGTM!The parameter is correctly added to
Test-TargetResourceto maintain parameter consistency withSet-TargetResourceas required by MOF resource guidelines. The help comment appropriately documents that the parameter is not used in this function.Also applies to: 948-950
1 participant
Pull Request (PR) description
DSC_SqlRS(particularly Windows Server 2025 in CI) by adding an optional
RestartTimeoutparameter that allows specifying a wait period (in seconds) after service
restart to allow Reporting Services to fully initialize before attempting
to get configuration data or run initialization methods. When not specified,
no additional wait time is applied, maintaining backward compatibility.
This Pull Request (PR) fixes the following issues
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