-
Notifications
You must be signed in to change notification settings - Fork 225
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.
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
SqlServerDsc: Resources work with SqlServer v22.0.49-preview #1808
SqlServerDsc: Resources work with SqlServer v22.0.49-preview #1808
Conversation
Codecov Report
@@ Coverage Diff @@
## main #1808 +/- ##
====================================
Coverage 91% 91%
====================================
Files 71 71
Lines 7272 7303 +31
====================================
+ Hits 6620 6654 +34
+ Misses 652 649 -3
|
This attempt to use latest preview version of SqlServer didn't fair well with the integration tests. |
Labeling this pull request (PR) as abandoned since it has gone 14 days or more since the last update. An abandoned PR can be continued by another contributor. The abandoned label will be removed if work on this PR is taken up again. |
This PR now uses dbatools, to see if that works. |
Dbatools is missing the assemblies for Analysis Services.
|
e852c21
to
4e40025
Compare
A new preview version of SQL Server was just released, reverting to testing that. |
01ee918
to
d816171
Compare
PoC of using SqlServer v22.0.49 is done, all tests pass. There are some breaking changes in v22 that need to be addressed a bit cleaner. Need to add an optional |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 11 of 17 files at r1, 27 of 27 files at r2, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions
CHANGELOG.md
line 151 at r2 (raw file):
- SqlSetup - Integration tests now used _SqlServer_ module version 22.0.49-preview when running against _SQL Server 2022_, when testing _SQL Server 2016_,
Suggest adding this to the README in tests/integration
for the resource SqlSetup, or ptreq at the top of the file 🤔
Code quote:
- Integration tests now used _SqlServer_ module version 22.0.49-preview
when running against _SQL Server 2022_, when testing _SQL Server 2016_,
_SQL Server 2017_, and _SQL Server 2019_ the module version 21.1.18256
source/DSCResources/DSC_SqlProtocol/README.md
line 31 at r2 (raw file):
## Known issues * When using the resource against an SQL Server 2022 instance, the module
Should be moved to Requirements
Code quote:
* When using the resource against an SQL Server 2022 instance, the module
_SqlServer_ v22.0.49-preview or newer must be installed.
source/DSCResources/DSC_SqlProtocolTcpIp/README.md
line 26 at r2 (raw file):
## Known issues * When using the resource against an SQL Server 2022 instance, the module
Should be moved to Requirements
Code quote:
* When using the resource against an SQL Server 2022 instance, the module
_SqlServer_ v22.0.49-preview or newer must be installed.
source/DSCResources/DSC_SqlReplication/README.md
line 13 at r2 (raw file):
## Known issues * When using the resource against an SQL Server 2022 instance, the module
Should be moved to Requirements
Code quote:
* When using the resource against an SQL Server 2022 instance, the module
_SqlServer_ v22.0.49-preview or newer must be installed.
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
line 480 at r2 (raw file):
} <#
We should remove this commemt.
Code quote:
<#
TODO: This should be made an optional parameter.
This changes the kind of encryption to use when connecting to SQL Server.
Default is that encryption is turned on, which generates the following
exception when the correct certificates are not enabled:
"A connection was successfully established with the server, but then
an error occurred during the login process. (provider: SSL Provider,
error: 0 - The certificate chain was issued by an authority that is
not trusted.)"
This value maps to the Encrypt property SqlConnectionEncryptOption
on the SqlConnection object of the Microsoft.Data.SqlClient driver.
When not specified, the default value is `Mandatory`.
This parameter is new in the module SqlServer v22.x.
For more details, see the article [Connect to SQL Server with strict encryption](https://learn.microsoft.com/en-us/sql/relational-databases/security/networking/connect-with-strict-encryption?view=sql-server-ver16)
and [Configure SQL Server Database Engine for encrypting connections](https://learn.microsoft.com/en-us/sql/database-engine/configure-windows/configure-sql-server-encryption?view=sql-server-ver16).
#>
source/DSCResources/DSC_SqlServiceAccount/README.md
line 14 at r2 (raw file):
* When using the resource against an SQL Server 2022 instance, the module _SqlServer_ v22.0.49-preview or newer must be installed.
Should be moved to Requirements
Code quote:
* When using the resource against an SQL Server 2022 instance, the module
_SqlServer_ v22.0.49-preview or newer must be installed.
tests/Integration/DSC_SqlScriptQuery.config.ps1
line 90 at r2 (raw file):
<# .SYNOPSIS Runs the SQL query as a SQL login.F
An extra 'F' was added by mistake
Code quote:
login.F
tests/Integration/DSC_SqlSetup.config.ps1
line 40 at r2 (raw file):
SupportedFeatures = 'SQLENGINE,REPLICATION' SqlServerModuleVersion = '22.0.49-preview' #21.1.18256
We can remove the comment.
Code quote:
#21.1.18256
tests/Integration/DSC_SqlSetup.config.ps1
line 289 at r2 (raw file):
The SqlServer module is purposely not added to 'RequiredModule.psd1' so that it does not conflict with the SqlServerStubs module that is used by
We can re-add this.
Code quote:
The SqlServer module is purposely not added to 'RequiredModule.psd1' so
that it does not conflict with the SqlServerStubs module that is used by
unit tests.
tests/Integration/DSC_SqlSetup.config.ps1
line 294 at r2 (raw file):
Configuration DSC_SqlSetup_InstallSqlServerModule_Config { Import-DscResource -ModuleName 'PowerShellGet' -ModuleVersion '2.1.2'
If we no longer use this we should remove it here too:
SqlServerDsc/RequiredModules.psd1
Line 50 in bed3b12
PowerShellGet = '2.1.2' |
Code quote:
Import-DscResource -ModuleName 'PowerShellGet'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 9 files at r3.
Reviewable status: 32 of 40 files reviewed, all discussions resolved
CHANGELOG.md
line 151 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Suggest adding this to the README in
tests/integration
for the resource SqlSetup, or ptreq at the top of the file 🤔
Done
source/DSCResources/DSC_SqlProtocol/README.md
line 31 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Should be moved to Requirements
Done
source/DSCResources/DSC_SqlProtocolTcpIp/README.md
line 26 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Should be moved to Requirements
Done
source/DSCResources/DSC_SqlReplication/README.md
line 13 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Should be moved to Requirements
Done
source/DSCResources/DSC_SqlRS/DSC_SqlRS.psm1
line 480 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We should remove this commemt.
Done
source/DSCResources/DSC_SqlServiceAccount/README.md
line 14 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
Should be moved to Requirements
Done
tests/Integration/DSC_SqlScriptQuery.config.ps1
line 90 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
An extra 'F' was added by mistake
Done
tests/Integration/DSC_SqlSetup.config.ps1
line 40 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We can remove the comment.
Done
tests/Integration/DSC_SqlSetup.config.ps1
line 289 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
We can re-add this.
Done
tests/Integration/DSC_SqlSetup.config.ps1
line 294 at r2 (raw file):
Previously, johlju (Johan Ljunggren) wrote…
If we no longer use this we should remove it here too:
SqlServerDsc/RequiredModules.psd1
Line 50 in bed3b12
PowerShellGet = '2.1.2'
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 7 of 9 files at r3, all commit messages.
Reviewable status: 39 of 40 files reviewed, all discussions resolved
Pull Request (PR) description
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