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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
