Azure / Azure/azure-sdk-for-python

save-package-properties runs twice per build job, costing ~50s per PR build

Open
#48,395 0 comments 2 reactions 1 assignee Claimed by @JennyPng View on GitHub
EngSys
Dominant language
Python
Stars
5.6k
Forks
3.4k
Avg merge
1d 21h
Merged PRs (30d)
193

Description

`eng/pipelines/templates/steps/build-package-artifacts.yml` invokes `save-package-properties.yml` twice: at line 91 (before the daily-dev version is applied) and again at line 102, after `set-dev-build.yml`.

`set-dev-build.yml` gates the actual version update on `SetDevVersion`, which `daily-dev-build-variable.yml` only sets to `true` when `Build.Reason == 'Schedule'` **and** `System.TeamProject == 'internal'`. The second `save-package-properties` invocation has no such gate, so on PR builds it appears to re-run after nothing has changed.

## Measured

Build 6647478 (`python - pullrequest`, definition 7050) runs it twice in each of three jobs:

| Job | durations | apparently redundant |
|---|---|---|
| Build_Linux | 21.6s, 15.3s | 15.3s |
| Build_MacOS | 17.9s, 13.4s | 13.4s |
| Build_Windows | 58.0s, 21.2s | 21.2s |

Roughly **50s of agent time per PR build**, though spread across parallel jobs, so the wall-clock impact depends on which job is on your critical path.

## Caveat

I am inferring the redundancy from the conditional structure and the timeline data, not from knowledge of this repo. If the second call has a side effect I'm not seeing on PR builds, please close this out — I'd rather be corrected than have you carry a bogus issue.

## Fix path

`save-package-properties.yml` has no `Condition` parameter, and it lives under `eng/common/`, which is synced from `Azure/azure-sdk-tools`. `SetDevVersion` is a runtime variable, so a compile-time `${{ if }}` won't work either.

**Azure/azure-sdk-tools#16640** proposes adding a `Condition` parameter (precedent exists — `verify-readme.yml` and `verify-changelog.yml` already take one). Once that lands, the change here would be to gate the second invocation on `eq(variables['SetDevVersion'], 'true')`.

## How this was found

While measuring CI step timings in `azure-sdk-for-js`, we found the same double-invocation in our `build.yml` (~30s per PR build, tracked in Azure/azure-sdk-for-js#39484). Checking whether the pattern was shared turned up the same structure here, so I verified it against your build timelines. Data came from the public ADO API, which is readable without auth for the `azure-sdk/public` project.

Filed by GitHub Copilot on behalf of @xirzec, from cross-repo CI measurement work. Happy to be told this is a non-issue.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.