You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "potiuk (via GitHub)" <gi...@apache.org> on 2026/07/28 12:30:04 UTC
[PR] Build the docs CI image once and let the registry share it [airflow]
potiuk opened a new pull request, #70618:
URL: https://github.com/apache/airflow/pull/70618
Addresses Question 1 of #70587, and takes Question 2's parallel option.
A provider docs publish built **two** CI images from the same sources — the docs job inline at `publish-docs-to-s3.yml:301-318` (python3.10) and the registry's own `build-ci-image` job (python3.12) — and only started the registry once the docs were already live on S3. Per #70587 the registry sub-workflow medians 23m, of which ~13m is that second image build, and it is 100% additive because it is strictly serial after the publish.
### What changes
* **One image per run.** A `build-ci-image` job in `publish-docs-to-s3.yml` builds from `inputs.ref` and stashes it; `build-docs` restores it instead of building inline, and `update-registry` passes `ci-image-already-built: true` so `registry-build.yml` skips its own build. A standalone registry dispatch has no such image and still builds one.
* **Both on one Python.** `default-python-version` moves 3.10 → 3.12, which is what the registry has always extracted on. This removes the second image that differed only by interpreter. Note `dev/registry/extract_metadata.py:47` and `extract_versions.py:49` fall back to `tomli` below 3.11 — that path is `# pragma: no cover`, so 3.12 is the better-exercised side.
* **Docs and registry run side by side.** `update-registry` moves from `needs: [publish-docs-to-s3, build-info]` to `needs: [build-info, build-ci-image]`. The registry reads nothing the docs publish produces.
* **The no-cache fallback is preserved**, moved into the build step it belongs to. `ci-image-build.yml` gains a `retry-without-cache` input (**default `false`**, so every other caller is unaffected); when the cached build fails it retries once with `DOCKER_CACHE=disabled`. This keeps the behaviour of the inline `breeze ci-image build || docker buildx build --pull` it replaces, for old refs whose `pyproject.toml` has drifted from main.
* **Mount cache retained.** Both image builds now set `upload-mount-cache-artifact: "true"`, so the next publish on the branch restores it rather than re-downloading the dependency set.
* **Cache source unchanged** — `docker-cache: registry` against `branch: main`, the cache the regular Test workflow keeps warm.
Resulting graph:
```
build-info -> build-ci-image -> { build-docs, update-registry }
```
### Consequences worth reviewing
1. **The failed-docs gate is gone.** `needs: [publish-docs-to-s3]` previously *gated* the registry, so a failed docs build skipped it. Running in parallel removes that: the registry can publish `/stable/` links to docs that never landed. #70587 measures the exposure window at median ~3 min, up to ~21 min, negative on 5 of 11 sampled runs. If that is not acceptable, the gate should move onto the S3 sync step rather than back onto the whole job.
2. **The registry publish job re-asserts the committer allowlist.** It previously inherited it by being `needs:` a job the allowlist skipped; now that job is also skipped on the shared-image path, so the check is stated explicitly on `build-and-publish-registry`.
3. **3.10 → 3.12 is exercised by the registry but new for the docs build.** Nothing in the docs path looked version-specific, but this is the change least verifiable without a real release run.
Not done here, and worth a separate look: restoring the canary image instead of building at all. It does not work through the stash — the Tests (AMD) run stashes only the default Python (`ci-image-save-v3-linux_amd64-3.10-…`), the stash name is branch-suffixed with no input to select a branch (current + base/default only), and artifacts expire in ~2 days. The cross-run-viable route would be pulling `ghcr.io/apache/airflow/<branch>/ci/python<ver>:latest`, which `push-image-cache.yml` publishes for every canary Python.
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes — Claude Code (Opus 5)
Generated-by: Claude Code (Opus 5) following [the guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Build the docs CI image once and let the registry share it [airflow]
Posted by "kaxil (via GitHub)" <gi...@apache.org>.
kaxil commented on code in PR #70618:
URL: https://github.com/apache/airflow/pull/70618#discussion_r3669136964
##########
.github/workflows/publish-docs-to-s3.yml:
##########
@@ -606,7 +630,9 @@ jobs:
--destination-location "${SCHEMAS_DESTINATION}" "${args[@]}"
update-registry:
- needs: [publish-docs-to-s3, build-info]
+ # Runs alongside the docs build rather than after it: the registry reads nothing the docs
+ # publish produces, so waiting only added its whole duration to the release manager's wait.
+ needs: [build-info, build-ci-image]
Review Comment:
On the gate question: the sync steps look like the cheap place for it. Measured on run 30335779728, everything the registry job does after the image is 4m30s, and only 1m21s of that is `Sync registry to S3` plus `Publish version metadata` (which is host-side, no CI image needed). The site already goes up as the `registry-site` artifact, so those two steps could move into a job that needs `publish-docs-to-s3`, and the docs branch (~27m of Sphinx plus 4m publish) is long enough that the registry would be idle waiting for it anyway, so the wall clock does not change.
That also retires the #70587 caveat that the registry becomes the critical path on 5 of 11 runs. That was only true while it built its own image.
##########
.github/workflows/publish-docs-to-s3.yml:
##########
@@ -226,8 +228,39 @@ jobs:
echo "publish-execution-api-schema=${PUBLISH_EXEC}" >> ${GITHUB_OUTPUT}
echo "publish-supervisor-schema=${PUBLISH_SUP}" >> ${GITHUB_OUTPUT}
- build-docs:
+ build-ci-image:
+ name: "Build CI image"
needs: [build-info]
+ uses: ./.github/workflows/ci-image-build.yml
+ permissions:
+ contents: read
+ packages: write
+ with:
+ runners: '["ubuntu-22.04"]'
+ platform: "linux/amd64"
+ # Built from the docs ref so the image matches the sources being documented, and
+ # cached against main's registry cache, which the regular Test workflow keeps warm.
+ checkout-ref: ${{ inputs.ref }}
+ push-image: "false"
+ upload-image-artifact: "true"
Review Comment:
Splitting the build into its own job means the docs path now pays an image round-trip it did not before. On run 30335779728, `Export CI docker image` plus `Stash CI docker image` was 4m26s, the consumer side (`Prepare breeze & CI image` in the registry job, minus the breeze install and disk cleanup it shares) about 5m, and the new job repeats the checkout / free-disk / breeze preamble for another ~1m50s. For a provider wave that is a clear net win against a 13-32m second build, but `build-ci-image` has no `if:`, so an `apache-airflow`-only publish skips `update-registry` and pays the whole ~11m for a stash that only `build-docs` reads. Per #70587 that cohort medians 21m, so it is a large relative hit. Worth gating the job on `registry-providers != '' || registry-full-build == 'true'` and leaving the inline build in `build-docs` for that case?
##########
.github/workflows/registry-build.yml:
##########
@@ -97,6 +116,27 @@ jobs:
timeout-minutes: 45
name: "Build & Publish Registry"
needs: [build-ci-image]
+ # `build-ci-image` is skipped when the caller stashed the image, so this cannot simply
+ # inherit its result — but it must keep enforcing the same committer allowlist that
+ # skipping that job used to enforce for us on a dispatch.
+ if: >
+ !cancelled() && needs.build-ci-image.result != 'failure' && (
+ github.event_name == 'workflow_call' ||
Review Comment:
`github.event_name` in a called workflow is the caller's event (`workflow_dispatch` here), never `workflow_call`, so this disjunct never fires and the job ends up gated on `github.event.sender.login` on both paths. Same net behaviour as before, since the old `needs: [build-ci-image]` inherited the identical check, but it does mean this allowlist is load-bearing for the shared-image path and it has drifted from the one on `build-info` in `publish-docs-to-s3.yml`, which also lists `vatsrahul1001`. A wave dispatched by them would publish docs and silently skip the registry.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Build the docs CI image once and let the registry share it [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #70618:
URL: https://github.com/apache/airflow/pull/70618#issuecomment-5108680537
### Backport failed to create: v3-3-test. View the failure log <a href='https://github.com/apache/airflow/actions/runs/30391418242'> Run details </a>
Note: As of [Merging PRs targeted for Airflow 3.X](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x)
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.
In matter of doubt please ask in [#release-management](https://apache-airflow.slack.com/archives/C03G9H97MM2) Slack channel.
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>❌</td>
<td>v3-3-test</td>
<td><a href="https://github.com/apache/airflow/commit/c6449eb429fd045037644dbc42986875fccec1f6"><img src='https://img.shields.io/badge/Commit-c6449eb-red' alt='Commit Link'></a></td>
</tr>
</table>
You can attempt to backport this manually by running:
```bash
cherry_picker c6449eb v3-3-test
```
This should apply the commit to the v3-3-test branch and leave the commit in conflict state marking
the files that need manual conflict resolution.
After you have resolved the conflicts, you can continue the backport process by running:
```bash
cherry_picker --continue
```
If you don't have cherry-picker installed, see the [installation guide](https://github.com/apache/airflow/blob/main/dev/README.md#how-to-backport-pr-with-cherry-picker-cli).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Build the docs CI image once and let the registry share it [airflow]
Posted by "potiuk (via GitHub)" <gi...@apache.org>.
potiuk merged PR #70618:
URL: https://github.com/apache/airflow/pull/70618
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org