Fix symbol publishing connectivity and extract into testable script#4171
Open
paulmedynski wants to merge 5 commits intorelease/6.1from
Open
Fix symbol publishing connectivity and extract into testable script#4171paulmedynski wants to merge 5 commits intorelease/6.1from
paulmedynski wants to merge 5 commits intorelease/6.1from
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the MDS pipeline symbol publishing configuration to use the working AKV-backed variables and extracts the publishing logic into a standalone, testable PowerShell script used by the pipeline template.
Changes:
- Switch MDS variables to pull symbol publishing configuration from the
akv-variables-v2variable group. - Replace inline symbol-publishing PowerShell in
publish-symbols-step.ymlwith ascriptPathinvocation ofPublish-Symbols.ps1and updated default variable names. - Add a documented
Publish-Symbols.ps1script plus Pester tests and a small test README.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| eng/pipelines/libraries/mds-variables.yml | Points MDS pipeline variables at akv-variables-v2 for symbol publishing values. |
| eng/pipelines/common/templates/steps/publish-symbols-step.yml | Uses new variable names and runs the extracted Publish-Symbols.ps1 via scriptPath. |
| eng/pipelines/common/templates/steps/Publish-Symbols.ps1 | New standalone script that registers/publishes/checks symbol publish requests via REST API. |
| eng/pipelines/common/templates/steps/tests/Publish-Symbols.Tests.ps1 | Adds Pester tests for validation, URL/body construction, and failure paths. |
| eng/pipelines/common/templates/steps/tests/README.md | Documents prerequisites and how to run the Pester tests. |
- Update MDS pipeline to use akv-variables-v2 variable group for working symbol server configuration (SymbolsPublishServer, SymbolsPublishTokenUri) - Extract inline PowerShell into Publish-Symbols.ps1 with full documentation, parameter validation, and error handling with expanded URI/body in error messages - Append System.JobAttempt to request name for retry uniqueness - Use AzureCLI@2 scriptPath invocation instead of inline script - Add Pester tests for parameter validation, URL construction, request bodies, and error handling
paulmedynski
force-pushed
the
dev/paul/release/6.1/symbols-publishing
branch
from
1734ee8 to
c7395e6
Compare
- Fix request name mismatch: remove JobAttempt from PS1, append _$(System.JobAttempt) in YAML to both PublishSymbols@2 and the script invocation so upload and publish use the same name - Add token validation: trim and check for empty/whitespace token after az CLI returns - Fix error handling: replace Write-Error + throw with throw in catch blocks to avoid masking errors under ErrorActionPreference=Stop - Remove backslash-escaped quotes in YAML >- arguments block - Update Pester tests: remove JobAttempt parameter, add empty token test, update assertions
eng/pipelines/common/templates/steps/tests/Publish-Symbols.Tests.ps1
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/tests/Publish-Symbols.Tests.ps1
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/tests/Publish-Symbols.Tests.ps1
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/tests/Publish-Symbols.Tests.ps1
Outdated
Show resolved
Hide resolved
eng/pipelines/common/templates/steps/tests/Publish-Symbols.Tests.ps1
Outdated
Show resolved
Hide resolved
- Move license into .NOTES section of Publish-Symbols.ps1 so it is visible only with Get-Help -Full - Add license header to Publish-Symbols.Tests.ps1 - Fix 'three steps' to 'four steps' in description - Update README test coverage table to reflect current tests
paulmedynski
added
the
Area\Engineering
Comment on lines
7
to
+10
| variables: | ||
| - group: Release Variables | ||
| # SymbolServer | ||
| # SymbolTokenUri | ||
| - group: Symbols Publishing | ||
| # Shared symbols publishing variables: | ||
| # SymbolsAzureSubscription |
There was a problem hiding this comment.
PR description says this change replaces Release Variables with akv-variables-v2, but this template now imports the Symbols Publishing variable group instead. Please align the PR description (or the variable group reference) so reviewers/operators know which variable group must exist for MDS pipelines to resolve Symbols* variables.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Release Variablesgroup withakv-variables-v2to help eliminate duplicate LIbrary variables.Testing