OSOE-481: Action pulls latest SQL Server (=2022-latest) but 2019-latest is used in Lombiq.GitHub.Actions #293

Psichorex · 2023-12-05T14:38:08Z

Summary by CodeRabbit

Refactor
- Updated the SQL Server setup process to allow specifying different SQL Server versions and customizing container names.
- Modified database access logic for more flexible server connection handling.
Tests
- Adjusted test configurations and connection strings to align with the new SQL Server setup and naming conventions.
Chores
- Updated GitHub Actions workflows to use specific references for testing actions, ensuring compatibility with the latest changes.

Psichorex · 2023-12-05T14:38:55Z

.github/actions/setup-sql-server/Initialize-SqlServer.ps1

    )

-    docker pull mcr.microsoft.com/mssql/server &&
+    docker pull mcr.microsoft.com/mssql/server:2022-latest &&


This is specified so if sql server will get a new release there won't be a missmatch between the pulled image and the detached one in docker.

Psichorex · 2023-12-05T14:44:54Z

As far as the documentation says choco install sql-server-express will install sql server 2022 express and it seems to be a unique command for this version. So I don't think further checking is needed.

Edit: Unless they will give a new command for 2022 when the new version will be out and that will just sql-server-express.
Alternatively we would use `choco install sql-server-2022 but that installs the developer edition and not the express.

dministro · 2023-12-06T19:32:51Z

.github/actions/setup-sql-server/Initialize-SqlServer.ps1

In line 3, please do what @Piedone asked here.

Thats what I asked for in my previous comment whether it is acceptable that we use SQL SERVER DEVELOPER instead of EXPRESS becasue I didn't find any particular command in the documentation that specifies for Express 2022.
So basically sql-server-express is dedicated for 2022 express and I didn't find any other command for it. sql-server-2022 will install Developer.

Developer would actually be better.

I changed sql-server-express to sql-server-2022 which eventually will install the Developer version

Also it makes sure that it's version 2022 and its fixed.

dministro · 2023-12-06T19:33:23Z

.github/actions/setup-sql-server/Initialize-SqlServer.ps1

@@ -5,14 +5,14 @@ if ($Env:RUNNER_OS -eq 'Windows')
 else
 {
    $dockerRunSwitches = @(
-        '--name', 'sql2019'
+        '--name', 'sql2022'


DRY, please use a variable or script parameter to store the SQLServer version. Do it here and below.

dministro · 2023-12-06T19:36:33Z

.github/actions/setup-sql-server/Initialize-SqlServer.ps1

    docker run @dockerRunSwitches &&
-    docker exec --user 0 sql2019 bash -c 'mkdir /data; chmod 777 /data --recursive; chown mssql:root /data'
+    docker exec --user 0 sql2022 bash -c 'mkdir /data; chmod 777 /data --recursive; chown mssql:root /data'


@Piedone do we need to use the SQLServer version as the container name? Can we use for example uitt-sqlserver here and Invoke-SolutionTests.ps1:L25. I think not using version makes everything a bit better.

Sure, though I don't think it matters much in practice, since the runners are ephemeral anyway.

The container name is used as an ENV variable that SqlServerManager will use so if we change that here we need to change the ENV var aswell. So do we want to change it or not?

dministro · 2023-12-07T17:11:46Z

.github/actions/setup-sql-server/Initialize-SqlServer.ps1

@@ -1,18 +1,20 @@
 if ($Env:RUNNER_OS -eq 'Windows')
 {
-    choco install sql-server-express --no-progress
+    choco install sql-server-2022 --no-progress


This was not addressed.

dministro · 2023-12-07T17:14:41Z

.github/actions/setup-sql-server/Initialize-SqlServer.ps1

 }
 else
 {
+    $sqlServerName = 'sql2022'
+    $sqlServerVersion = '2022-latest'


You can use only the version eg $sqlServerVersion = '2022' and with string interpolation, you can use it everywhere "mcr.microsoft.com/mssql/server:${sqlServerVersion}-latest".

Why don't you want to create a script parameter for this?

Because we are making a load of changes to a script that was working fine and nobody wanted to touch it, until we realised that 2019 should be swapped to 2022. So according to clean code yes your recommendations are valid but the use case here is so minor and easy I think we are overcomplicating this at this point.

We are making changes because there is a bug in our script and we found a maintainability problem while fixing it.

Script parameter:

param ( [string]$SqlServerVersion = '2022' ) ...

Variable:

... $sqlServerVersion = '2022' ...

I did change to param now.

dministro · 2023-12-07T17:20:01Z

.github/actions/test-dotnet/Invoke-SolutionTests.ps1

@@ -22,7 +22,7 @@ else
 {
    $connectionStringStem = 'Server=.;Database=LombiqUITestingToolbox_{{id}};User Id=sa;Password=Password1!'

-    $Env:Lombiq_Tests_UI__DockerConfiguration__ContainerName = 'sql2019'
+    $Env:Lombiq_Tests_UI__DockerConfiguration__ContainerName = 'sql2022'


You can set the container name here. This is the answer to your question (#293 (comment)), value uitt-sqlserver.

dministro · 2023-12-07T17:21:28Z

.github/actions/setup-sql-server/Initialize-SqlServer.ps1

    $dockerRunSwitches = @(
-        '--name', 'sql2019'
+        '--name', $sqlServerName


Set the container name here, value 'uitt-sqlserver'. Also see my comment below (Invoke-SolutionTests.ps1:L25).

…ssue/OSOE-481

dministro

See my comment: Lombiq/Open-Source-Orchard-Core-Extensions#637 (review)

…ions into issue/OSOE-481

dministro · 2023-12-19T19:22:18Z

.github/actions/setup-sql-server/Initialize-SqlServer.ps1

@@ -1,18 +1,20 @@
 if ($Env:RUNNER_OS -eq 'Windows')
 {
-    choco install sql-server-express --no-progress
+    choco install sql-server-2022 --no-progress


This was not addressed.

Piedone · 2023-12-29T22:42:13Z

@coderabbitai review

I'm testing the AI code review tool CodeRabbit here. Please bear with me. And if it comments some useful feedback, please address it.

coderabbitai · 2023-12-29T22:42:25Z

Important

Auto Review Skipped

Auto reviews are limited to the following labels: coderabbit-review. Please add one of these labels to enable auto reviews.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository.

To trigger a single review, invoke the @coderabbitai review command.

Walkthrough

The changes involve updating GitHub Actions scripts and workflows to handle SQL Server versions more flexibly. The setup script now accepts a parameter to specify the SQL Server version, enabling the use of different versions for different scenarios. The changes also include updates to the way SQL Server is accessed and the naming of Docker containers. Workflow files have been modified to reference specific branches or versions of actions, indicating a move to align testing environments with specific requirements.

Changes

File Path	Change Summary
`.github/actions/setup-sql-server/...`	Added `$sqlServerVersion` parameter to specify SQL Server version for installation and Docker setup; introduced `$sqlServerName` and `$sqlServerLink` for Docker customization.
`.github/actions/setup-sql-server/Wait-SqlServer.ps1`	Modified `sqlcmd` to remove `.\SQLEXPRESS` reference in server connection.
`.github/actions/test-dotnet/Invoke-SolutionTests.ps1`	Updated database connection string and Docker container name for UI tests.
`.github/workflows/.../build-and-test-dotnet.yml`	Updated `uses` field to reference `@issue/OSOE-481` instead of `@dev` for test actions.
`.github/workflows/.../build-and-test-orchard-core.yml`	Changed `uses` field in steps for SQL Server setup and test running to reference different action versions/branches.
`.github/workflows/.../msbuild-and-test.yml`	Modified `uses` field in `Tests` job to use specific action branch `@issue/OSOE-481`.

Assessment against linked issues

Objective	Addressed	Explanation
Ensure the action pulls the correct SQL Server version as specified (`2019-latest` instead of `2022-latest`) [#116]	✅	The introduction of the `$sqlServerVersion` parameter allows for the correct version of SQL Server to be specified and used.
Update workflows to utilize the correct branches or versions of GitHub Actions for consistency with the issue requirements [#116]	✅	The `uses` fields in the workflow files have been updated to reference the specific issue branch `@issue/OSOE-481`, aligning with the issue's requirements.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat with CodeRabbit Bot (`@coderabbitai`)

You can directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit-tests for this file.
You can tag CodeRabbit on specific lines of code or entire files in the PR by tagging @coderabbitai in a comment. Examples:
- @coderabbitai generate unit tests for this file.
- @coderabbitai modularize this function.
You can tag @coderabbitai in a PR comment and ask questions about the PR and the codebase. Examples:
- @coderabbitai generate interesting stats about this repository from git and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit tests.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid.
- @coderabbitai read the files in the src/scheduler package and generate README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
The JSON schema for the configuration file is available here.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

coderabbitai · 2023-12-29T22:42:28Z

@Piedone, I'm ready to review the pull request as requested. Please provide the specific files or code snippets you would like me to focus on, or let me know if you want a general review of the entire pull request.

Action performed

Review triggered.

Piedone · 2023-12-30T00:26:08Z

@coderabbitai review

Sorry about that.

coderabbitai · 2023-12-30T00:26:34Z

@Piedone, no problem at all! I'm here to assist you. Could you please specify which parts of the pull request you would like me to review? If you want a general review of the entire pull request, just let me know and I'll proceed with that.

Action performed

Review triggered.

Piedone · 2023-12-30T00:49:21Z

@coderabbitai review

coderabbitai

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits

Files that changed from the base of the PR and between e97046f and 1ab8ff7.

Files selected for processing (6)

.github/actions/setup-sql-server/Initialize-SqlServer.ps1 (1 hunks)
.github/actions/setup-sql-server/Wait-SqlServer.ps1 (1 hunks)
.github/actions/test-dotnet/Invoke-SolutionTests.ps1 (1 hunks)
.github/workflows/build-and-test-dotnet.yml (1 hunks)
.github/workflows/build-and-test-orchard-core.yml (2 hunks)
.github/workflows/msbuild-and-test.yml (1 hunks)

Additional comments: 10

.github/actions/setup-sql-server/Wait-SqlServer.ps1 (1)

9-9: The removal of the .\SQLEXPRESS instance name from the sqlcmd command will make the script connect to the default SQL Server instance. Confirm that this change aligns with the intended SQL Server configuration and that all necessary instances are configured as default on their respective environments.

.github/actions/setup-sql-server/Initialize-SqlServer.ps1 (3)

1-3: The introduction of the $sqlServerVersion parameter with a default value of '2022' is a good practice for making the script version-agnostic. Ensure that all scripts and workflows that call this script are updated to pass this parameter if a version other than the default is required.

7-7: The update to the Chocolatey install command to use the $sqlServerVersion parameter is correct and allows for dynamic installation of different SQL Server versions. Confirm that the Chocolatey feed used has the expected versions available for installation.

11-24: The Docker commands have been updated to use the $sqlServerVersion parameter for pulling the correct SQL Server image. Ensure that the Docker image tags corresponding to the SQL Server versions are correctly formatted and exist in the container registry.

.github/actions/test-dotnet/Invoke-SolutionTests.ps1 (2)

19-19: The change in the database connection string from '.\SQLEXPRESS' to '.' for Windows environments is consistent with the removal of the instance name in the Wait-SqlServer.ps1 script. Verify that this change is intended and that the default SQL Server instance is configured correctly on Windows runners.

25-25: The Docker container name has been updated to 'uitt-sqlserver'. Confirm that this new name is reflected in all places where the container is referenced or managed, to maintain consistency across the workflows.

.github/workflows/msbuild-and-test.yml (1)

123-123: The uses field in the Tests job has been updated to reference the issue/OSOE-481 branch. Ensure that this branch contains the updated actions and that it is intended to be used for testing purposes before merging into the main branch.

.github/workflows/build-and-test-dotnet.yml (1)

208-208: The uses field in the Tests job has been updated to reference the issue/OSOE-481 branch, similar to the change in the msbuild-and-test.yml workflow. Confirm that this branch is intended for use and contains the correct action versions for the testing workflow.
.github/workflows/build-and-test-orchard-core.yml (2)
238-238: The uses field for setting up SQL Server has been updated to reference the issue/OSOE-481 branch. This change should be verified to ensure that the branch contains the correct and updated SQL Server setup action.

Verification successful

The output from the shell script confirms that the issue/OSOE-481 branch includes updates to the setup-sql-server action, specifically to the Initialize-SqlServer.ps1 and Wait-SqlServer.ps1 scripts. This aligns with the PR's objective to standardize the SQL Server version used in the GitHub Actions.
Scripts Executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Verify that the issue/OSOE-481 branch contains the updated setup-sql-server action.
gh pr view 293 --repo Lombiq/GitHub-Actions --json files --jq '.files[].path' | grep 'setup-sql-server'
Length of output: 213
* 248-248: The `uses` field for running tests has been updated to reference the `issue/OSOE-481` branch. This change should be verified to ensure that the branch contains the correct and updated test action.

Verification successful

The output from the shell script confirms that the issue/OSOE-481 branch includes changes to the test-dotnet action, specifically to the Invoke-SolutionTests.ps1 script and the build-and-test-dotnet.yml workflow file. This suggests that the uses field in the .github/workflows/build-and-test-orchard-core.yml file is correctly pointing to a branch that contains the updated test action as intended by the PR.
Scripts Executed

The following scripts were executed for the analysis:

Script:
#!/bin/bash
# Verify that the issue/OSOE-481 branch contains the updated test-dotnet action.
gh pr view 293 --repo Lombiq/GitHub-Actions --json files --jq '.files[].path' | grep 'test-dotnet'
Length of output: 195

Using better variable name

Updating script to use sqlserver 2022.

0a1f2bd

Psichorex commented Dec 5, 2023

View reviewed changes

Updating branch reference.

56381dc

Psichorex added 2 commits December 5, 2023 18:39

Using sql2022

9518cc6

Updating branch references.

ec6e6fd

BenedekFarkas mentioned this pull request Dec 5, 2023

Hidden, but syntactically incorrect links inserted to issue and PR descriptions (OSOE-750) #294

Closed

dministro requested changes Dec 6, 2023

View reviewed changes

Psichorex added 3 commits December 7, 2023 11:53

Merge remote-tracking branch 'origin/dev' into issue/OSOE-481

7cecb90

Drying script.

76f1c96

Using sqlserver2022 directly.

0e895ef

dministro requested changes Dec 7, 2023

View reviewed changes

Psichorex and others added 5 commits December 7, 2023 19:29

Dry and renaming.

75a2b21

Dry and renaming.

0ecf4cd

Using single quotes.

ea2c39b

Using param.

a5e354a

Merge branch 'dev' of https://github.com/Lombiq/GitHub-Actions into i…

9c5553e

…ssue/OSOE-481

dministro requested changes Dec 10, 2023

View reviewed changes

Psichorex added 12 commits December 11, 2023 12:50

Testing.

b313016

Merge branch 'issue/OSOE-481' of https://github.com/Lombiq/GitHub-Act…

d481e0e

…ions into issue/OSOE-481

Merge remote-tracking branch 'origin/dev' into issue/OSOE-481

04cb286

Using var and version.

31a9026

Using exact version number as it is mandatory.

7ba9378

Using param.

2e8bddf

Fix typo.

1085d06

Trying SQL2022DEVELOPER again.

c3138ff

Removing temp var.

941f172

Using local instead of MSSQLSERVER.

a06d50c

Using MSSQLServer.

0bc635f

Testing.

ba19eaa

Psichorex added 2 commits December 19, 2023 19:36

Reverting to SQL express.

d03e6d4

Trying . as server name.

53a6d0e

dministro requested changes Dec 19, 2023

View reviewed changes

Using var.

1ab8ff7

coderabbitai bot reviewed Dec 30, 2023

View reviewed changes

dministro approved these changes Dec 30, 2023

View reviewed changes

Referencing dev branch

6eaae21

Using better variable name

dministro approved these changes Dec 30, 2023

View reviewed changes

dministro enabled auto-merge December 30, 2023 17:48

Revert changing branch references

4c3861f

dministro added this pull request to the merge queue Dec 31, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 31, 2023

dministro added this pull request to the merge queue Dec 31, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Dec 31, 2023

Referencing dev in workflows

44f0be8

dministro added this pull request to the merge queue Dec 31, 2023

Merged via the queue into dev with commit d209575 Dec 31, 2023
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OSOE-481: Action pulls latest SQL Server (=2022-latest) but 2019-latest is used in Lombiq.GitHub.Actions #293

OSOE-481: Action pulls latest SQL Server (=2022-latest) but 2019-latest is used in Lombiq.GitHub.Actions #293

Psichorex commented Dec 5, 2023 •

edited by coderabbitai bot

Loading

Psichorex Dec 5, 2023

Psichorex commented Dec 5, 2023 •

edited

Loading

dministro Dec 6, 2023

Psichorex Dec 7, 2023

Piedone Dec 7, 2023

Psichorex Dec 7, 2023

Psichorex Dec 7, 2023

dministro Dec 6, 2023

dministro Dec 6, 2023

Piedone Dec 6, 2023

Psichorex Dec 7, 2023

dministro Dec 7, 2023

dministro Dec 19, 2023

dministro Dec 7, 2023

Psichorex Dec 7, 2023 •

edited

Loading

dministro Dec 7, 2023

Psichorex Dec 8, 2023

dministro Dec 7, 2023

dministro Dec 7, 2023

dministro left a comment

dministro Dec 19, 2023

Piedone commented Dec 29, 2023

coderabbitai bot commented Dec 29, 2023 •

edited

Loading

Auto Review Skipped

Chat with CodeRabbit Bot (`@coderabbitai`)

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (`.coderabbit.yaml`)

CodeRabbit Discord Community

coderabbitai bot commented Dec 29, 2023

Piedone commented Dec 30, 2023

coderabbitai bot commented Dec 30, 2023

Piedone commented Dec 30, 2023

coderabbitai bot left a comment

OSOE-481: Action pulls latest SQL Server (=2022-latest) but 2019-latest is used in Lombiq.GitHub.Actions #293

OSOE-481: Action pulls latest SQL Server (=2022-latest) but 2019-latest is used in Lombiq.GitHub.Actions #293

Conversation

Psichorex commented Dec 5, 2023 • edited by coderabbitai bot Loading

Summary by CodeRabbit

Choose a reason for hiding this comment

Psichorex commented Dec 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Psichorex Dec 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dministro left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Piedone commented Dec 29, 2023

coderabbitai bot commented Dec 29, 2023 • edited Loading

Auto Review Skipped

Walkthrough

Changes

Assessment against linked issues

Chat with CodeRabbit Bot (@coderabbitai)

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configration File (.coderabbit.yaml)

CodeRabbit Discord Community

coderabbitai bot commented Dec 29, 2023

Piedone commented Dec 30, 2023

coderabbitai bot commented Dec 30, 2023

Piedone commented Dec 30, 2023

coderabbitai bot left a comment

Choose a reason for hiding this comment

Psichorex commented Dec 5, 2023 •

edited by coderabbitai bot

Loading

Psichorex commented Dec 5, 2023 •

edited

Loading

Psichorex Dec 7, 2023 •

edited

Loading

coderabbitai bot commented Dec 29, 2023 •

edited

Loading

Chat with CodeRabbit Bot (`@coderabbitai`)

CodeRabbit Configration File (`.coderabbit.yaml`)