Sync eng/common directory with azure-sdk-tools for PR 16674 - #48536
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 9 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Note
Copilot could not run the full agentic suite for this review because it was automatically requested on a bot-authored pull request. Request a review from Copilot under Reviewers to retry with the full agentic suite. Improved support for bot-authored pull requests is coming soon.
Adds per-agent-pool overrides for PR job batching when generating the PR matrix, allowing different pools to process different numbers of PackageInfo entries per job.
Changes:
- Introduced
PRJobBatchSizeByPoolparameter to specify per-poolPackagesPerPRJoboverrides (including runtime macro support). - Added PowerShell logic to resolve/validate the effective batch size per pool and pass it into
Create-PrJobMatrix.ps1.
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Wei Lim (weikanglim)
left a comment
There was a problem hiding this comment.
The synchronized eng/common change is ready. Required checks are passing and all review threads have been addressed.
Amp-Thread-ID: https://ampcode.com/threads/T-019fc928-8629-77e7-9030-1412de6b8f36 Co-authored-by: Amp <amp@ampcode.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
ee3a11b to
78ad766
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (2)
eng/common/pipelines/templates/jobs/generate-job-matrix.yml:166
- Casting with
[int]will accept some unintended formats (e.g.,'1.9'becomes1, and strings with leading/trailing whitespace may parse). If the intent is to require a strict positive integer, validate with a full numeric regex (e.g.,^[1-9]\\d*$) before converting, and throw if it doesn’t match.
if (-not [string]::IsNullOrWhiteSpace($overrideValue) -and $overrideValue -notmatch '^\$\(.+\)$') {
try {
$batchSize = [int]$overrideValue
}
catch {
throw "PR job batch size override for ${{ pool.name }} must be an integer, got '$overrideValue'."
}
eng/common/pipelines/templates/jobs/generate-job-matrix.yml:159
- This can be simplified and made easier to follow by directly indexing the property (e.g., reading
$overrides.'poolName') instead of enumeratingPSObject.Propertiesand filtering. Direct access avoids extra pipeline steps and makes it clearer what key is being used for the lookup.
$overrides = '${{ convertToJson(parameters.PRJobBatchSizeByPool) }}' | ConvertFrom-Json
$poolOverride = $overrides.PSObject.Properties |
Where-Object { $_.Name -eq '${{ pool.name }}' } |
Select-Object -First 1
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (4)
eng/common/pipelines/templates/jobs/generate-job-matrix.yml:62
- The new comments at lines 56–57 and 61–62 are not indented to the same level as the surrounding
parameterslist items. While YAML parsers typically ignore comment indentation, keeping comments aligned with nearby nodes improves readability and reduces the chance of future indentation mistakes during edits. Consider indenting these comment lines to match the adjacent- name:entries.
# Default number of PackageInfo entries assigned to each PR job.
# Used when a pool-specific override is absent or its runtime variable is unset.
- name: PRJobBatchSize
type: number
default: 10
# Optional map of pool names to per-job batch-size overrides.
# Values may be numbers or runtime variables populated by PreGenerationSteps.
- name: PRJobBatchSizeByPool
type: object
default: {}
eng/common/pipelines/templates/jobs/generate-job-matrix.yml:153
- Embedding JSON into a single-quoted PowerShell string can break if the JSON ever contains a single quote character (e.g., a pool key/value containing
'), producing invalid PowerShell and failingConvertFrom-Json. To make this robust, avoid single-quoted embedding and instead use an escaped/here-string approach (or write the JSON to a file and read it) so the JSON is passed to PowerShell without being affected by quoting rules.
$batchSize = ${{ parameters.PRJobBatchSize }}
$overrides = '${{ convertToJson(parameters.PRJobBatchSizeByPool) }}' | ConvertFrom-Json
eng/common/pipelines/templates/jobs/generate-job-matrix.yml:152
PRJobBatchSizeis defined as a YAMLnumber, but-PackagesPerPRJobconceptually requires an integer (and overrides are explicitly validated/cast to[int]). To keep behavior consistent and fail fast, consider applying the same integer validation/cast to the default$batchSizeas well (e.g., reject non-integer values rather than allowing a decimal through toCreate-PrJobMatrix.ps1).
$batchSize = ${{ parameters.PRJobBatchSize }}
eng/common/pipelines/templates/jobs/generate-job-matrix.yml:172
PRJobBatchSizeis defined as a YAMLnumber, but-PackagesPerPRJobconceptually requires an integer (and overrides are explicitly validated/cast to[int]). To keep behavior consistent and fail fast, consider applying the same integer validation/cast to the default$batchSizeas well (e.g., reject non-integer values rather than allowing a decimal through toCreate-PrJobMatrix.ps1).
if ($batchSize -le 0) {
throw "PR job batch size for ${{ pool.name }} must be greater than zero."
}
Sync eng/common directory with azure-sdk-tools for PR Azure/azure-sdk-tools#16674 See eng/common workflow