feat: Add a check for MsSql tools path on MsSqlContainer
What does this PR do?
It adds a check on the mssql-tools path which changed during 2022-CU14 update.
Why is it important?
Because from the 2022-CU14 version, Microsoft changed the path to the mssql tools, breaking CI for many folks who have been using the 2022-latest image tag.
Related issues
- Closes #1220
Follow-ups
I'm not a fan of duplicating the logic between WaitUntil and the MsSqlContainer :'(
Deploy Preview for testcontainers-dotnet ready!
| Name | Link |
|---|---|
| Latest commit | 0fb255daf8e6d9992a10b948c8f79688d3f649a6 |
| Latest deploy log | https://app.netlify.com/sites/testcontainers-dotnet/deploys/66d07dca0abbd8000860f2cd |
| Deploy Preview | https://deploy-preview-1221--testcontainers-dotnet.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
Sure no problem :)
Even if Microsoft adds symlink support, changes will be neccessary depending on tool version (especially regarding encryption flags).
Thank you for preparing the PR. I would like to wait a few more days to see if MS decides whether they will add symlink support (or placing the commands in the base path) or not: microsoft/mssql-docker#892 (comment).
Is this PR still in consideration for being merged or not?
Yes, but I still hope that Microsoft responds to the issue and decides to address it properly in their image. IIRC, this affects older versions as well. If we need to work around the issue, we will have to fix it for the older versions of MSSQL and different versions of mssql-tools too 🙃.
Since Microsoft is taking their time to address the issue, I think we should move forward with this pull request. Could we adjust the command to determine the sqlcmd file path to something like this?
SQLCMD_PATH=$(find /opt/mssql-tools*/bin/sqlcmd -type f 2>/dev/null | head -n 1)
It should work for all image versions, including new releases. WDYT?
I've changed the strategy and utilized find to find the sqlcmd binary to avoid relying on fixed paths. This should support upcoming versions and changes as well. I also need to apply the changes to ExecScriptAsync(string, CancellationToken) or find a way to share the approach. I just haven't had the time to finalize it yet. We could probably set sqlCmdFilePath only once, similar to what we're doing in Pulsar.
Thanks for the initial PR and your patience 🙏. I think we now have a good approach that should work for new releases (and different MSSQL versions) right out of the box.