-
-
Notifications
You must be signed in to change notification settings - Fork 3
Update mssql.yml #53
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
base: master
Are you sure you want to change the base?
Update mssql.yml #53
Conversation
rossaddison
commented
Jan 27, 2026
| Q | A |
|---|---|
| Is bugfix? | ✔️/❌ |
| New feature? | ✔️/❌ |
| Breaks BC? | ✔️/❌ |
|
Microsoft's Copilot recommended adjusting the mssql.yml workflow specifically with an empty flag for 2017 and also with a 'less specific more general' tools/path. The toolspath recommendation for 2019 and 2022 is more specific however. Hence the possible reason why 2017 was failing in the workflow given the current settings. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #53 +/- ##
============================================
- Coverage 96.33% 96.21% -0.12%
+ Complexity 102 100 -2
============================================
Files 24 24
Lines 300 291 -9
============================================
- Hits 289 280 -9
Misses 11 11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: Sergei Predvoditelev <sergey.predvoditelev@gmail.com>
Co-authored-by: Sergei Predvoditelev <sergey.predvoditelev@gmail.com>
|
@rossaddison see logs: |
First sed: Changes HTTP to HTTPS in the repository URL (more secure and sometimes required) Second sed: Adds the [signed-by=/usr/share/keyrings/microsoft-prod.gpg] directive to tell apt which keyring to use for verifying packages This ensures apt can properly verify the Microsoft repository packages using the keyring you stored in the first curl command.
|
Driver 18 requires encryption setting whereas 17 does not. So 18 with encryption should be used first. |
|
|
Yes it is not used and is running happily on 18. So I think we just drop legacy support for 17 and use 18. mssql server 2017 should run with odbc-version 18 and then the encryption requirement is consistent across all mssql versions. Will run a different test as close to your original. |
.github/workflows/mssql.yml
Outdated
| curl -fsSL https://packages.microsoft.com/keys/microsoft.asc | gpg --dearmor | sudo tee /usr/share/keyrings/microsoft-prod.gpg > /dev/null | ||
| curl -fsSL https://packages.microsoft.com/config/ubuntu/$(lsb_release -rs)/prod.list | sudo tee /etc/apt/sources.list.d/mssql-release.list | ||
| sudo apt-get update | ||
| sudo ACCEPT_EULA=Y apt-get install -y msodbcsql18 || sudo ACCEPT_EULA=Y apt-get install -y msodbcsql17 |
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.
Question about msodbcsql17 is actual.
Force the workflow to actually run the msodbcsql17 instead of the msodbcsql18.
The later Ubuntu packages do not have msodbcsql17
|
I get this response when the workflow uses ubuntu 20.4: Evaluating tests.if |
|
Ok yiisoft/data-cycle is currently on ubuntu 20.04 so upgrading to ubuntu 22.04 does the trick. The tests have passed but I will check if the tests actually used msodbcsql17. |
|
Ok all the latest-2017 php workflow tests actually use msodbcsql17 from ubuntu 22.04 and do not use the encryption requiring msodbcsql18 and the workflow has been improved to include three additional steps namely:
So the current workflow has been upgraded to, as you suggested to a newer package, namely ubuntu 22.04 from the older ubuntu 20.04 and three additional steps have been included for verification purposes. |
vjik
left a comment
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.
Why do you revert changes from previous reviews?
|
@rossaddison Do you use LLM? |
|
I have been using LLM. Yes not always that consistent. I only use a small subscription of LLM. Most of last month coding without it. The md and mssql.yml was produced with copilot. Useful to see alternatives and learning. |
.github/workflows/mssql.yml
Outdated
| uses: actions/checkout@v3 | ||
|
|
||
| uses: actions/checkout@v6 | ||
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.
Remove unexpected spaces