shahar1 commented on code in PR #74466:
URL: https://github.com/apache/airflow/pull/74466#discussion_r4317047343
##########
scripts/docker/install_airflow_when_building_images.sh:
##########
@@ -41,6 +45,9 @@ function install_from_sources() {
if [[ ${VIRTUAL_ENV=} != "" ]]; then
extra_sync_flags="--active"
fi
+ if [[ ${INSTALL_LOCKED_THIRD_PARTY_ONLY=} == "true" ]]; then
+ extra_sync_flags="${extra_sync_flags} --no-install-workspace"
Review Comment:
**nit** — reviewer judgement, no project rule.
The header says that this mode installs "only the third-party dependencies
locked in uv.lock". But if `UPGRADE_RANDOM_INDICATOR_STRING` is also set, the
script takes the upgrade branch and runs `uv sync --resolution highest ...
--no-install-workspace`, which is neither locked nor without a fallback. Today
only the `[[ -z "${UPGRADE_RANDOM_INDICATOR_STRING}" ]]` guard in Dockerfile.ci
prevents that. Consider rejecting (or returning early on) that combination in
the script itself, so that the mode's contract does not depend on every caller
repeating the guard.
##########
Dockerfile.ci:
##########
@@ -1945,13 +1974,23 @@ RUN bash /scripts/docker/install_packaging_tools.sh
COPY --from=scripts install_airflow_when_building_images.sh /scripts/docker/
+COPY --from=dependency-manifests /dependency-manifests/ ${AIRFLOW_SOURCES}/
+ARG UPGRADE_RANDOM_INDICATOR_STRING=""
+# Only preinstall locked third-party dependencies, with the same uv sync as
the full installation.
+# The full sync after COPY still installs workspace packages from the current
checkout and retains
+# its existing resolution fallback, so a stale uv.lock only skips this warm-up
instead of failing the build.
+RUN
--mount=type=cache,id=ci-$TARGETARCH-$DEPENDENCY_CACHE_EPOCH,target=/root/.cache/
\
+ if [[ -z "${UPGRADE_RANDOM_INDICATOR_STRING}" ]]; then \
+ INSTALL_LOCKED_THIRD_PARTY_ONLY="true" bash
/scripts/docker/install_airflow_when_building_images.sh \
+ || echo "Skipping the locked dependency pre-install; the full sync
below installs them"; \
Review Comment:
**nit (optional)** — reviewer judgement, no project rule.
The fail-soft fixes the stale-lock case that potiuk reported, but it also
swallows failures that are not about the lock: a transient index or network
error, or an `lxml`/`xmlsec` source-build failure. When that happens on a
`main` build, the skipped layer is exported with `mode=max` and reused by every
build until the next `pyproject.toml`/`uv.lock` change. The saving then
disappears without any signal in the build result. A permanently broken
pre-install (for example, after a later script change) would also go unnoticed.
One way to keep both properties: decide skip-or-run with a frozen dry run
first. Your own matrix shows that `uv sync --frozen ... --dry-run` already
reports `Missing workspace member` and `Unable to find lockfile`. Then let a
real failure after a passing dry run fail the build. At minimum, put the exit
status in the skip message so the log shows why the step was skipped.
--
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]
