Sync eng/common directory with azure-sdk-tools for PR 16674 - #39562
Conversation
|
Azure Pipelines: Successfully started running 1 pipeline(s). 10 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 support for pool-specific PR matrix job batching so different agent pools can process different numbers of PackageInfo entries per PR job.
Changes:
- Introduces a new template parameter
PRJobBatchSizeByPoolfor per-pool batch size overrides. - Adds PowerShell logic to resolve the effective batch size (default vs. override) and validate it.
- Updates the PR matrix generation invocation to pass the resolved
$batchSizeinstead of the global default.
💡 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>
35e3f38 to
b600ab6
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:153
- Embedding JSON into a single-quoted PowerShell string can break if the JSON contains a single quote character (e.g., a pool name or value containing
'), causingConvertFrom-Jsonto fail. To make this robust, write the JSON to a file (similar tomatrix.json) and read/parse it, or use a PowerShell here-string that safely contains the JSON payload.
$batchSize = ${{ parameters.PRJobBatchSize }}
$overrides = '${{ convertToJson(parameters.PRJobBatchSizeByPool) }}' | ConvertFrom-Json
eng/common/pipelines/templates/jobs/generate-job-matrix.yml:156
- This property lookup is more complex than necessary and harder to read/maintain (pipeline + filtering) for a simple key lookup. Consider using a direct lookup approach (e.g., retrieving the property by name from
PSObject.Propertiesor indexing into a hashtable) to reduce indirection and make the intent clearer.
$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 (2)
eng/common/pipelines/templates/jobs/generate-job-matrix.yml:152
- PRJobBatchSize
is declared as a YAMLnumber, which can include non-integer values (e.g.2.5). The current logic only validates> 0and passes$batchSizethrough to-PackagesPerPRJob`, which is conceptually an integer count. Please enforce that the default value is an integer as well (e.g., by validating it’s a whole number and throwing a clear error if not), not only the override path. This prevents accidental fractional values from causing incorrect batching or downstream script errors.
$batchSize = ${{ parameters.PRJobBatchSize }}
eng/common/pipelines/templates/jobs/generate-job-matrix.yml:172
- PRJobBatchSize
is declared as a YAMLnumber, which can include non-integer values (e.g.2.5). The current logic only validates> 0and passes$batchSizethrough to-PackagesPerPRJob`, which is conceptually an integer count. Please enforce that the default value is an integer as well (e.g., by validating it’s a whole number and throwing a clear error if not), not only the override path. This prevents accidental fractional values from causing incorrect batching or downstream script errors.
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