Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors SQL Server configuration in Azure DevOps pipelines by centralizing password management and adding diagnostic capabilities for macOS agent setup.
Changes:
- Removed password parameter passing through template layers, replacing it with direct use of the
$(Password)variable from the ADO Library "ADO Test Configuration Properties" - Removed temporary password generation/verification steps from the test job template
- Added documentation comments explaining the Password variable source across all SQL Server configuration step templates
- Enhanced macOS SQL Server setup with timestamp-based diagnostic output and improved error message clarity
- Cleaned up trailing whitespace throughout the pipeline files
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| eng/pipelines/common/templates/steps/configure-sql-server-win-step.yml | Removed password parameter, replaced all parameter references with $(Password) variable, added documentation comment, cleaned up trailing whitespace |
| eng/pipelines/common/templates/steps/configure-sql-server-step.yml | Removed password parameter definition and parameter passing to child templates |
| eng/pipelines/common/templates/steps/configure-sql-server-macos-step.yml | Removed password parameter, replaced with $(Password) variable, added documentation, added PS4 timestamp prompt for diagnostics, improved error log message |
| eng/pipelines/common/templates/steps/configure-sql-server-linux-step.yml | Removed password parameter, replaced with $(Password) variable, added documentation comment, cleaned up trailing whitespace |
| eng/pipelines/common/templates/jobs/ci-run-tests-job.yml | Removed password generation and verification steps that created a temporary GUID-based password |
| - name: operatingSystem | ||
| type: string | ||
| default: '' | ||
| values: |
There was a problem hiding this comment.
I ensured we're defining this parameter as an enum everywhere since we're doing string comparisons to make decisions based on it.
| - ${{ if ne(parameters.prebuildSteps, '') }}: | ||
| - ${{ parameters.prebuildSteps }} # extra steps to run before the build like downloading sni and the required configuration | ||
|
|
||
| - powershell: | |
There was a problem hiding this comment.
We dont' need to generate a GUID and use it as a password. Our Library already has a $(Password) variable meant for SQL Server SA and User passwords.
| echo $SQLCMD_ERRORS | ||
| echo "Errors will be written to: $SQLCMD_ERRORS" | ||
|
|
||
| # Configure the prompt to show the current timestamp so we can see how long each command takes. |
There was a problem hiding this comment.
Diagnostic to see how long each command takes. Azure DevOps refuses to consistently show timestamps.
| displayName: Test job timeout (in minutes) | ||
| type: number | ||
| default: 60 | ||
| default: 70 |
There was a problem hiding this comment.
There's no way to supply the job timeout based on arithmetic because of the way ci-run-tests-job is designed, so we need to increase the timeout for all test jobs. Fingers crossed an extra 10 minutes eliminates the macOS cancellations.
Codecov Report✅ All modified and coverable lines are covered by tests.
Additional details and impacted files@@ Coverage Diff @@
## main #3928 +/- ##
==========================================
- Coverage 74.93% 67.57% -7.36%
==========================================
Files 269 263 -6
Lines 43247 66191 +22944
==========================================
+ Hits 32407 44731 +12324
- Misses 10840 21460 +10620
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…to arithmetically add extra time to macOS jobs.
352833b to
58cb1bb
Compare
Work in progress.